You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Commit ee8751d
Browse filesBrowse the repository at this point in the historyBrowse files
feat(storage)!: retire the storage scope public from StorageScopeSchema and refuse it at the upload doors (#22443) (#22469)
Part of #22443
Clause-②: no
Retires the storage scope `public`, which never made a file publicly
readable, and keeps `acl: 'public_read'` (ADR-0104) the one opt-in for
anonymous download: the triage answer on the card ("trim"). This PR
lands the half that every remaining option shares. **The other half, the
`sys_file` object's `public` scope option and rows already stored with
it, is a decision rather than an edit, and stays on the card** (see
"Left for a decision"). #22443 remains open.
## What lands
- **`StorageScopeSchema`** (`@objectstack/spec`,
`system/object-storage.zod.ts`). `public` leaves the enum through
`enumWithRetiredValues`. `tsc` refuses it, and a parse, alone or as
`ObjectStorageConfig.scope`, answers with a prescription that names
`acl: 'public_read'` instead of zod's generic enum message.
- **The two upload-starting handlers in `registerStorageRoutes`**
(presigned and chunked, `@objectstack/service-storage`). A request
naming scope `public` is answered `400 INVALID_REQUEST` before any file
row, session row, upload URL or backend multipart exists.
`INVALID_REQUEST` is an existing code, registered under this package in
the ADR-0112 ledger and already used by these handlers for a request
they cannot take. The message names the remedy and where it lives: on
the stored file record, because the upload request carries no `acl` and
every upload is stored `acl: 'private'`. Every other scope, and an
omitted one, is taken exactly as before.
- **The upload request contract** (`api/storage.zod.ts`). The `scope`
description on `GetPresignedUrlRequestSchema` and
`InitiateChunkedUploadRequestSchema` no longer offers `public` as an
example, and says what the scope is and is not. The shape is unchanged
(`z.string()`).
- **D3 entry `storage-scope-public-retired`** (step 18), and one
changeset: `minor` on both packages, a declared narrowing, disposition
`registered storage-scope-public-retired`. No D2 conversion (H2 below).
`spec-changes.json` and the upgrade guide do not move, because step 18
is not projected yet.
- The two reference pages, regenerated.
The download handlers are not touched, so their verdict from PR #22439
stands. A pin here asserts it on an already-stored `public`-scoped row
with the default acl: anonymous `401 AUTH_REQUIRED`, signed-in `200`.
## Measurements: the four dispatch hypotheses
All readings at origin/main da159f7 unless stated.
- **H1, where the vocabulary is declared or checked. Holds.** Five
sites, and the spec enum is not the upload vocabulary:
1. `StorageScopeSchema`. Consumed only by
`ObjectStorageConfigSchema.scope`, and `ObjectStorageConfigSchema` has
no runtime consumer: no parse of it outside `packages/spec`.
2. `GetPresignedUrlRequestSchema.scope` and
`InitiateChunkedUploadRequestSchema.scope`. Open `z.string()`, and the
first one's description taught `public`. No server handler parses a
request with them.
3. The `sys_file` object's `scope` select (`user`, `tenant`, `public`,
`private`, `temp`, `attachments`). This is the stored vocabulary and,
before this PR, the only enforcement: the engine refuses an insert
outside it (`invalid_option`).
4. The two upload-starting handlers. No check of their own before this
PR; they pass the caller's value to site 3.
5. `sys_upload_session.scope`. A free `Field.text`, copied from the
request; no vocabulary.
- **H2, an authored spelling and stored rows. No authored spelling, so
no conversion.** `scope: 'public'` has one hit in the tree, a spec
request-schema test fixture (re-pointed to `tenant` here), against 29
hits for `scope: 'attachments'`, the control. At the `.objectui-sha` pin
f0268ad and at objectui main 2063f7a: zero hits; the console passes
no scope and the record attachments panel passes `attachments` (the
control). No metadata type carries either schema, and stored `sys_file`
rows are data, which the ADR-0087 chain does not reach. Measured on a
real `ObjectQL` over sqlite, with the real `SystemFile` minus its
`public` option and a legacy `public` row written through the driver:
- `findOne` and `find` pass the row through, and an update that does not
name `scope` succeeds. Reading is unaffected.
- **The field-reference copy path (`copyOwnedFile` in
`file-reference-lifecycle.ts`) re-inserts the source row's scope, and
that insert is refused `VALIDATION_FAILED`, `invalid_option` on
`scope`.** An update that writes `scope: 'public'` is refused the same
way.
- So retiring the option without touching stored rows would break
writing an already-owned legacy `public` file id into a second record
field. Per the dispatch, that is a fork for the maintainer, not a choice
for this PR.
- **H3, the error code. Holds.** `INVALID_REQUEST` is reused; no new
code, so `Clause-②` stays `no`.
- **H4, `storage-route-ledger.ts`. Holds.** Its `'public'` is a route
disposition (an unauthenticated browser route), not a storage scope.
Untouched.
## Left for a decision
The `sys_file` `scope` select keeps its `public` option in this PR. The
options, as measured above:
- **A. Keep it as a stored-only value.** After this PR no upload can
write it; legacy rows, and copies of them, keep it. No data migration.
What remains: the stored vocabulary keeps a member labelled "Public"
that grants nothing.
- **B. Retire the option and rewrite stored `public` rows to `user`,
once.** A storage-service backfill, the shape the `sys_file`
organization backfill already takes. Declared and enforced agree
everywhere afterwards. It costs a data rewrite on every deployment, a
no-op where there are no such rows. Keys keep their `public/` prefix and
no bytes move.
- **C. Retire the option with no rewrite.** Measured to break the copy
path above. Not viable.
- **D. Retire the option and stop the copy path inheriting the source
scope.** No data rewrite, but it changes the copy rule for every copy,
attachments-scope sources included, which is a design question of its
own.
The recommendation, reasoned on the four axes, is in the report on the
card.
## Tests
All at `149062a9db` (this branch merged with origin/main 3ca71b6):
- `@objectstack/service-storage`: `vitest run`, 47 files and 789 tests
passed. `typecheck` exit 0.
- `@objectstack/spec`: `vitest run --project local`, 629 files and 18783
tests passed (1 todo). `typecheck` exit 0, which includes the test
layer, so the `@ts-expect-error` on the retired member compiles as a
real check. Repo-project files that read these surfaces
(`retired-key-migrate-sentence`, `step18-rationale-merge`,
`conversions-major18-merge`, `error-catalog-docs`): 4 files and 41 tests
passed. The rest of `--project repo` is declared to CI.
- Gates: `dispatch-gates --commands` derived 119 families on this head,
plus `check:generated` from the dispatch list. All 119 ran with exit 0,
and `dispatch-gates --ran` reports 119 run, 0 not measured, 0 unrun.
- Lint, narrowed: eslint on the 8 touched `.ts` files, 8 files in the
JSON output, 0 errors, 0 warnings. The population is
`eslint.config.mjs`'s own TS glob, 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.
- Reverse verification, each a committed fix mutated through
`scripts/ablation-replace.mjs` and restored to `git diff HEAD` empty:
- Door gate disabled: the refusal pin goes red (1 failed, 3 passed); the
control, body-acl and legacy-download pins stay green.
- `public` re-admitted to the enum: both prescription pins go red (2
failed, 64 passed), and the unknown-value control stays green. A first
run of this ablation used the dummy retired key `publik`, which is the
control's own value, so it reddened the control as well. It was rerun
with an unrelated key; the reading above is the rerun.
## Acceptance notes
- **Reported to the seat to file (reproducible):** an upload naming any
scope outside the `sys_file` select, a free key prefix for instance, is
answered `500 INTERNAL` by the upload-starting handlers. The engine's
`invalid_option` is relayed as an internal error whose message tells an
operator to restore the data engine. Measured with the presigned handler
over a real engine. objectui's upload adapter documents `scope` as a
free logical key prefix, with examples that are not select options. Not
fixed here: a different defect, with its own shape to decide.
- `ObjectStorageConfigSchema`, and with it the rest of
`StorageScopeSchema`, has no runtime consumer. Noted, not filed (an
unconsumed declaration with no pull).
- `acl: 'public_read'` has no setter at either upload handler (both
store `private`), and ADR-0104's "the field declares a public posture"
half has no spec key. So the remedy this PR names can be applied only by
a write to the stored `sys_file` row. Noted for the guest-model family
(#22146).
- No liveness ledger row moves: the ledger walks metadata types, and
none carries `StorageScope`.
---
_Generated by [Claude
Code](https://claude.ai/code/session_01VZqqwTj2wsihZEbfT6yyYN)_
---------
Co-authored-by: Claude <noreply@anthropic.com>
feat(storage)!: the storage scope `public` is retired — no scope ever made a file publicly readable, and `acl: 'public_read'` stays the one opt-in for anonymous download (#22443)
**BREAKING** — an accept-set narrowing on two published surfaces, shipped as `minor` under the launch-window convention for accept-set narrowings.
13
+
14
+
`StorageScopeSchema` described `public` as "publicly accessible static assets", and the upload doors stored a caller's `scope: 'public'` on the new file. Neither ever made a file public. The download doors judge a file by its `acl`, the `attachments` scope and field ownership alone, so a file uploaded with scope `public` and the default acl was stored private, needed a signed-in caller, and nobody was told (ADR-0049 enforce-or-remove). ADR-0104 makes `acl: 'public_read'` the one opt-in for anonymous download, so the scope is retired, not enforced: enforcing it would have let any uploader make a file anonymous at upload.
15
+
16
+
### What now refuses `public`
17
+
18
+
-**The presigned upload door and the chunked upload door** (`@objectstack/service-storage`) answer an upload naming `scope: 'public'` with `400 INVALID_REQUEST`, before any file row, session row, upload URL or backend upload exists. The message names the remedy. Every other scope, and an omitted one, is taken exactly as before.
19
+
-**`StorageScopeSchema`** (`@objectstack/spec`): `public` left the enum. Writing it fails `tsc`, and parsing it, on its own or as `ObjectStorageConfig.scope`, fails with the prescription instead of zod's generic enum message.
20
+
21
+
### FROM → TO
22
+
23
+
| before | what to write instead |
24
+
| --- | --- |
25
+
| an upload with `scope: 'public'`| another scope, or no `scope` for the default `user`; then set `acl: 'public_read'` on the stored file record of each file that must be readable before sign-in |
26
+
|`ObjectStorageConfig` with `scope: 'public'`| another scope, or no `scope` for the default `global`|
27
+
28
+
**The one-line fix: stop naming scope `public`, and mark each file that must render before sign-in `acl: 'public_read'` on its stored file record.** The upload request carries no `acl`: every upload is stored `acl: 'private'`, so adding an `acl` to the upload request changes nothing.
29
+
30
+
**Files already stored with scope `public`** are not touched. They download exactly as they did: a signed-in caller gets them, an anonymous one gets `401`, unless the file is `acl: 'public_read'`.
31
+
32
+
### The retirement kit
33
+
34
+
-**Schema.**`StorageScopeSchema` retires the member with `enumWithRetiredValues`. No D2 conversion: no metadata type carries this schema or the upload request, so `os migrate meta` has no authored source to rewrite.
35
+
-**D3 entry `storage-scope-public-retired`** carries the judgement no rewrite can make: whether a file that was uploaded as `public` must really be readable before sign-in.
36
+
-**Upload request contract.** The `scope` description on the presigned and chunked request schemas no longer offers `public` as an example, and says what the scope is and is not.
37
+
-**Liveness.** No ledger row: the liveness ledger walks metadata types, and no metadata type carries `StorageScope`.
38
+
39
+
**Measured producers: none.** At origin/main da159f74e6, nothing in `packages/`, `examples/`, `apps/`, `skills/` or `content/docs/` uploads with scope `public` or declares an `ObjectStorageConfig` with it. The one hit was a `@objectstack/spec` request-schema test fixture, changed to `tenant` here. The same search finds 29 `scope: 'attachments'` writes, which is its control. At the `.objectui-sha` pin f0268ad784 and at objectui main 2063f7a, the upload adapter forwards a caller's scope and no caller names `public`: the console passes none, and the record attachments panel passes `attachments` (the control). Deployed callers and stored rows NOT MEASURED.
|**scope**|`string`| optional (default: `"user"`) |Storage scope the new file is filed under (default user; attachments for the record attachments surface). Not an access setting: the upload doors refuse public, and a file is served without sign-in only when its stored file record carries acl 'public_read' (ADR-0104). The upload request carries no acl; every upload is stored private.|
228
228
|**bucket**|`string`| optional | Specific bucket override (admin only) |
229
229
230
230
@@ -240,7 +240,7 @@ const result = CompleteChunkedUploadRequestSchema.parse(data);
240
240
|**mimeType**|`string`| ✅ | File MIME type |
241
241
|**totalSize**|`integer`| ✅ | Total file size in bytes |
242
242
|**chunkSize**|`integer`| optional (default: `5242880`) | Size of each chunk in bytes (minimum 5MB per S3 spec) |
|**scope**|`string`| optional (default: `"user"`) |Storage scope the new file is filed under (default user; attachments for the record attachments surface). Not an access setting: the upload doors refuse public, and a file is served without sign-in only when its stored file record carries acl 'public_read' (ADR-0104). The upload request carries no acl; every upload is stored private.|
244
244
|**bucket**|`string`| optional | Specific bucket override (admin only) |
Copy file name to clipboardExpand all lines: docs/protocol-upgrade-guide.md
+3Lines changed: 3 additions & 0 deletions
Display the source diff
Display the rich diff
Original file line number
Diff line number
Diff line change
@@ -1299,6 +1299,9 @@ This is a RUNTIME registration API, not stored metadata, so — like `hook-regis
1299
1299
- **`startup-orchestrator-retired`** — `the startup-ORCHESTRATION surface of kernel/startup-orchestrator.zod.ts and contracts/startup-orchestrator.ts — 3 emitted defs and 8 exported names: StartupOptionsSchema / StartupOptions / StartupOptionsParsed, HealthStatusSchema / HealthStatus, StartupOrchestrationResultSchema / StartupOrchestrationResult, and the IStartupOrchestrator interface (orchestrateStartup / rollback / checkHealth / startWithTimeout). The startup RESULT survives, re-declared: PluginStartupResultSchema and PluginStartupResult stay on both entries` → (removed — there is no declarative replacement, because nothing ever implemented the interface or parsed the schemas. Plugin startup is the kernel own boot loop: ObjectKernel.start() calls startPluginWithTimeout() per plugin, which races that plugin start() against PluginMetadata.startupTimeout and, when KernelConfig.rollbackOnFailure is set, destroys the already-started plugins and rethrows the original error as the new error cause. So: instead of StartupOptions.timeoutMs declare startupTimeout on the plugin; instead of StartupOptions.rollbackOnFailure set rollbackOnFailure on the kernel config; instead of StartupOrchestrationResult.results read the per-plugin durations through ObjectKernel.getPluginStartupDurations(). StartupOptions.healthCheck and HealthStatus have NO replacement at all — no startup probe system exists, and one returns only through the enforce route of ADR-0049 with a new ADR, the probe first and the vocabulary second. StartupOptions.parallel and StartupOptions.context likewise: the kernel starts plugins sequentially and passes its own PluginContext)
1300
1300
- Why not automatic: ADR-0049 enforce-or-remove; maintainer ruling 2026-09-06, option 3: keep a startup-result contract re-declared as the shape the kernel ships, and retire the rest. The module declared an orchestration design that never landed, and the spec and the kernel had already drifted into disagreement about the one shape that did: PluginStartupResultSchema described a plugin object, a required durationMs and a health member, while @objectstack/core shipped pluginName, an optional durationMs and timedOut. The ruling keeps a startup-result contract that describes what the kernel actually produces, and retires the rest. Re-measured on this card: zero implementers and zero consumers of the four retired surfaces in this repository and in the pinned objectui checkout, with lit same-corpus controls (defineStack, ManifestSchema); every remaining reference was a generated artifact or a released CHANGELOG.md. healthCheck and HealthStatus are the sharpest of the four: they name a per-plugin health probe the runtime has never had, the shape of the plugin sandboxing / integrity / approval config that was never wired to anything, which an AI author (ADR-0033) reads as proof the capability exists. With no authored document carrying any of the three defs there is no seam for a D2 conversion and no author to tombstone for: route 3, the shape of the dynamic plugin-loading family's removal and the advanced plugin-lifecycle config's retirement — RETIRED_DEFS_BY_MAJOR plus this entry ARE the declaration. The two keys of the SURVIVING result schema that leave (plugin, health) are tombstoned instead, and registered in RETIRED_KEYS_BY_MAJOR, because that def keeps emitting and its type is imported by @objectstack/core. A third key arrives on the spec surface only to leave it: core deprecated startTime alias, which held the same elapsed milliseconds as durationMs under a name that promises an instant. The re-declaration had to either mirror it or tombstone it, and mirroring is refused by check:duration-unit-keys (ruling B on duration-shaped number keys: the unit lives in the key name) since it is an elapsed number whose key name carries no unit and matches neither of that rule two schema-declared exemptions. So the L1 window closes here and the kernel stops populating it in the same change.
1301
1301
- Done when: No code imports any of the 8 retired names from @objectstack/spec, @objectstack/spec/kernel or @objectstack/spec/contracts — every one is TS2305 after upgrade, pinned by resolved symbol identity in kernel/startup-orchestrator-retirement.test.ts. No metadata document needs editing: none of the three defs was reachable from a metadata-type binding, a stack collection or a manifest embed, so no authored document could ever carry one. PluginStartupResult SURVIVES on both entries with the shape the kernel ships — pluginName, success, optional durationMs, the serializable error projection, timedOut — and @objectstack/core now imports that type instead of declaring a twin, so the drift cannot recur. Writing plugin, health or startTime on a PluginStartupResult is a tsc error and a parse error carrying the rename or the deletion; a reader of the removed startTime alias reads durationMs, which has always carried the same value. Runtime behaviour is unchanged except for that one alias: nothing ever read the retired ORCHESTRATION surfaces, the kernel boot loop is untouched, and the only observable difference is that a startup result no longer carries startTime beside durationMs.
1302
+
- **`storage-scope-public-retired`** — `the storage scope public — the scope of an upload request, presigned or chunked, and ObjectStorageConfig.scope (StorageScope)` → another scope, or none for the default (`user` on an upload, `global` on a storage configuration), and `acl: 'public_read'` on the stored file record of each file that must be readable before sign-in (ADR-0104)
1303
+
- Why not automatic: A storage scope never made a file publicly readable. The download doors judge a file by its `acl`, the `attachments` scope and field ownership alone, so a file uploaded with scope public and the default acl was stored private and needs a signed-in caller, while its scope said otherwise. The value is retired rather than enforced: enforcing it would let any uploader make a file anonymous at upload, and ADR-0104 keeps `acl: 'public_read'` the one opt-in for anonymous download. Whether a given file must be readable before sign-in is the caller's call, so no rewrite can make it: an upload that meant public needs its stored file record marked, and one that did not needs only another scope. The upload request itself carries no acl, and every upload is stored private. Files already stored with scope public are not touched and download exactly as before. ADR-0049
1304
+
- Done when: No upload call names scope public and no ObjectStorageConfig declares it; each upload that did now names another scope or none and is answered 200. Each file that must render before sign-in has acl 'public_read' on its stored file record, and fetching it with no session serves it; fetching any other uploaded file with no session is answered 401.
1302
1305
- **`strategy-context-aggregation-method-narrowed`** — `StrategyContext.executeAggregate aggregations[].method (contracts/analytics-service.ts, exported from @objectstack/spec/contracts) - the parameter type, declared as bare string` → AggregationFunction (count | sum | avg | min | max | count_distinct, data/query.zod.ts) - the same closed vocabulary IDataEngine.aggregate already declares for the identical slot (AggregationNodeSchema.function; the analytics bridge renames method to function and forwards). A caller filling method from a string-typed value narrows the value to the enum - typing it AggregationFunction, or parsing with the spec's own AggregationFunction zod enum where the value enters from data. Values outside the six were never served: the bridge has parsed-and-refused them at runtime since it stopped declaring its own engine type and began parsing the method with the spec enum, and that refusal stays as defence in depth
1303
1306
- Why not automatic: Maintainer ruling 2026-08-28 (option A, census-first): one slot, one declaration. Two spec-declared surfaces described the same value and disagreed about its type: IDataEngine.aggregate's aggregations[].function is the closed six-value AggregationFunction enum while StrategyContext.executeAggregate declared the same slot aggregations[].method: string, so nothing on the analytics side of that seam was compile-checked against the engine's vocabulary - an author, very often an AI (ADR-0033), writing an analytics strategy got no compile-time help and could carry any method name all the way to the bridge's runtime refusal. One slot now has one declaration. Bookkeeping: this is a TYPE narrowing on a runtime TS interface member - no authorable metadata key, no wire shape and no walked-shape def changed, so nothing lands in RETIRED_KEYS_BY_MAJOR / RETIRED_DEFS_BY_MAJOR and the surface ratchets are expected byte-identical. It is a SEMANTIC entry rather than a D2 conversion because there is no authored document or sys_metadata row for the chain to rewrite: the only consumers are TypeScript call sites, and the compile error is the channel that reaches them. In-repo census at the ruling (hard precondition, measured before the narrowing landed): every implementor and every call site filling method is legal under the enum - ObjectQLStrategy.resolveMeasureAggregation emits only the six once it refuses a custom-SQL measure up front, the two literal producers write count, and every test fixture is implementor-side and stays assignable by contravariance.
1304
1307
- Done when: External implementors of StrategyContext stay source-compatible: a handler accepting method: string accepts a superset and remains assignable to the narrowed member. External callers filling method with a string-typed or out-of-vocabulary value fail tsc at the executeAggregate call site on upgrade; the fix is narrowing the value's type to AggregationFunction (parsing with the spec enum where it enters from data), never widening a local mirror of the contract. Runtime behaviour is unchanged: the bridge's parse-and-refuse accepts and rejects exactly the same sets before and after, and no stored metadata or document needs editing.
0 commit comments