Repository navigation
Commit dd986d8
fix(service-storage)!: downloading a file with no attachments scope and no field owner requires a signed-in caller (#22439)
Fixes #22431
Clause-②: no (narrowing)
Executes ruling
[`6074960686`](#22146 (comment))
item 3 (#22146), as triage routed it: a storage download of a file with
no scope and no field owner requires a signed-in caller, and a file
declared `acl: 'public_read'` stays anonymous (ADR-0104). Function level
only, as the card requires.
## What changes
- **`packages/services/service-storage/src/storage-routes.ts`,
`authorizeDownload`** (the one gate both download routes call). A file
with neither an attachments scope nor a field owner (an upload no record
has claimed) now needs a session from the existing `resolveSession`; a
caller with none is refused `401 AUTH_REQUIRED`, the pair the upload
gate and the attachments gate already answer. No new code, no spec
change. `acl: 'public_read'` is checked first and stays anonymous for
every class. Attachments-scope and field-owned files keep their
`authorizeFileRead` verdicts, untouched. A resolver that throws, or a
session with no user, fails closed.
- **Bare kernel unchanged.** With no `resolveSession` wired, these
downloads stay open as before, and the module now says so once (the
existing one-time notice names only the upload routes).
- **Docblocks made true.** The `resolveSession` docblock no longer calls
download gating "a tracked follow-up", and the `authorizeFileRead`
docblock no longer names an organization logo as anonymous.
- **`content/docs/permissions/attachments-access.mdx`** said that
avatars, image-field thumbnails and organization logos keep an anonymous
capability URL. This change makes the avatar and logo half false, and
the image-field half has been false since field-owned files were gated.
The paragraph and two table rows now state the three classes. This file
is outside the dispatch's file fence; see Acceptance notes.
- **One changeset** for `@objectstack/service-storage`, `minor`, with
the BREAKING banner, the remedy (sign in, or mark the file
`public_read`) and an ADR-0087 disposition `not-required
(no-migration-prescription)`.
## Measured before building (the card's stop condition)
Measured on `origin/main` `b9222dc701` on a booted showcase
(`objectstack dev --fresh`, `single` posture). The readings stay in the
seat's container. Classes only here:
- **Reproduction.** An anonymous download of an unclaimed upload was
served at both download routes, bytes included. Controls: an
attachments-scope file and a field-owned file were refused `401
AUTH_REQUIRED` to the same caller, and an anonymous upload was refused
`401 AUTH_REQUIRED`.
- **Producers of unclaimed files.** In this repository only the two
upload routes create a `sys_file` row
(`StorageMetadataStore.createFile`). The copy-on-claim copy is claimed
by construction. No seed, branding, theme or import path creates one,
and the showcase seeds none. The rows that stay unclaimed come from what
clients do with an upload: the console writes an uploaded avatar into
the user's `image` URL field and an uploaded organization logo into the
organization's `logo` URL field. Neither is a file-class field, so
neither is ever claimed. A picked file stays unclaimed until its record
is saved, an abandoned upload stays unclaimed, and so does a file whose
owner released it. Nothing in the repository produces a `public_read`
file.
- **Readers of such a file, and when they render.** The console renders
avatars and organization logos (header, user menu, profile, members,
organizations, organization settings) and pre-save upload previews.
Every one of them is behind sign-in. The surfaces that render before
sign-in read nothing in this class. The sign-in pages draw their logo
from operator configuration, never from an upload. The invitation page
draws no logo or avatar. A public form cannot upload anonymously. The
share page renders no stored file. No in-repo email template renders an
avatar or a logo.
- **How a signed-in browser reaches the routes.** The console signs in
through the better-auth client, which sends credentials by default, so
the browser holds the HttpOnly, SameSite=Lax session cookie the sign-in
sets, beside the bearer token. In a real Chromium session signed in that
way, image tags pointing at the routes loaded the already-gated classes
(attachments-scope and field-owned). A session with no cookie (a
bearer-only client) failed them. The routes' `resolveSession` reads the
cookie the same as a bearer header.
- **Verdict:** no surface the repository ships stops rendering, for a
signed-out or a signed-in viewer. Built.
## Tests
- **Unit, `storage-routes.test.ts`:** a new block for the unclaimed
class. It covers the refusal at both doors (code, status, envelope, no
URL minted, authorizer not consulted), parity with the upload gate's
anonymous answer, fail-closed on a throwing resolver and on a user-less
session, and a signed-in caller served as before (302, and the presigned
TTL read back out of the minted URL). It also covers `public_read`
anonymous without a session read, the parent-governed verdicts unchanged
and the resolver not consulted, a missing file 404 before the session is
asked, and a bare kernel open with one notice. Against the unfixed code:
**4 failed / 40 passed** (the refusal pins and the notice red; the
controls green). With the fix: green.
- **Conformance, `error-envelope.conformance.test.ts`:** the new refusal
joins the driven error branches.
- **Dogfood, `storage-unclaimed-download.dogfood.test.ts`** (one file,
booted showcase with the storage plugin): the anonymous refusal at both
doors, a bearer caller served, a **cookie-only** caller served (the
transport an image tag uses), `public_read` anonymous and back, and
controls (anonymous upload, attachments-scope download). Against main's
`dist`: **2 failed / 4 passed**. With the fix: **6 / 6**.
- **Ablation** (committed fix first, mutation through
`scripts/ablation-replace.mjs`, restore trap held): deleting the
session-gate call in `authorizeDownload` landed (anchor 1 → 0, blob
changed). After a rebuild, `ablation-dist-preflight --absent` showed the
call gone from all 4 built files. Unit + conformance went **5 failed /
57 passed**, and the dogfood file went **2 failed / 4 passed**. Restore:
blob equal to HEAD, `git diff HEAD` empty, `git status --porcelain`
empty. The rebuild preflight showed the call present in `dist/index.js`
and `dist/index.cjs`, then **62 / 62** and **6 / 6**. (The mutated
build's DTS step failed on the now-unused helper, `TS6133`. ESM and CJS
were rebuilt, and those are what the suites import.)
- **Full `@objectstack/service-storage` suite:** 46 files, **782
passed**; `typecheck` exit 0 (with `check:test-typecheck`).
- **Every dogfood file that uploads, downloads or touches `sys_file`**
(18 files) at `42fe8d498c`: **175 passed, 1 skipped** (the pre-existing
`skipIf(!organizationsAvailable)` block), exit 0. `@objectstack/dogfood`
`typecheck` exit 0, with the new file in the program.
## Gates (at `42fe8d498c`, after merging `origin/main` `05c7c3fa3b`)
- `node scripts/pm/dispatch-gates.mjs --commands --repo
objectstack-ai/objectstack` (no paths) derived **98** commands from the
six changed paths. All 98 were run, and all exited 0. `--ran`
reconciliation: `98 derived famil(ies) accounted for — 98 run, 0
NOT-MEASURED (a DERIVED zero — all 98 recorded an exit code and none of
them is 3)`.
- `pnpm check:error-status-conformance` (also by hand, per the
dispatch): `✓ every derivable runtime status is documented, and every
documented status is reachable.`
- Changeset, `check-changeset-no-major` with this body as the event: `✓
This diff introduces no major bump.` and `✓ LEVEL AXIS: this PR declares
clause-② no (narrowing), and no package whose packages/**/src/** it
moves is graded patch.` `check-adr-0087-registration`: `✓ 1
declared-breaking changeset(s), each carrying an ADR-0087 disposition`
(`BREAKING+bang+clause-②-narrowing`, `not-required
(no-migration-prescription)`).
- `check:nul-bytes`: `OK (... no raw ASCII control bytes)`.
`check:route-envelope`, `check:doc-authoring`,
`check:cross-package-test-inputs` and `check:test-source-alias` all
exited 0.
- ESLint, narrowed to the four changed TypeScript files and proven: each
is in the config's population (`--print-config` resolves for every one),
the JSON output counts 4 files with 0 errors and 0 warnings, and
`eslint.config.mjs` never enables type-aware linting, so this diff
cannot move a verdict on an untouched file. The repo-wide `pnpm lint` is
CI's run.
## Acceptance notes
- **File fence.** The dispatch fenced `storage-routes.ts` and its tests,
one dogfood file and one changeset. This PR also corrects
`content/docs/permissions/attachments-access.mdx`, because the change
makes its statement false. The standing dev rules require a published
statement this change falsifies to be fixed in the same change. The
conflict is named here rather than settled silently. Drop the commit if
the seat rules otherwise.
- **TTL kept.** A signed-in download of an unclaimed file still mints
its URL with the presigned TTL, not the short gated-download TTL.
"Served as today" was the instruction.
- **Order kept.** The routes look the file up before judging the caller,
so a missing file answers 404 before the session is asked, exactly as
the parent-governed classes already do.
- **Where a signed-in page would still go dark** (not a shipped shape,
noted): a console served cross-site from its API (a SameSite=Lax cookie
is not sent on a cross-site image request), and a session restored from
a bearer token alone. Field-owned images already fail the same way there
today.
- **Boot wording elsewhere.** `mountStorageRoutes`' unbound-gates
warning and the `StorageRoutesMountReport.sessionResolver` docstring (in
`storage-service-plugin.ts`, which PR #22396 edits and this PR does not
touch) still describe the resolver as gating uploads. Both are still
true and now incomplete. Carrier: none.
- `.changeset` grading: `check-changeset-no-major` and
`check-adr-0087-registration` verdicts are under Gates.
---
_Generated by [Claude
Code](https://claude.ai/code/session_01WkL6Eijt432S1Y7ekb6ovQ)_
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent bf492c8 commit dd986d8
6 files changed
Lines changed: 452 additions & 27 deletions
File tree
- .changeset
- content/docs/permissions
- packages
- qa/dogfood/test
- services/service-storage/src
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
111 | 111 | | |
112 | 112 | | |
113 | 113 | | |
114 | | - | |
| 114 | + | |
115 | 115 | | |
116 | 116 | | |
117 | 117 | | |
| |||
132 | 132 | | |
133 | 133 | | |
134 | 134 | | |
135 | | - | |
136 | | - | |
137 | | - | |
138 | | - | |
| 135 | + | |
| 136 | + | |
| 137 | + | |
| 138 | + | |
| 139 | + | |
| 140 | + | |
| 141 | + | |
| 142 | + | |
| 143 | + | |
139 | 144 | | |
140 | 145 | | |
141 | 146 | | |
| |||
174 | 179 | | |
175 | 180 | | |
176 | 181 | | |
177 | | - | |
| 182 | + | |
178 | 183 | | |
179 | 184 | | |
180 | 185 | | |
| |||
Lines changed: 180 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
| 113 | + | |
| 114 | + | |
| 115 | + | |
| 116 | + | |
| 117 | + | |
| 118 | + | |
| 119 | + | |
| 120 | + | |
| 121 | + | |
| 122 | + | |
| 123 | + | |
| 124 | + | |
| 125 | + | |
| 126 | + | |
| 127 | + | |
| 128 | + | |
| 129 | + | |
| 130 | + | |
| 131 | + | |
| 132 | + | |
| 133 | + | |
| 134 | + | |
| 135 | + | |
| 136 | + | |
| 137 | + | |
| 138 | + | |
| 139 | + | |
| 140 | + | |
| 141 | + | |
| 142 | + | |
| 143 | + | |
| 144 | + | |
| 145 | + | |
| 146 | + | |
| 147 | + | |
| 148 | + | |
| 149 | + | |
| 150 | + | |
| 151 | + | |
| 152 | + | |
| 153 | + | |
| 154 | + | |
| 155 | + | |
| 156 | + | |
| 157 | + | |
| 158 | + | |
| 159 | + | |
| 160 | + | |
| 161 | + | |
| 162 | + | |
| 163 | + | |
| 164 | + | |
| 165 | + | |
| 166 | + | |
| 167 | + | |
| 168 | + | |
| 169 | + | |
| 170 | + | |
| 171 | + | |
| 172 | + | |
| 173 | + | |
| 174 | + | |
| 175 | + | |
| 176 | + | |
| 177 | + | |
| 178 | + | |
| 179 | + | |
| 180 | + | |
Lines changed: 13 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
314 | 314 | | |
315 | 315 | | |
316 | 316 | | |
| 317 | + | |
| 318 | + | |
| 319 | + | |
| 320 | + | |
| 321 | + | |
| 322 | + | |
| 323 | + | |
| 324 | + | |
| 325 | + | |
| 326 | + | |
| 327 | + | |
| 328 | + | |
| 329 | + | |
317 | 330 | | |
318 | 331 | | |
319 | 332 | | |
| |||
0 commit comments