Repository navigation
Commit e5f9f01
fix(metadata-protocol)!: the save door refuses a
Fixes #23096
Clause-②: no (narrowing)
The Clause-② arm is `(narrowing)` under `scripts/pm/clause2-line.mjs`:
the answer to the widening question is a truthful `no`, but the save
door's accept set narrows. A body it answered `200` is now refused
`400`. That makes this a BREAKING change, shipped `minor` under the
launch-window convention. The changeset carries the same line and its
ADR-0087 disposition. The claim line read a bare `no`, and the dispatch
asked for the arm wherever the repo's rules call for one. They do here.
## What this does
The metadata save door (`PUT /api/v1/meta/{type}/{name}`, which reaches
`saveMetaItem`) now refuses a `protection` block with a located `400
VALIDATION_ERROR` on every type whose schema declares the block. It no
longer stores the block verbatim and serves a lock that binds nothing.
The `protection` TSDoc in `packages/spec` now says what the door does.
## Which branch, and why: ADR-0010
Triage 6133319631 asked which door ADR-0010 states for locking an
environment item. These are the governing sentences:
- **Phase 4.3, the block's door:** "Public author surface
(`packages/spec/src/shared/protection.zod.ts`) that translates into the
private `_lock` envelope at load time". Also: "Loader
(`metadata/plugin.ts` + `objectql/registry.ts`) calls
`applyProtection(item, { packageId })` to translate the block into the
private `_lock`/... envelope and strips the public `protection` block so
it never leaks into the overlay row."
- **§3.5, what an environment item is:** "`org` | Authored at runtime by
a tenant via the metadata API. Set on first save." Also: "Tenant `org`
artifacts ignore L3 / L4 entirely (the tenant owns them in full, can
also delete them)."
- **§3.2 (L2):** "Package authors declare a default lock in the package
manifest. The loader applies it ...". This is not implemented, and
`metadataDefaults` exists nowhere in `packages/spec/src`.
ADR-0010 does not state that an environment item takes `protection` at
save. It states the opposite for the lock itself (§3.5), and it names
one door for the block: a code package's loader. That selects triage's
refusal branch. Translating instead would open a lock door ADR-0010 does
not state, so the translation branch was not taken. The refusal names
the door ADR-0010 does state.
## The change
- `packages/metadata-protocol/src/protocol.ts`:
- `protectionBlockRefusal` is a new module function, called once in
`saveMetaItem`. It runs after the authorization refusals, so a locked
item still answers `ITEM_LOCKED` and a sealed one `NOT_OVERRIDABLE`. It
runs before the destructive diff, the per-type parse and the authoring
gates.
- It asks `overlaySchemaDeclaredKey(type, 'protection')`, the same
structural reading the unmanaged import uses. On a type that does not
declare the block, the parse's own `422` stands.
- Envelope: `VALIDATION_ERROR` / `400`, the envelope of the door's other
content refusals (`savedItemNameRefusal`, the hook-without-body
refusal). No code is minted.
- Located twice: the message names the key, and `issues[0].path` is
`protection`. Every present value is refused, a malformed block
included.
- The message prescribes first, then names ADR-0010's door ("declare
`protection` on it in a code package's source"). It names no `_lock*`
key.
- `withoutProtectionBlock`: the server-stated rewrites strip the block
instead of failing. These are a `source`-stating re-save
(`migrateStoredMetadata`, and the flow credential move, which re-saves
stored flow rows with `source: 'migrate-stored'`) and
`duplicatePackage`. They are the same two that
`tenantAuthoredWriteRefusal` exempts and `stripDerivedProvenance` strips
for. A row stored before this change may carry the block, and it never
bound. Refusing would fail a stored migration, or leave a flow
credential in cleartext, over a key that does nothing.
- `packages/spec`: the `protection` TSDoc on all 15 mounts (app, view
container, view item, dashboard, report, dataset, object, flow, webhook,
agent, tool, skill, position, permission set, email template), plus one
paragraph in the `shared/protection.zod.ts` module docblock. Prose only.
`content/docs/references/shared/protection.mdx` was regenerated from
that docblock (`gen:schema && gen:docs`).
- The new code never calls `applyProtection`: which types declare the
block is read off the registered schema by the existing
`overlaySchemaDeclaredKey`, so no translation is copied anywhere.
- Patch round 1 (`94c3c852b9`), prose only:
- The changeset supersedes one sentence of the unreleased
`15213-unmanaged-lock-strip` entry, in the house form that entry uses.
On a type that declares `protection`, an item the unmanaged install
hands on with its block still in it (a malformed block, or a block
beside a refused `_lock*` value) now meets this door refusal, `400
VALIDATION_ERROR`, and the install is still rolled back. A refused
`_lock*` value with no block, and a block on a type that does not
declare one, still answer `422 INVALID_METADATA`.
- `stripUnmanagedImportLock`'s docblock (`package-unmanaged-install.ts`)
states the same split instead of "refuses it as before".
- Measured through the real `importUnmanagedPackage` with a temporary
probe, not committed: a malformed block gave `400 VALIDATION_ERROR`, a
valid block beside `_lock: 'locked'` gave `400 VALIDATION_ERROR`, and
`_lock: 'locked'` alone gave `422 INVALID_METADATA`, each rolled back.
The probe was restored, blob == HEAD.
## Readings: the card's probes on a showcase boot (`pnpm dev -- --fresh
-p 41099`, seeded admin)
**Before**, at `origin/main` `abf86554c9`:
- `PUT /meta/app/probe_locked_app` with `protection: { lock: 'full',
reason }`: `200`.
- `GET`: `lock: 'none'`, `editable: true`, `deletable: true`, while
`item.protection` is `{ lock: 'full', ... }`.
- Second `PUT`: `200`. `DELETE`: `200`.
- `dashboard/probe_locked_dash`: the same, `200` / `none` / `200` /
`200`.
- The stored row: `sys_metadata.metadata` is
`{"name":"probe_row_app","label":"Row","protection":{"lock":"full","reason":"probe"}}`,
with no `_lock`.
**After**, at `f8644fa685` (`metadata-protocol` dist built at that
commit):
- The `app` and `dashboard` probes answer `400 VALIDATION_ERROR` with
`issues: [{ path: 'protection', ... }]`. `sys_metadata` holds 0 rows
under either name.
- CONTROL, a body without `protection`: `PUT 200`, second `PUT 200`,
`DELETE 200`. The stored row is `{"name":...,"label":...}`.
- CONTROL, the managed package item: `GET /meta/app/setup` reads `lock:
'full'`, `lockSource: 'package'`, `editable: false`, `deletable: false`.
- On an earlier boot of the fix (`1e6bf4fe48`, the same authoring path):
- a `PUT` on `app/setup` carrying `protection` answered `403
NOT_OVERRIDABLE`, not this refusal;
- a `PUT` of a `protection` block into a writable base row
(`?package=com.probe.base`) answered `400`.
## Tests
`packages/metadata-protocol/src/protocol.save-door-protection.test.ts`
has 12 cases. It runs the real save door over a row-keeping engine stub,
copied from the pinned `package-unmanaged-install.test.ts` double, with
the dispatch predicates. Refusals are asserted as
`resolveThrownHttpError` envelopes (`status` and `code`), never as a
bare throw.
- **Probes (`app`, `dashboard`):** `400 VALIDATION_ERROR`,
`issues[0].path === 'protection'`, and zero `sys_metadata` writes.
- **Same refusal:** draft mode, a malformed block (`lock: 'sealed'`, a
missing `reason`, `null`), the unmanaged import's face, and an overlay
of a packaged dashboard that declares no lock.
- **Server-stated rewrites** (`source: 'migrate-stored'`, `writeFace:
'package-duplicate'`): the rewrite lands, the row carries no
`protection`, and the caller's body is not mutated.
- **CONTROLS:**
- The same probe bodies without `protection` save, carry none, and stay
editable and deletable.
- A packaged dashboard, translated by `applyProtection` with
`protection: { lock: 'full' }`, refuses an overlay save `403
ITEM_LOCKED`, with or without a block in the body.
- `page`, which does not declare the block, keeps the parse's `422
INVALID_METADATA`.
**Ablation, both legs**, through `scripts/ablation-replace.mjs` from the
committed state at `edad2a33fd` (`protocol.ts` blob `94ad6a645ede`). The
subject is imported from `./protocol.js`, its source, so no dist was
involved.
- **Leg A**, the verbatim store (`if (protectionRefusal) throw
protectionRefusal;` replaced, anchor 1 to 0, blob `94ad6a645ede` to
`a685b721e5be`): 6 failed, 6 passed.
- Red: both probes, draft, malformed, the unmanaged face and the
overlay. The probes resolved ("expected the call to be refused, but it
resolved"), and the malformed case answered 422.
- Green: the 2 probe CONTROLs, the 2 rewrite strips, the managed
`ITEM_LOCKED` and `page`'s 422.
- **Leg B**, rewrites keep the block (`request.item =
withoutProtectionBlock(...)` replaced, blob to `116fbe9f7340`): 2 failed
(the two rewrite strips), 10 passed.
- **Restore**, after each leg: blob `== HEAD` (`94ad6a645ede`), and `git
diff HEAD` is empty.
**Patch round 1, at `94c3c852b9`** (a changeset sentence and a docblock
only): `pnpm --filter @objectstack/metadata-protocol typecheck` exit 0,
and `package-unmanaged-install.test.ts` +
`protocol.save-door-protection.test.ts` 74/74.
**Suites at `f8644fa685`:**
- `pnpm --filter @objectstack/metadata-protocol typecheck`: exit 0. The
new test is in the tsc program (`--listFiles` lists it once).
- `pnpm --filter @objectstack/metadata-protocol exec vitest run
--maxWorkers=2`: 229 files passed, 3 skipped; 28236 tests passed, 19
skipped.
- `pnpm --filter @objectstack/spec typecheck` and `pnpm --filter
@objectstack/spec exec vitest run --maxWorkers=2`: exit 0, 716 files,
20975 tests passed, 1 todo.
## Gates, at `94c3c852b9` (patch round 1; first run at `f8644fa685`,
same derivation)
- `node scripts/pm/dispatch-gates.mjs --commands` on this diff derived
121 commands: the lead list's 80, plus the changeset, docs-reference and
ledger families this diff adds.
- All 121 exited 0, and so did `pnpm --filter @objectstack/spec
check:generated`.
- `--ran` reconciliation: "121 derived, 121 run, 0 NOT-MEASURED, 0
UNRUN", with every exit code recorded.
- The changeset gates among them: `check-changeset-no-major`,
`check-adr-0087-registration` (`not-required
(no-migration-prescription)` accepted with the supersession sentence in
the body) and `check-empty-changeset`, each exit 0.
- `check:engine-double-contract` first asked the coverage ledger to
learn the new pinned double: `--write`, +3 rows, all for the new file, 0
lost. `check:docs` asked for the regenerated reference page. Both are
committed, and both gates are green at the head above.
- Lint, a declared narrowing: `pnpm exec eslint --no-inline-config
--format json` over the 18 changed `.ts` files gave 18 files, 0 errors,
0 warnings, and none of them is ignored by the config (`isPathIgnored`).
`eslint.config.mjs` never enables type-aware linting (no
`parserOptions.project`), so this diff cannot move any untouched file's
verdict. The full `pnpm lint` is CI's.
## Acceptance notes (observed, not fixed here)
- **Package export.** On `abf86554c9`, a base row saved with
`protection` was copied verbatim into `GET
/packages/com.probe.base/export`'s manifest. Measured: `apps: [{ ...,
protection: { lock: 'full', ... } }]`. By code reading, the package
loader translates such a block when that manifest is installed
(`registerApp`, then `registerItem`, then `applyProtection`). With this
change a base row can no longer carry the block, so a base author
declares the lock in the manifest they install instead. No producer in
this repo writes `protection` through the save door, and the clone doors
already drop it. The changeset states this narrowing.
- **Stored rows.** A row stored before this change keeps its bytes and
is still served with its inert block until something rewrites it.
- **Flow clone.** `FLOW_CLONE_DROPPED_KEYS` (runtime `flow-clone.ts`)
does not list `protection`, while `PERMISSION_SET_CLONE_DROPPED_KEYS`
does. Registry-hydrated flows are already stripped by `applyProtection`.
A clone whose source body still carried a pre-change stored block would
now be refused. Not measured.
- **Envelope choice.** Triage asked for a `400`.
`tenantAuthoredWriteRefusal` uses `INVALID_METADATA` / `422` for a body
carrying keys a client may not persist. This PR follows triage, with the
`VALIDATION_ERROR` envelope of the door's other content refusals.
---
_Generated by [Claude
Code](https://claude.ai/code/session_01ADzJtzYTLUfgrRZHxkagkX)_
---------
Co-authored-by: Claude <noreply@anthropic.com>protection block instead of storing a lock that binds nothing (#23115)1 parent c8a9783 commit e5f9f01
21 files changed
Lines changed: 570 additions & 49 deletions
File tree
- .changeset
- packages
- metadata-protocol/src
- spec/src
- ai
- automation
- data
- identity
- security
- system
- ui
- scripts
| 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 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
44 | 44 | | |
45 | 45 | | |
46 | 46 | | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
47 | 53 | | |
48 | 54 | | |
49 | 55 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
194 | 194 | | |
195 | 195 | | |
196 | 196 | | |
197 | | - | |
| 197 | + | |
198 | 198 | | |
199 | | - | |
| 199 | + | |
| 200 | + | |
| 201 | + | |
| 202 | + | |
| 203 | + | |
| 204 | + | |
200 | 205 | | |
201 | 206 | | |
202 | 207 | | |
| |||
0 commit comments