Skip to content

Commit 61ea810

Browse files
os-helpclaude
andauthored
fix(runtime): refuse to disable or delete a read-only package (#7560) (#7599)
`PATCH /packages/<id>/disable` and `DELETE /packages/<id>` answered 200 on a platform package, and the DELETE really removed it from the running process's registry listing. One authorized API call took platform functionality out of a live deployment. Reproduced on two platform packages in the QA run behind #7514. Blast radius, measured: the card said the packages come back after a restart — true for DELETE (they are code-loaded), but NOT for disable. `setPackageDisabled` persists the choice to `<OS_HOME>/package-state/<env>.json`, which SchemaRegistry replays at boot, so a disabled platform package stayed disabled across restarts. Two axes, not one. #7033 / PR #7083 gave the domain CALLER authorization — who may call the route. This is the second, missing check on the same routes: what the route may do once the caller is allowed. The caller gate is untouched. No new vocabulary: the refusal is ADR-0070's existing 422 / WRITABLE_PACKAGE_REQUIRED, the code `saveMetaItem` already throws when asked to author INTO a read-only package. The predicate moved out of ObjectStackProtocolImplementation's private method into `@objectstack/metadata-protocol`'s exported `isWritablePackage(engine, id)` and is now referenced by both callers, so "which packages are read-only" has one definition rather than two that drift. Both signals are covered: a booted code package (`engine.manifests`) and a `system`/`cloud` manifest scope. Deliberately not caller-sensitive — no isSystem bypass, unlike the write gate. Read-only is a property of the package. Internal teardown calls `registry.uninstallPackage` directly and never passes through a gate. Packages an org owns still disable, re-enable and delete exactly as before. The new suite drives a REAL SchemaRegistry and asserts the registry LISTING in both directions — the status code alone would not have caught the original harm. Claude-Session: https://claude.ai/code/session_01Q1pL2j4cMGRFc7jAYYmR8U Co-authored-by: Claude <noreply@anthropic.com>
1 parent 9c82146 commit 61ea810

7 files changed

Lines changed: 530 additions & 28 deletions

File tree

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
---
2+
"@objectstack/runtime": patch
3+
"@objectstack/metadata-protocol": patch
4+
"@objectstack/spec": patch
5+
---
6+
7+
fix(runtime): refuse to disable or delete a read-only package on the `/packages` lifecycle routes (#7560)
8+
9+
`PATCH /packages/<id>/disable` and `DELETE /packages/<id>` answered **200** on a
10+
platform package, and the `DELETE` really removed it from the running process's
11+
registry listing. One authorized API call took platform functionality out of a
12+
live deployment. Reproduced on two platform packages in the QA run behind #7514.
13+
14+
**Blast radius, measured.** The card reported that the packages come back after a
15+
restart — true for `DELETE` (they are code-loaded, so nothing is permanently
16+
destroyed), but **not** for `disable`: `setPackageDisabled` persists the choice
17+
to `<OS_HOME>/package-state/<env>.json`, which `SchemaRegistry` replays at boot.
18+
A disabled platform package stayed disabled across restarts.
19+
20+
**Two axes, not one.** #7033 / PR #7083 gave the whole `/packages` domain caller
21+
authorization (`manage_metadata` on writes, the ADR-0106 D4 set on reads, an
22+
anonymous floor) — *who may call the route*. This is the second, missing check
23+
on the same routes: *what the route may do once the caller is allowed*. An
24+
authorized admin — and `isSystem` — is now refused, because read-only is a
25+
property of the **package**, not of the caller. The caller gate is unchanged;
26+
tightening it would not have fixed this and would have broken legitimate admins.
27+
28+
**No new vocabulary.** The refusal is ADR-0070's existing one, reused: `422` /
29+
`WRITABLE_PACKAGE_REQUIRED`, the code `saveMetaItem` already throws when asked to
30+
author *into* a read-only package. The predicate behind it moved out of
31+
`ObjectStackProtocolImplementation`'s private method into
32+
`@objectstack/metadata-protocol`'s exported `isWritablePackage(engine, id)` and
33+
is now **referenced** by both callers — a second hand-kept copy of "which
34+
packages are read-only" is exactly the drift that let `DELETE` remove a platform
35+
package while `saveMetaItem` was refusing to add one field to it. Both read-only
36+
signals are covered: a booted code package (`engine.manifests`) and a
37+
platform-delivered manifest `scope` of `system` / `cloud`.
38+
39+
Packages an org owns (project-scoped bases, ADR-0048 authoring workspaces) still
40+
disable, re-enable and delete exactly as before — pinned in both directions, on
41+
the registry listing rather than on the status code, since the listing is where
42+
the original defect's harm actually showed.

packages/metadata-protocol/src/index.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,13 @@ export type { MetadataDiagnostics } from './metadata-diagnostics.js';
6363

6464
export type { MetadataHostEngine } from './host-engine.js';
6565

66+
// [#7560] ADR-0070's read-only-package rule. The authoring path (`saveMetaItem`
67+
// → `WRITABLE_PACKAGE_REQUIRED`) and the `/packages` lifecycle gate in
68+
// `@objectstack/runtime` (`PATCH /:id/disable`, `DELETE /:id`) both ask it, so
69+
// "which packages are read-only" has ONE definition rather than two that drift.
70+
export { isWritablePackage, READ_ONLY_PACKAGE_SCOPES } from './package-writability.js';
71+
export type { PackageWritabilityEngine } from './package-writability.js';
72+
6673
// #4556 — the `sys_metadata_history.recorded_by` sentinel → NULL conversion,
6774
// as an ADR-0119 D2 migration plan. Run by `os migrate recorded-by`.
6875
export {
Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* ADR-0070 — the read-only-package predicate, in ONE place.
5+
*
6+
* A package is either a **writable base** (an org may author into it, and its
7+
* lifecycle is the org's to manage) or **read-only** (it belongs to the
8+
* deployment that ships it). Until #7560 this distinction was a private method
9+
* on {@link ObjectStackProtocolImplementation}, reachable only by the metadata
10+
* authoring path — so `saveMetaItem` refused to author INTO a platform package
11+
* while `PATCH /packages/:id/disable` and `DELETE /packages/:id` happily took
12+
* the whole package out of the running deployment.
13+
*
14+
* The two callers now share this function rather than each spelling the rule:
15+
* a third read-only signal added here reaches the authoring gate and the
16+
* lifecycle gate together, which is the only way the two can't drift apart.
17+
*/
18+
19+
/**
20+
* The engine surface this predicate reads. Structural on purpose — it is
21+
* satisfied by the real `ObjectQLEngine`, by `MetadataHostEngine`, and by the
22+
* partial doubles the gate tests build, and it keeps this module free of a
23+
* dependency on `@objectstack/objectql`.
24+
*/
25+
export interface PackageWritabilityEngine {
26+
/** Booted code packages, keyed by manifest id (`registerApp` populates it). */
27+
manifests?: { has?(id: string): boolean };
28+
registry?: {
29+
getPackage?(id: string): { manifest?: { scope?: string } } | undefined;
30+
};
31+
}
32+
33+
/**
34+
* Manifest scopes that mark a package as platform-delivered, hence read-only.
35+
* `system` is the platform's own; `cloud` is marketplace / control-plane
36+
* delivered. Anything else (`project`, or an absent scope) is an org's own.
37+
*/
38+
export const READ_ONLY_PACKAGE_SCOPES: readonly string[] = ['system', 'cloud'];
39+
40+
/**
41+
* True when `packageId` is a **writable base** — a DB-backed package an org or
42+
* the AI may author *new* metadata into, and whose lifecycle the org owns
43+
* (ADR-0070 D2). The two read-only kinds return `false`:
44+
*
45+
* • **Booted code packages** — they register a manifest into the engine at
46+
* startup (`registerApp` → `engine.manifests`); their items are code-shipped
47+
* artifacts. Only `allowOrgOverride` overlays are allowed (ADR-0005), never
48+
* fresh authored items.
49+
* • **Installed / platform packages** — manifest `scope` is `system` or
50+
* `cloud` (marketplace / platform-delivered).
51+
*
52+
* A project-scoped DB package, or a bare ADR-0048 *authoring-workspace* id with
53+
* no registered manifest, is writable.
54+
*
55+
* NOTE: the code-package signal is the engine manifest map ONLY — we
56+
* deliberately do NOT fall back to "owns ≥1 registered object" (the old
57+
* `isLoadedPackage` heuristic). A writable base accrues registered objects once
58+
* its drafts publish, and that must never flip the base to read-only — that is
59+
* the exact #2252 read-only-after-publish trap ADR-0070 removes.
60+
*
61+
* NOTE: this is a property of the PACKAGE, not of the caller. There is
62+
* deliberately no `isSystem` escape hatch: #7033 decided *who may call* the
63+
* package routes, and #7560 is what those routes may do once the caller is
64+
* allowed. An authorized admin — and the engine itself — still may not disable
65+
* or delete a package the deployment ships. Internal code that legitimately
66+
* tears a code package down calls `registry.uninstallPackage` directly and never
67+
* passes through a gate.
68+
*
69+
* An absent/empty `packageId` is NOT writable: the authoring path treats "no
70+
* base resolved" as a refusal (`WRITABLE_PACKAGE_REQUIRED`), and answering
71+
* "writable" for an unknown would make this predicate fail open.
72+
*/
73+
export function isWritablePackage(engine: unknown, packageId: string | null | undefined): boolean {
74+
if (!packageId) return false;
75+
const e = engine as PackageWritabilityEngine | null | undefined;
76+
// Booted code package → read-only artifact source.
77+
if (e?.manifests?.has?.(packageId)) return false;
78+
// Installed / platform package → read-only by manifest scope.
79+
const scope = e?.registry?.getPackage?.(packageId)?.manifest?.scope;
80+
if (typeof scope === 'string' && READ_ONLY_PACKAGE_SCOPES.includes(scope)) return false;
81+
// Project-scoped base, or unregistered authoring-workspace id → writable.
82+
return true;
83+
}

packages/metadata-protocol/src/protocol.ts

Lines changed: 15 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,9 @@ import { readEnvWithDeprecation, resolveTenancyPosture } from '@objectstack/type
1212
import { postureEnforcesWall } from '@objectstack/spec/security';
1313
import type { MetadataHostEngine } from './host-engine.js';
1414
import { evaluateRuntimeAuthoringGate } from './runtime-authoring-gate.js';
15+
// [#7560] ADR-0070's read-only-package rule, shared with the `/packages`
16+
// lifecycle gate in `@objectstack/runtime` — see `./package-writability.js`.
17+
import { isWritablePackage as isWritablePackageShared } from './package-writability.js';
1518
import type { RuntimeAuthoringIssue } from './runtime-authoring-gate.js';
1619
// [#6418] `sys_metadata`'s overlay-uniqueness indexes: probe-first DDL plus the
1720
// ADR-0120 D4 reporting that replaced this file's empty `catch` blocks.
@@ -8631,35 +8634,20 @@ export class ObjectStackProtocolImplementation implements
86318634

86328635
/**
86338636
* True when `packageId` is a **writable base** — a DB-backed package an
8634-
* org or the AI may author *new* metadata into (ADR-0070 D2). The two
8635-
* read-only kinds return `false`:
8636-
*
8637-
* • **Booted code packages** — they register a manifest into the engine
8638-
* at startup (`registerApp` → `engine.manifests`); their items are
8639-
* code-shipped artifacts. Only `allowOrgOverride` overlays are allowed
8640-
* (ADR-0005), never fresh authored items.
8641-
* • **Installed / platform packages** — manifest `scope` is `system` or
8642-
* `cloud` (marketplace / platform-delivered).
8643-
*
8644-
* A project-scoped DB package, or a bare ADR-0048 *authoring-workspace* id
8645-
* with no registered manifest, is writable.
8646-
*
8647-
* NOTE: the code-package signal is the engine manifest map ONLY — we
8648-
* deliberately do NOT fall back to "owns ≥1 registered object" (the old
8649-
* `isLoadedPackage` heuristic). A writable base accrues registered objects
8650-
* once its drafts publish, and that must never flip the base to read-only
8651-
* — that is the exact #2252 read-only-after-publish trap this ADR removes.
8637+
* org or the AI may author *new* metadata into (ADR-0070 D2).
8638+
*
8639+
* [#7560] The rule itself moved to {@link isWritablePackage} in
8640+
* `./package-writability.js` because it gained a SECOND caller: the
8641+
* `/packages` lifecycle routes, which must refuse to disable or delete a
8642+
* read-only package the same way this path refuses to author into one. Two
8643+
* hand-kept copies of "which packages are read-only" is precisely the drift
8644+
* that let `DELETE /packages/:id` remove a platform package from a live
8645+
* deployment while `saveMetaItem` was refusing to add one field to it. This
8646+
* method stays as the in-class spelling; the shared function is the
8647+
* definition, and its doc comment carries the reasoning.
86528648
*/
86538649
private isWritablePackage(packageId: string | null | undefined): boolean {
8654-
if (!packageId) return false;
8655-
const engine = this.engine as any;
8656-
// Booted code package → read-only artifact source.
8657-
if (engine?.manifests?.has?.(packageId)) return false;
8658-
// Installed / platform package → read-only by manifest scope.
8659-
const scope = engine?.registry?.getPackage?.(packageId)?.manifest?.scope;
8660-
if (scope === 'system' || scope === 'cloud') return false;
8661-
// Project-scoped base, or unregistered authoring-workspace id → writable.
8662-
return true;
8650+
return isWritablePackageShared(this.engine, packageId);
86638651
}
86648652

86658653
/**

0 commit comments

Comments
 (0)