feat(service-storage): confine the S3 adapter to a required key namespace (Part of #17571) - #17599

Part of #17571

Clause-②: yes

What this is

S3StorageAdapterOptions gains a required keyPrefix: string | null — the key namespace an adapter instance is confined to, or null for bucket-root keys — applied at every door and stripped on every way back, so a caller holding the adapter has no door through which it can reach an unprefixed key.

The ruled shape (cloud#1969 option B, quoted on the card by the seat that holds the ruling) is structural isolation:

③ 防 AI 犯错：B 的隔离是结构性的——前缀由内核工厂注入，AI 和租户代码都碰不到它，写错了也只能写进自己的前缀。

Everything below is this repo's call, which the ruling explicitly left open on three of the four points.

Premise re-verified

Measured on today's origin/main = 29d00cc539c2822be79d4d728779312094b35525 (the branch point), not on the abc4b83ce the domain:services seat measured:

• S3StorageAdapterOptions (packages/services/service-storage/src/s3-storage-adapter.ts:36-51 at that sha) carried no key-prefix option of any spelling — a case-insensitive prefix|namespace|root|scope grep over those lines returned nothing, with a positive control (the same word matches 4× elsewhere in the file, so both the grep and the file were live).
• The adapter passed the caller's key straight through at every door: Key: key at :169 :183 :193 :203 :217 :344 :370 :389 :413 :434 :452 and Prefix: prefix at :283.
• buildKey(scope, fileId, filename) (storage-routes.ts:962) composes ${scope}/${fileId}${ext} with scope defaulting to 'user' — no environment dimension.

The premise holds. The card's own line references were still byte-accurate at the branch point.

The shape chosen, and why — this is the part to scrutinise

Name: keyPrefix. Not because the card suggested it (it carries no authority and says so), but because it is what this repo's conventions produce. Prime Directive #3 puts TS config keys in camelCase; the word has to distinguish the thing from two neighbours that already exist in this package — basePath, which StorageTargetInput documents as "URL prefix the adapter signs against — not a storage location", and list(prefix, …)'s own caller-facing argument. A bare prefix would collide with both. keyPrefix names the S3 key namespace and matches the Key / Prefix vocabulary the file already speaks.

Enforcement: a required key whose type is string | null. The ruling allows a required option, a refusing constructor, or a non-empty default a host cannot silently inherit. This is the first, with the opt-out made explicit:

• Omission is a tsc error at the call site. That is this repo's own recorded remedy for a contract change on adapter code — from the list(prefix) retirement: "a storage adapter is CODE, never stack metadata … The enforced channel is tsc, and it reports at the call site." Measured here, on the two in-repo call sites: error TS2345 … Property 'keyPrefix' is missing in type '{ bucket: string; region: string; }' but required in type 'S3StorageAdapterOptions'.
• | null rather than ''. A single-tenant host must not be forced to invent a namespace and migrate its bucket — that pressure is exactly what would later reopen the option to an empty value, which is the gap. Writing keyPrefix: null makes opting out a deliberate, greppable act; a fleet can audit unprefixed buckets with one grep.
• '' and whitespace-only are refused at construction, loudly. That string is what an unset environment variable looks like after interpolation, and quietly reading it as bucket-root is the silent-degradation shape the ruling rejects. A leading / and any .. segment are refused too.
• A missing trailing / is appended, and this is load-bearing. S3 Prefix is a raw string match, so tenant_1 also matches tenant_10/…: without the normalisation, one namespace would enumerate its neighbour through the isolation mechanism itself. Pinned by a case, not a comment.

Keys are concatenated, never path-joined, so a caller key of ../elsewhere stays a literal key inside the namespace instead of escaping it. Pinned.

Two seams that had to move, or the option would have been a lie

Neither is in the card. Both are the same defect class as the card, inside the same package, and leaving either would have shipped an option that a deployment can lose without noticing — PD #10, never advertise a capability the runtime does not deliver.

1. StorageServicePlugin.buildAdapterFromValues rebuilds the adapter from the storage settings namespace, constructing a fresh S3StorageAdapterOptions from values and ignoring this.options.s3 entirely. A constructor-only prefix would therefore be dropped by the first settings save, returning a hosted deployment to a shared, unprefixed key space. The host's namespace is now carried onto every rebuilt adapter — and deliberately not read out of values: a boundary an administrator inside the deployment can set or clear is a preference, not a boundary. There is correspondingly no s3_key_prefix key in the storage settings manifest, and a test pins that one appearing there later still changes nothing.
2. resolveStorageTarget decides whether a swap strands existing bytes. Two prefixes in one bucket are two disjoint object sets, so the namespace belongs in location, not merely in the fingerprint — otherwise a prefix change swaps the adapter with no migration warning at all. env_7 and env_7/ normalise through the adapter's own function so the same namespace spelled two ways is not read as a move (the StorageServicePlugin warns "storage adapter swapped (LocalStorageAdapter → LocalStorageAdapter) … files may be unreachable" on every clean boot #4096 shape, one field along).

LocalStorageAdapterOptions — deliberately unchanged

The ruling left this to this repo, noting rootDir may already do the job. It does, structurally: LocalStorageAdapter.resolvePath() refuses any key containing .. and join(rootDir, key)s everything, and every door goes through it. That is a containment boundary of the same kind, already enforced. A second mechanism would be two ways to say one thing.

Verification

Declared narrowing — verification ran UNLOCKED. scripts/pm/os-verify-lock.sh
could not take the shared verify lock on this host: no usable flock. The shared
verify lock is declared Linux-only (flock is util-linux, and a stock macOS does
not ship it), so the command below was run directly, without the lock —
a declared narrowing, not a silent one. No serialization guarantee held for this
run, nor for any sibling agent in this container while it ran.

NODE_OPTIONS=--max-old-space-size=4096 pnpm --filter @objectstack/service-storage test

Ablation — these pins can fail

Run from the committed state, each leg proving the mutation reached disk before the run and proving the restore by blob hash:

On-disk proof per leg: deleted-text count 1 → 0 and injected-text count 0 → 1, anchored on the exact text. Restore proof per leg: git diff HEAD empty and git hash-object back to ab068bf93da82d65c5185042428b5b58e6075f72, the file's HEAD blob. The ablation script carried trap … EXIT INT TERM with absolute paths; no ablation artefact is left in the tree.

Landing

• Part of, not Fixes. The implementation half is complete and the card's acceptance criterion is driven end to end, but two things outlive this PR and both live on the card: the Clause-② contract review owed before it may be queued, and the downstream obligation the domain:services seat recorded — when this lands, the consuming card is filed in objectstack-ai/cloud by the cloud epic seat, which accepted that in writing. Leaving feat(service-storage): S3 适配器没有 key 前缀选项 —— 多租户共享 bucket 时对象存储层没有第二道隔离防线（cloud#1969 已裁形状的落点） #17571 open keeps both visible.
• Nothing the consumer must do is implicit. For the cloud seat's card: the kernel factory passes keyPrefix in the plugin's s3 options (new StorageServicePlugin({ adapter: 's3', s3: { bucket, region, keyPrefix: '<environment id>' } })). It must be non-empty — an interpolated empty env var throws at construction rather than writing to the bucket root — and it must not be plumbed through the storage settings namespace, which would hand the boundary to the tenant. Existing objects are not migrated into the new namespace; changing the prefix on a live environment strands what the old one held, and the swap says so.
• No governed surface in the diff (docs/adr/** · .claude/** · skills/** · AGENTS.md · CLAUDE.md).

Acceptance notes

• noted, not filed: packages/services/service-storage/src/s3-storage-adapter.ts carries an uploadChunk docblock that narrates a WeakMap design the code does not implement (it is a plain Map, _uploadKeys) and reads as in-progress reasoning rather than description. It is stale prose, not a defect, a contract violation, or a metadata-authoring trap, so it is out of the three filing classes. Successor: the next PR touching the multipart doors in this file — there is no other one queued.

Generated by Claude Code