Uh oh!
There was an error while loading. Please reload this page.
fix(runtime): refuse to disable or delete a read-only package on the /packages lifecycle routes (#7560) - #7599
Merged
Conversation
`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. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Q1pL2j4cMGRFc7jAYYmR8U
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Contributor
📓 Docs Drift CheckThis PR changes 3 package(s): 112 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
⛔ 7 release-owned page(s) also reference the affected code. These are read-only:
|
os-help
marked this pull request as ready for review
August 11, 2026 08:06
Uh oh!
There was an error while loading. Please reload this page.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes#7560
The defect
PATCH /packages/<id>/disableandDELETE /packages/<id>answered 200 on a platform package, and theDELETEreally 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 — one correction to the card
The card states the packages come back after a restart. That is true for
DELETE(they are code-loaded, so nothing is permanently destroyed). It is not true fordisable:setPackageDisabled(packages/runtime/src/package-state-store.ts) persists the choice to<OS_HOME>/package-state/<env>.json, andSchemaRegistryreplays it at boot viainitialDisabledPackageIds(packages/objectql/src/registry.ts:2202). A disabled platform package stayed disabled across restarts — so the disable half was the more durable of the two, not the lesser.Two axes, not one
#7033 / PR #7083 gave the whole
/packagesdomain caller authorization —manage_metadataon writes, the ADR-0106 D4 read set, plus a domain-wide anonymous floor. That decides 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. In every new test the caller holds
manage_metadataand is refused anyway, because read-only is a property of the package, not of the caller. Tightening the caller gate would not have fixed this and would have broken legitimate admins.The refusal — ADR-0070's existing vocabulary, not a new one
422/WRITABLE_PACKAGE_REQUIRED— the codesaveMetaItemalready throws when asked to author into a read-only package (packages/metadata-protocol/src/protocol.ts, ADR-0070 D1/D2). Same status, same code, samedetails.packageId/details.docs. The sentence is lifecycle-specific only because the authoring one ("switch to a writable package in the package selector") names a remedy that makes no sense for a delete.The predicate behind it moved out of
ObjectStackProtocolImplementation's privateisWritablePackageinto an exportedisWritablePackage(engine, packageId)in@objectstack/metadata-protocol, and both callers now reference it. A second hand-kept copy of "which packages are read-only" is exactly the drift that letDELETEremove a platform package whilesaveMetaItemwas refusing to add one field to it. Both read-only signals are covered:engine.manifests, populated byregisterApp) — the shape the repro used;scopeofsystemorcloud.Deliberately not caller-sensitive: no
isSystembypass, unlikerequireManageMetadata. Internal teardown callsregistry.uninstallPackagedirectly and never passes through a gate.Not an outage
A gate that refuses everything is not a fix. Packages an org owns (project-scoped bases, ADR-0048 authoring workspaces) still disable, re-enable and delete exactly as before, and an unknown package id still gets its 404 rather than being re-labelled 422 — so the gate never becomes an existence oracle of its own.
What the tests assert
packages/runtime/src/domains/packages-readonly-gate.test.ts(15 tests) drives a realSchemaRegistry, notvi.fn()doubles, and asserts the registry listing —getAllPackages()/pkg.enabled— in both directions. The original defect's harm was that the package left the listing of the running process; a mockuninstallPackagecannot tell you whether it survived, and the status code alone would not have caught it.Mutation table — every new test proven able to fail
Each mutation applied to the merged tree, suite run (
packages-readonly-gate+ the pre-existingpackages-capability-gate, 91 tests), then reverted. Predicate mutations required ametadata-protocolrebuild, since@objectstack/runtimeresolves the builtdist.DELETEgate removed (pre-fix state)disablegate removed (pre-fix state)false— the outage shapeengine.manifests) droppedscopesignal droppedsystem+ bothcloudcases, isSystem, axis, envelope, verbDELETEgate moved afteruninstallPackagedisablegate moved afterdisablePackageenabled/statusassertionsPACKAGE_READ_ONLYM3 is the load-bearing one for the accept bar: refusing everything breaks not only the new writable-package cases but four of #7033's own allow-path tests.
Gates
pnpm lint— cleanpnpm typecheck(spec, runtime; metadata-protocol has notypecheckscript) — clean@objectstack/runtime2005 ✓ ·@objectstack/spec9805 ✓ ·@objectstack/metadata-protocol1051 ✓ ·@objectstack/objectql3153 ✓ ·@objectstack/rest1344 ✓content/docs/releases/**edits.Scope notes
packages/rest/src/rest-server.tsuntouched. The lifecycle routes are wired inpackages/runtime/src/domains/packages.ts, so this does not collide with SECURITY:DELETE /api/v1/reports/:idanswers 500 for another owner's report but 204 for a nonexistent id — an enumeration oracle over report ids #7523 / [rest] Hook refusals carrying an explicitstatusCodeare not mapped by/api/v1/data— they leak as HTTP 500INTERNAL_ERRORwith no located guidance #7525 / Three ledgered /meta routes are never mounted and die in the/meta/:typecatch-all — the route audit can't see this class because it treats the ledger as ground truth for what's mounted #7526.POST /api/v1/packages/publishanswers a misleading 405 (Allowed: DELETE, GET, HEAD, PATCH) — the request is absorbed by/packages/:idbecause the REST package registrar is not mounted on showcase #7563 left alone (REST package registrar not mounted on showcase). Different defect, same route family. This change neither helps nor hinders it: it adds a gate inside an already-reached handler and touches no mounting or route-matching.PATCH /packages/:id(manifest edit — name/description/version) mutates a platform package's manifest with the same absent gate. Not fixed here because the card scopes to disable + delete; the one-line fix is the samerequireWritablePackagecall.Generated by Claude Code