Add stricter TARGETPATH and PATHPATTERN checks

[lukpueh]

Description of issue or feature request:
#1008 removes ambiguity in interfaces that take TARGETPATH and/or PATHPATTERN arguments, by only accepting paths and path patterns that use "/" as directory separator and don't start with "/".

To make the interface even stronger, we should only accept paths and path patterns that completely adhere to the recommended format.

Note that, although the specification only recommends a format, we agreed that an implementation can be more assertive than the spec (see spec#63 and spec#67).

Current behavior:

• Functions that take TARGETPATH and/or PATHPATTERN arguments only accept strings that use "/" as directory separator and don't start with "/".

Expected behavior:

• Functions that take TARGETPATH arguments only accept path-relative-scheme-less-URL strings.

• Functions that take PATHPATTERN arguments only accept path-relative-scheme-less-URL strings that may include shell-style glob characters. I suggest to align this with whatever the client updater supports.

[jku]

I wonder if there's anything more to do here?

[lukpueh]

I think the big question is how much input validation should be performed in the Metadata API, which still remains unanswered (#1130)

The checks requested in this ticket are currently not performed in the relevant interfaces:

TARGETPATH not checked at all

python-tuf/tuf/api/metadata.py

Line 1503 in 8ac7167

self.path = path

PATHPATTERN only checked to be strings

python-tuf/tuf/api/metadata.py

Lines 1278 to 1279 in 8ac7167

	if paths is not None and any(not isinstance(p, str) for p in paths):
	raise ValueError("Paths must be strings")

I lean towards implementing the requested checks, also to be consistent with places where we do check more strictly eg.

python-tuf/tuf/api/metadata.py

Lines 1498 to 1499 in 8ac7167

	self._validate_length(length)
	self._validate_hashes(hashes)

cc @MVrachev (head of validation)

[MVrachev]

We have implemented validation for quite many attributes of the Metadata classes (see #1140 (comment)).
For some of them, we have decided there is no sense in checking their type or content (as for Root.keys for example) for others we have added as for paths.
I am open to rediscussing why some of the attributes are missing validation.
We have generally decided to not apply the same approach for all attributes as the different attributes are used differently.

In this particular case:

TARGETPATH not checked at all

Together with @jku we had discussions about how we should handle TARGETPATH and METAPATH and we have come up with: #1668 (comment)

PATHPATTERN only checked to be strings

Again with @jku we had discussions for PATHPATTERN as well here: #1461 (comment)

@lukpueh what do you think about those arguments?
It's important to consider the different opinions as it's possible we have made a mistake.

[lukpueh]

Thanks for sharing those resources, @MVrachev. Do I understand correctly that they argue that we don't need the check because the client will fail gracefully for any string even if it does not have the recommended format? That would be a fair argument. Although it could also be used against the _validate_length-check.

[jku]

I suppose for TARGETPATH my logic was this:

• handling things like "/ cannot be a starting character" or "\ cannot be a directory separator" are workarounds
• the issue they worked around is that TARGETPATH was being used as a local file path. This is impossible to implement comprehensively (filesystems have different requirements and TARGETPATH must still be a URL fragment) and very hard to implement safely -- leading to workarounds
• the proper solution is to not use TARGETPATH as a local file path, and treat it as just an identifier that is a URL path fragment (a specific client could define rules that make it possible to use TARGETPATH as a filepath (like define only [a-z] as valid chars) but a library probably should not)

PATHPATTERN just follows from there.

[lukpueh]

Thanks for the follow-up, @jku! I added a note about this workaround to the repository tool feature request ticket for a convenience function that builds TARGETPATHs from local files.

I agree that the Metadata API should not provide that feature (workaround) and as a consequence does not have to vet these parameters as strictly. Closing here..