Tuf: Conflicting file path assertions in `delegate()` and `add_paths()`

[lukpueh]

Description of issue or feature request:

Related to #957

The Targets.delegate()method fails if one of the file paths or patterns passed via the paths argument is absolute, i.e. starts with a directory separator. At the same time it warns if it the path does not have self._targets_directory as prefix, which very well may be an absolute path.

https://github.com/theupdateframework/tuf/blob/409eef1a4872a88b610e81b375f36b07a8e0f996/tuf/repository_tool.py#L2286-L2294

The Targets.add_paths() methods emits a similar message (with debug level). Note that the method does not check whether the passed paths have a leading directory separator, although it should, in order to be consistent with above function.

https://github.com/theupdateframework/tuf/blob/409eef1a4872a88b610e81b375f36b07a8e0f996/tuf/repository_tool.py#L1868-L1872

Current behavior:

• Targets.delegate() and Targets.add_paths() emit warning/debug message if a passed path or path pattern is not prefixed with the targets directory base path, i.e. self._targets_directory.
• Targets.add_paths does not check if the passed paths are indeed relative.

Expected behavior:

Either,

• Targets.delegate() and Targets.add_paths() should not care about the prefix of the passed paths or path patterns.
• Targets.add_paths should probably assert that the passed paths are relative, to be consistent with the sibling the corresponding functionality of delegate().

[lukpueh]

When fixing this issue, don't forget to remove the comment in a tutorial code snippet that asks the reader to ignore a misleading warning about the location of a delegation path pattern (see 41d7bbde5dbeabb938b1cb90f4c7bc20bfdee725 of #775).

[joshuagl]

The specification states:

A TARGETPATH is a path to a file that is relative to a mirror's base URL of targets. To avoid surprising behavior when resolving paths, it is RECOMMENDED that a TARGETPATH uses the forward slash (/) as directory separator and does not start with a directory separator. The recommendation for TARGETPATH aligns with the "path-relative-URL string" definition in the WHATWG URL specification.

Therefore I think the latter expected behaviour is most consistent with the spec, i.e. we should change such that:

Targets.add_paths should probably assert that the passed paths are relative, to be consistent with the sibling the corresponding functionality of delegate().

[lukpueh]

To make sure we are on the same page, we are not talking about TARGETPATH but PATHPATTERN here. But the specification states something similar:

To avoid surprising behavior when matching targets with PATHPATTERN, it is
RECOMMENDED that PATHPATTERN uses the forward slash (/) as directory
separator and does not start with a directory separator, akin to
TARGETSPATH.

Also, just to double check, do others think raising an exception is reasonable? The spec only recommends relative paths, but IIRC we agreed (theupdateframework/specification#63, theupdateframework/specification#67) that an implementation can be more assertive than the spec in this regard. So I think leaving the strict behavior in delegate and adding it to add_paths is indeed reasonable.

[lukpueh]

I also just saw that I didn't suggest an expected behavior for the other issue, i.e. the warning/debug messages on not path.startswith(targets_directory).

I strongly suggest to remove them, because if a PATHPATTERN does indeed start with a targets directory, I think, it won't even match TARGETPATHs, as they explicitly leftstrip the targets directory, see e.g. add_target:
https://github.com/theupdateframework/tuf/blob/409eef1a4872a88b610e81b375f36b07a8e0f996/tuf/repository_tool.py#L1962

[lukpueh]

Fixed in #1008