Repository navigation
fix(store): S3 upload policies are scoped to a directory and signed through a private _post_policy - #93
Merged
Conversation
…icy, as GCS does issue_upload_policy built its policy by calling the public signed_post. It now calls a private _post_policy, which signed_post also calls, so signed_post behaves as before. A store layered on S3ObjectStore can then issue upload policies, and later make signed_post a shim over issue_upload_policy, without the two calling each other. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
signed_post passed its prefix through as given, so a prefix without a trailing slash admitted sibling keys (root also matched root-evil/). _post_policy now appends the slash, as the GCS store's does, and issue_upload_policy no longer needs to. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
S3ObjectStore.issue_upload_policybuilt its POST policy by calling the store's publicsigned_post. It now calls a private_post_policy(url_prefix, expires_in, max_bytes), which holds the existinggenerate_presigned_postbody. This is the arrangementGCSObjectStorealready has.signed_postcalls_post_policytoo._post_policyalso scopes the policy to a directory, as GCS's does: it appends/to the prefix if it's missing. Before,signed_post("s3://bucket/root")signedstarts-with $key root, which also admittedroot-evil/….issue_upload_policyalready appended the slash, and it no longer needs to./: the namespace grant inobject_transfer, and the routing store, which forwards. So their policies are unchanged.rootmeaning a filename prefix now getsroot/. Nothing in this repo does that, and the GCS store has never allowed it.This is the first step toward retiring
signed_postin favour of the typedissue_upload_policy. A store layered onS3ObjectStorecan now issue upload policies through_post_policy. Later,signed_postcan become a deprecated shim overissue_upload_policywithout the two calling each other.Testing
S3ObjectStorewhosesigned_postraises still issues an upload policy, with the same key template, size condition and grant field names. This fails onmain.signed_postprefix without a trailing slash gets theroot/key template andstarts-withcondition._post_policy, both prefix tests fail.signed_postunit tests pass unchanged. They pin the exactgenerate_presigned_postcall.maineither. It would need moto's server mode, which isn't a dev dependency today.tst/unit6023 passed (13 skipped), protocol 330 passed.🤖 Generated with Claude Code
The PR appears safe to merge; no blocking issue remains.
What we checked:
_post_policysigns the prefix with a trailing slash, so the sibling key does not match.Summary
S3’s typed and raw upload-policy methods now use one private signing builder, so the typed path no longer depends on
signed_post. A bare prefix likecaptures/oneis also treated as thecaptures/one/folder.Reviews (2) · Last reviewed commit: "Scope S3 upload policies to a directory ..." · Reviewed by Greptile