[opieter-aws]

Description

The context-offloader S3Storage writes every object under the configured prefix in store(), but retrieve() never re-checked that invariant: it accepted a raw key verbatim, and for an s3:// URI it validated only the bucket before stripping s3://bucket/ and using the remainder as the key. The retrieve_offloaded_content tool (registered by default) passes a model-supplied reference straight through, so an agent that takes untrusted input could be steered to read any object elsewhere in the same bucket that the SDK's credentials can GetObject — a prefix-scope bypass. This is the confused-deputy gap the prefix parameter implies protection against, and the sibling FileStorage.retrieve() already enforces the equivalent directory boundary.

The fix adds the missing invariant in both SDKs: after resolving a reference to a key, reject it if a prefix is configured and the key falls outside it. Empty prefix keeps today's unrestricted behavior, and in-prefix keys and store()-returned URIs are unaffected, so this is backward-compatible for every reference store() produces. The newer unified strands.storage.S3Storage is not affected — it prepends the prefix rather than parsing it from caller input — so no change there.

Making retrieve() prefix-sensitive surfaced a pre-existing normalization mismatch: the TS constructor stripped only trailing slashes from prefix while Python's strip("/") strips both ends, so a leading-slash prefix (e.g. "/artifacts") produced different scopes and non-round-tripping store() keys across SDKs. The TS side now strips both ends to match Python, so the two SDKs normalize any prefix identically.

Documentation PR

No documentation changes. The user guide already documents S3Storage with bucket/prefix and the default retrieval tool; behavior now matches that scope. A follow-up could add a defense-in-depth note recommending prefix-scoped IAM or a dedicated bucket.

Type of Change

Bug fix

Testing

• Python: ran the context-offloader unit suite (hatch test tests/strands/vended_plugins/context_offloader/), which covers the S3 store/retrieve paths including the new raw-key and s3://-URI out-of-prefix rejections, and linted the changed files with hatch fmt --linter (ruff + mypy strict).

• TypeScript: ran the offloader storage and plugin unit suites (vitest, node project), which cover the mirrored rejections and the retrieval-tool path, plus eslint and tsc --project src/tsconfig.json on the changed files.

• Reproduced the original bypass against the real storage.py with an in-memory fake S3 client, then confirmed post-fix that both attack forms raise KeyError, the normal store()→retrieve() round trip still returns content, and cross-bucket references remain rejected.

• I ran hatch run prepare

Checklist

• I have read the CONTRIBUTING document
• I have reviewed and understand every line of code in this PR, including any generated by AI tools, and I can explain why it works
• My change is focused and reasonably small; I have split unrelated work into separate PRs
• I have added any necessary tests that prove my fix is effective or my feature works
• I have updated the documentation accordingly
• I have added an appropriate example to the documentation to outline the feature, or no new docs are needed
• My changes generate no new warnings
• Any dependent changes have been merged and published

---

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

[opieter-aws]

@strandly-the-agent review

[github-actions]

Assessment: Approve

Well-scoped, correct security fix that closes the prefix-scope bypass in the context-offloader S3Storage.retrieve() and restores cross-SDK prefix normalization parity. The prefix always carries a trailing /, so sibling-prefix collisions (artifactsX/ vs artifacts/) are correctly rejected, and since S3 keys are literal there's no ..-traversal escape hatch. Behavior is backward-compatible for every reference store() produces.

Review Categories

• Correctness/Security: Logic is identical and correctly placed in both SDKs; guarded on a truthy prefix so empty-prefix keeps existing unrestricted behavior. No parsing/traversal gaps.
• API surface: No signature changes; the behavior tightening is the intended security boundary and only affects out-of-prefix reads that store() never emits.
• Testing: Strong coverage (raw key, s3:// URI, sibling prefix, leading-slash normalization, round-trip). One minor parity gap — Python rejection tests don't assert get_object was never called (see inline comment).

Nicely mirrored across both SDKs and the regression tests clearly reference the originating report.

[strandly-the-agent]

REQUEST_CHANGES — the TypeScript change breaks persisted references created with leading-slash prefixes; preserve the existing key namespace while adding confinement.

✅ Reviewed exact head af3f2a86; CI rollup is green.
✅ Prefix-confinement attack matrices passed twice; focused Python/TS suites and TS build passed.
🔴 One compatibility blocker at strands-ts/src/vended-plugins/context-offloader/storage.ts:386, reproduced cold twice against head and merge-base.
🧭 This changes a public method’s accepted inputs/errors and requires the API-review label workflow; no API meeting is needed.
🧪 The nonblocking Python mock assertion is already covered by discussion_r3631980159; I haven’t duplicated it.

Blocker and verification ledger

🔴 Preserve the existing leading-slash namespace

At strands-ts/src/vended-plugins/context-offloader/storage.ts:386, prefix: "/artifacts" is now normalized to artifacts/. The previous implementation stored /artifacts/<id> and returned s3://b//artifacts/<id>; the parser at storage.ts:428-434 resolves that legacy URI to /artifacts/<id>, then the new check at storage.ts:436-438 rejects both the URI and raw legacy key before contacting S3. Please retain the historical trailing-only normalization while adding confinement, and handle any namespace migration separately rather than implicitly authorizing two namespaces.

• Head rejects legacy raw and URI references using /artifacts/<id> and writes new keys as artifacts/<id>.
• Merge-base reads both legacy forms and retains the leading slash on writes.
• Verified: Python S3 tests 13 passed; TS storage 32 passed; TS plugin 50 passed; TS build and git diff --check passed.
• Adversarial matrices covered 20 Python + 20 TypeScript cases, each twice. S3 clients preserve dot/slash key semantics; no confinement bypass reproduced.
• Advanced API/adversarial tiers were unavailable due retention-mode validation; both passes were retried on the default tier.
• Suppressed as pre-existing: TS broad error catch/tool-result status. TSDoc/security guidance would be useful follow-up work, but is not another blocker here.

[strandly-the-agent]

Inline compatibility finding attached to the existing changes-requested review.

[opieter-aws]

@strandly-the-agent re-review

[strandly-the-agent]

APPROVE — the TypeScript namespace compatibility blocker is resolved at 40c30078; no code blockers remain.

✅ Historical /artifacts/... writes are preserved and legacy s3://b//artifacts/foo resolves to /artifacts/foo.
✅ Prefix confinement remains unchanged; the prior blocker thread is resolved/outdated.
✅ Local: TS storage 32/32, build, full lint, full format check, and diff-check passed; clean tree.
✅ Exact-head CI: all Python/TS unit matrices, lint/quality/audit/package, Docs, and CI Gate passed.
⚠️ I could not rerun Python locally because pytest is unavailable; CI covered all 11 Python jobs.
🧭 Before merge, complete the required api/needs-review → api/review-complete label workflow; both labels are still absent.

Re-review scope

Reviewed only af3f2a86..40c30078. The delta restores the released trailing-slash-only normalization and adds a regression that independently reads the old double-slash URI and asserts the exact S3 key. It does not broaden reads or introduce a dual namespace.

The existing Python get_object.assert_not_called() suggestion remains in discussion_r3631980159; I did not duplicate or escalate that previously nonblocking test-hardening request.

Two integration jobs were still running during the detailed check; the final exact-head rollup is now SUCCESS.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!