Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tests for rolename/targetpath encoding #125

Open
jku opened this issue Aug 12, 2024 · 4 comments
Open

tests for rolename/targetpath encoding #125

jku opened this issue Aug 12, 2024 · 4 comments

Comments

@jku
Copy link
Member

jku commented Aug 12, 2024

We want to test that

  • clients use correctly URL encoded rolenames and targetpaths in the request URLs. My read on spec is:
    • rolenames should be URL encoded so that also "/" is encoded: rolename should not include "sub-directories"
    • targetpath should be URL encoded so that "/" is not encoded: targetpath may include sub-directories
  • clients do not use unsafe encoding when handling filepaths:
    • we don't want to define how to encode filepaths because the spec does not
    • we can test that when we download delegated metadata or artifacts
      • the metadata download or artifact download succeeds when we expect it to
      • a file appears in clients TARGET_DIR or METADATA_DIR (and was not e.g. written to ../)

I have a crude initial test, will upload after #115

@jku
Copy link
Member Author

jku commented Aug 12, 2024

this is easier with #126

@jku
Copy link
Member Author

jku commented Aug 13, 2024

part of this is in #138

@jku
Copy link
Member Author

jku commented Aug 16, 2024

Current state:

  • rolenames have a test now (it has a handful of unusual rolenames, most importantly a path traversal attempt)
  • there is no test yet for targetpath encoding:
    • I think it makes sense to add another test in test_quoting_issues that tries to download a bunch of unusual targetpaths: my take is that "?", "#" and such should get encoded but "/" should not
    • the case I'm not sure about is ".." as target path component:
      • rfc3986 seems to say client must canonicalize urls (See remove_dot_segments) and http libs usually do
      • this means "../artifact" could end up pointing to a file "outside the artifact store" -- repositories likely won't want to do this but I don't think this is the clients problem?
      • this also means targetpaths "a/../a" and "a" must be the same artifact -- I don't think this is the clients problem either?
      • I believe client can just follow rfc and allow ".." in the targetpath, but then we want to test for path traversal attempt when storing "../artifact"
      • question is, is it ok for a client to not allow ".." as targetpath component?

@jku
Copy link
Member Author

jku commented Aug 20, 2024

a very quick test shows clients down't handle targetpaths that need encoding very well -- this likely needs to start with either some spec discussion or clients documenting what they intend to do WRT targetpath encoding.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant