feat(#470): C4/H1 — manifest-declared, operator-consented public-path grants with exclusive prefix ownership and a terminating early mount - #782
Merged
Conversation
…(C4/H1)
Closes gap G6: a plugin could not contribute an auth exemption, and core
could not revoke one on uninstall.
The obvious fix — making `publicPaths` a dynamic set — rebuilds the hole it
is meant to close. `requireAuth` runs before routing, so it sees a URL and
nothing else and structurally cannot know which router will answer. An entry
there says "this URL needs no session"; it does not say "and only plugin A
may answer it".
So `auth/publicPaths.ts` stays a frozen core-owned literal (both static
exemptions untouched — they leave in C12), and the grant lives in a mount
slot placed before `requireAuth` which TERMINATES: a request under a granted
prefix is dispatched to the owning plugin's router, and if that router does
not handle it the mount answers 404 rather than passing it on into the
authenticated stack.
Three independent gates, all required:
1. Declaration — `permissions.public_paths` in the manifest (zod-validated:
no wildcards, no percent-encoding, no dot segments, no core-reserved
roots, no collision with an existing static exemption, at least two
segments deep, and it must lie under `/api/plugins/` or a prefix the
plugin actually registers a router at).
2. Exclusive ownership — claimed at activation. First plugin wins; a second
one declaring an overlapping prefix fails to activate with an error
naming both sides. Released on deactivate alongside the routers.
3. Operator consent — a row in `plugin_public_path_grants` (migration
0044). Declared-but-ungranted prefixes hold the reservation and serve
nothing.
Fail-closed by construction: no grants, no store, no registry, no live plugin
— every one of those is a `next()` into `requireAuth`, i.e. a 401. There is
no failure mode of this mount that yields less authentication than a build
without it.
Also makes unknown `permissions.*` keys warn instead of vanishing silently
(implementation.md §2.5): a plugin declaring `public_paths` against an
unpatched core used to activate with no grant and no error.
… (C4/H1)
Middleware, on the runtime router (behind requireAuth like every other
runtime endpoint):
GET /installed/:id/public-paths — what the manifest asks for and which of
those the operator granted, reported per declared path rather than as
two lists to diff. Orphaned grants (declaration since withdrawn) are
surfaced rather than hidden.
PUT /installed/:id/public-paths — the COMPLETE consented set, not one path
at a time: consent to an unauthenticated surface should be reviewed as
a whole. Consent can never exceed the declaration — without that check
the consent endpoint would itself be a way to make an arbitrary URL
public, a bigger hole than the one this epic closes. Revokes are
written before grants so a half-failed write leaves the more
restrictive state, and the live registry is updated so a revoke closes
the surface immediately rather than at next boot.
Store detail UI renders the declared prefixes as their own labelled block
rather than another permission chip. It is the most consequential thing a
plugin can request, and the copy says plainly that declaring is not granting
and that consent is withdrawn on deactivate. i18n keys added to en.json and
de.json; no hardcoded strings (i18n literal scan unchanged at 4 for this
file, `translate` bucket still 0).
…he table The consent PUT wrote revokes to plugin_public_path_grants first and only called setGranted() after every grant() had also succeeded. The in-memory registry — not the table — is what the terminating mount consults, so a grant() that threw after the revokes landed left every revoked prefix still answering without a session until the process restarted. Narrow the registry before the table, and re-sync it from the table on the error path (falling back to nothing granted if that re-read also fails). Also corrects two comments that justified the percent-encoding ban with the claim that Express decodes req.path. It does not: req.path is the raw pathname, so the ban removes the second representation outright rather than relying on a decode that never happens. Behaviour unchanged. Adds errorHelp copy (en + de) and ERROR_HELP_CODES entries for the four codes the new endpoints emit, fixing the red web-ui required check.
Resolves the specs/470 README conflict by keeping both sections — main's Phase A / C1 plugin-api snapshot entry and this branch's C4/H1 entry are additive, not competing. Renumbers the public-path grants migration 0044 -> 0046. main landed 0044_sandbox_registry.sql while this branch was open, and 0045 is claimed by the in-flight credentials work, so 0044 here would have collided on a forward-only, filename-ordered series. Updates the two references to the number (publicPathGrantStore.ts doc comment, specs README).
…blic
Three tests drive the real PUT /installed/:id/public-paths against the real
terminating mount, sharing one PublicPathGrantRegistry between the two mounts
exactly as the process does:
- a grant() that fails after the revokes landed still closes the revoked
prefix (401, not 200 from the plugin)
- the happy path leaves exactly the consented set serving
- an error-path re-read that also fails grants nothing at all, rather than
preserving the previous, more permissive set
Mutation check: restoring the pre-fix ordering (no pre-narrowing, no re-sync)
fails tests 1 and 3, 28/30 passing. The happy-path test survives by design —
it is not the regression test.
Also retitles an existing case that repeated the same false premise about
Express decoding req.path.
Weegy
added a commit
that referenced
this pull request
Aug 20, 2026
Weegy
added a commit
that referenced
this pull request
Aug 20, 2026
…grations C4 / H1 landed on main while this branch was being re-based on C2b. The two touch the same manifest surface, and — as C7's spec note predicted — the overlap resolves as unions rather than choices. - src/plugins/manifestLoader.ts: seven conflicts, all unions. `parseSqlPermission` and the `publicPathGrants` imports both kept; the permissions record carries both `sql:` and `public_paths:`; KNOWN_PERMISSION_KEYS is the union `public_paths` + `secrets` + `sql`; C4's `extractPublicPaths` kept whole. Main's wording wins on the unknown-key warning; C7's wording wins on the two doc comments, because it names the retired key's NOTE and is not specific to one permission. - specs/470-dev-platform-plugin/README.md: both sections kept, ordered C1 -> C4 -> C7 -> C8. Renumbered the C7 migration 0045 -> 0047. Main took 0045 for `publish_versions` (#785) and 0046 for `plugin_public_path_grants` (C4), so `0045_plugin_sql_grants.sql` would have shipped as a second 0045. The runner sorts on the full filename and would have applied both, but a colliding number is a trap for the next author; all five prose references moved with the file. Verified against the C4-inclusive main: build, typecheck, typecheck:test (406, baseline 406), lint, api:check, core-decoupling ratchet 3300 (unchanged from main's baseline), pg suites 340/340 serial as CI runs them.
Weegy
added a commit
that referenced
this pull request
Aug 20, 2026
…locked plugin migrations (epic #470 C7/G4) (#787) * feat(#470): grant-gate ctx.services.get and cut plugin-api 1.0.0 (C2b) Closes the second half of C2 (epic #470 Phase A): the G8 contract break and bug B1. G8 — the contract break, taken once, deliberately. C2a (#555) already deleted `ctx.devJobs` and moved the `DevJob*` view types out of `@omadia/plugin-api` into `middleware/src/devplatform/devJobTypes.ts`, where they travel with the extraction. This records that break and cuts the package at 1.0.0: there is no installed base, nothing is published to npm, and every consumer is a repository we control, so the break is cheap now and expensive later (`implementation.md` §1 row 4). Adds a CHANGELOG naming the removed types, so a consumer grepping its own source for `DevJobDescriptor` lands on the migration note. `harness-channel-api` pinned `^0.1.0` and would have failed to resolve against the bump; repinned to `*` like its siblings. B1 — `ctx.services.get` was a bare pass-through. Any installed plugin could resolve any registered service, `graphPool` included, with no manifest declaration and nothing in the install dialog. `serviceRegistry.ts`'s own header conceded it: "enforcement lives at the consumer seam". The seam now exists. `get` resolves only capability names the plugin declares in `requires:` (or `provides:`, to read back its own registration) and throws the new typed `ServiceNotDeclaredError` otherwise, naming the capability and the manifest field that would grant it. `has` stays ungated — existence is not a capability. Removing `ctx.devJobs` without this would have converted a permission-gated, kernel-attributed accessor into an ungated, self-attributed one (§2.2). So `provide` also accepts `perCallerService(factory)`: the kernel invokes the factory with the id it activated the consumer under, never with an argument the consumer supplies. The factory is a symbol-branded object, so a service that is itself a function cannot be mistaken for one; value providers are untouched. A call-site audit across this repo's built-in plugin packages and all ten standalone plugin repos found 27 (plugin, capability) pairs consumed without being declared — a fail-closed gate in one step would have broken every shipped plugin. Those exact pairs sit behind a dated, frozen, per-plugin allowlist: they warn once and resolve, everything else fails closed. The allowlist is closed in both directions — a different plugin asking for the same name still throws, and an allowlisted plugin asking for a new name still throws. Counter-proof: reverting the gate to `return serviceRegistry.get<T>(name)` fails 9 of the 25 new tests; restoring it passes all 25. Ratchet 3296 → 3300, hand-raised for `middleware/packages` only. All five lines are the new CHANGELOG documenting a removal; three of them are literal strings that cannot be reworded (a spec path, a test filename, the future package name). The first measurement was +31 — the avoidable 26 were reworded away rather than excused, leaving `middleware/test` at its 1,030 baseline. Justification recorded in the baseline JSON, README and acceptance.md. Also corrects plan.md §4.2, which still claimed the `DevJob*` types stay in core — `implementation.md` §2.5 had already flagged it as contradicting §4.1, and shipped code now settles it. * feat(#470): permissions.sql + shared plugin migration runner (C7 / G4) Plugins can own tables. Three things had to become true for that to be safe rather than merely possible, and this commit is all three. 1. Reaching a Postgres pool is now a DECLARED, GRANTED permission. C2b made `ctx.services.get(name)` resolve only capabilities the manifest declares in `requires:` (bug B1). That is the right rule for an ordinary capability, but not for `graphPool`: the pool is not a service another plugin provides, it is the operator's own database — the same one core writes user data through. A `requires:` line is the plugin author's own say-so, and that is the wrong bar for handing it over. So pool-shaped capabilities need both halves: `permissions.sql` in the manifest (visible in the install dialog) AND an operator grant row. The two denial reasons stay distinct types because they have different fixers — `undeclared` is the author's, `ungranted` is the operator's, and collapsing them would send every author chasing a manifest bug that is not theirs. The existing `graphPool` capability name is reused; no second name is invented for the same pool. `POOL_SHAPED_CAPABILITIES` is a closed set rather than a heuristic, so a new pool capability is ungated until someone adds it — a reviewable omission instead of an invisible one. 2. A ledger is unambiguously ITS plugin's ledger. A plugin able to name any table could forge another plugin's migration history and thereby suppress that plugin's schema changes at its next boot. The name is charset-validated (`^[a-z][a-z0-9_]{2,62}$`) BEFORE it is quoted into DDL — an allowlist that rejects `"` cannot be defeated by a cleverer `"`, whereas an escaping approach has to be right about every case. Exclusive ownership is enforced by `UNIQUE (ledger)` in `plugin_sql_grants`, not by the prefix rule. The prefix rule cannot separate `acme_tool` from `acme_tool_extra`: a name carrying both prefixes would pass for either plugin. The constraint has no such edge, so the prefix check is documented as the cheap offline half, not as the boundary. 3. Applying migrations is serialised, once, for everyone. `implementation.md` B3 recorded core migrators racing on multi-replica boot: two replicas read an empty ledger and both execute. Handing plugin authors a documented pattern to copy would have recreated that bug once per plugin, in code the operator cannot patch. So there is one `runPluginMigrations` and a plugin cannot opt out of any of it. `pg_advisory_xact_lock` rather than the session-scoped variant: it is released by COMMIT or ROLLBACK, including the server's rollback on a dead connection, so it needs none of the release-or-destroy care a leaked session lock demands. `SET LOCAL lock_timeout` rather than a try-lock poll loop: "wait, but not forever" in one statement. A plugin-only advisory namespace (4420) rather than core's 4410, so a plugin-supplied ledger name that happens to hash onto a core ledger is not an availability lever. One transaction for the whole batch, not one per file: a half-applied plugin schema is a state no file in the directory describes. Empty directory throws. A plugin declaring `permissions.sql.migrations` is asserting it ships schema; a directory with nothing in it means the build dropped the files. "0 applied, all good" would let it activate and fail later against tables that were never created, several layers from the cause. Checksum drift on an applied file throws unless explicitly opted into. An edited migration means the database and the package disagree about what ran. `.js`/`.mjs` migrations receive the same transaction-bound client as `.sql` ones, so the codegen path in implementation.md D6 is not a weaker path. Unknown `permissions.*` keys now warn instead of vanishing silently — same implementation and same `KNOWN_PERMISSION_KEYS` shape C4 (#782) introduced, so the two branches merge as a one-line union of the key set. Migration 0045 (0044 is taken by sandbox_registry on main). * feat(#470): wire the SQL gate and auto-migrate at activate (C7 / G4) The grant is read ONCE, at activate, before the context is built. `ctx.services.get` is synchronous and cannot await a database read. Doing the lookup inside the accessor would force either an async accessor (a breaking change to every plugin) or a cached best-effort answer that is permissive while the cache is cold — and a permission that is permissive while cold is not a permission. So the async read happens where awaiting is free and the gate is a pure function of its result. The grant must also still MATCH the manifest. An operator granted a specific ledger; a package that later ships a manifest naming a different one has not been granted that one. Carrying the stale row forward would let a plugin update silently move its schema somewhere the operator never approved, so a mismatch is treated as ungranted and logged. `ctx.sql` exists only for a plugin that declared, was granted, and has a package root to resolve paths against. The root is not optional bookkeeping: the migrations directory comes from a plugin-supplied manifest, so `../../../etc` is a string a package can ship. Every other core migrator resolves its directory from its own `import.meta.url` and inherits containment for free; this one takes a path from a manifest and therefore has to re-establish it. The `+ path.sep` on the containment comparison is load-bearing — without it a sibling directory named `<root>-evil` passes a bare `startsWith`. Migrations run BEFORE the plugin's `activate()`, and only when the manifest declares `permissions.sql.migrations`. The ordering is not a convenience: it is what makes "the tables exist" an invariant `activate()` can rely on rather than a race each plugin re-loses in its own way. A failure fails the activation, because the alternative is a plugin running against a schema it was not built for — which fails later, further away, and with the database in a state nobody chose. The grant store resolves `graphPool` per call rather than capturing it. `ToolPluginRuntime` is constructed ~600 lines before the pool exists, because the pool is published by a plugin this very runtime activates. A store bound at construction would therefore always be the null one. A plugin that activates before the provider (or before migration 0045 has been applied) reads no grant and is treated as ungranted — the fail-closed direction, self-correcting on the next boot once ordering has settled. * test(#470): C7 gate + migration suites, with the lock counter-proof (G4) 42 tests across two suites, split by what they can honestly prove. `pluginSqlPermission.test.ts` (22, no database) covers the decision table and the name validation — both pure, so the outcome is asserted directly rather than inferred from which exception escaped. `pluginMigrations.pg.test.ts` (20, real Postgres) covers everything whose property belongs to the SERVER rather than to this code: that `pg_advisory_xact_lock` actually serialises, that `CREATE TABLE IF NOT EXISTS` under it cannot double-create, that `UNIQUE (ledger)` actually refuses a second owner. A hand-rolled fake would have all three by construction and would prove only that the fake has them. THE COUNTER-PROOF The concurrency cases are only worth something if they fail without the lock. Removing the `pg_advisory_xact_lock` call and re-running: without lock: RUN 1..5 → pass=0 fail=2 (10/10 failures) with lock: RUN 1..5 → pass=2 fail=0 (10/10 passes) Failure mode is `23505` on `pg_type_typname_nsp_index` — both transactions pass the `IF NOT EXISTS` existence check and both reach the catalog insert. That is the exact race B3 documented, reproduced deterministically, which is the only way to know the lock is load-bearing rather than decorative. Cases chosen so the assertion is the mechanism, not the report: - Ordering is proven by the second migration's column existing, not by the report's own array; the files are created in an order that disagrees with lexical order, so a runner using readdir() order would pass on some filesystems and fail on others. - `allowChecksumDrift` asserts the file is SKIPPED, not re-applied — re-running the CREATE would have thrown 42P07, so the assertion also proves the opt-in does not silently re-execute. - Batch rollback checks the ledger table is gone too, including the row for the migration that individually succeeded. - The injection case re-queries `plugin_sql_grants` afterwards: the table still being there is the proof the name never reached SQL. - A case asserts two DIFFERENT plugins do not serialise, or every boot would funnel every plugin's schema through one lock. - The store's read path is exercised against an unreachable database to show it degrades to "ungranted" rather than to "granted". The `PG_URL` narrowing is an `assert.ok`, not a non-null assertion — the suite-level skip is not a type narrowing, and a bad value should not reach `pg`. Suite uses `PLUGIN_SQL_PG_TEST_URL` first and skips cleanly with a reason when no test database is configured (issue #572: no hardcoded default port). * fix(#470): close six defects found by cross-family review of C7 / G4 Adversarial review of the permissions.sql gate and the plugin migration runner, from a different model family than the one that wrote them. Six findings; each fix carries a test that fails without it (verified by mutation, not assumed). 1. Ledger names now live in a reserved `plg_<sanitized-id>_` namespace. The old rule only required a ledger to START WITH the plugin's folded id, and some plugin ids fold onto real core table names (`@tasks` -> `tasks`, `@agents` -> `agents`, `@plugin/sql` -> `plugin_sql_grants`). For those, `CREATE TABLE IF NOT EXISTS` adopted core's table as the plugin's ledger. Nothing downstream re-checked the name; the only thing that stopped a write was the adopted table happening to lack the ledger's columns, which is a coincidence of column shape rather than a rule. Core migration ledgers were safe only because they all start with `_`, which the charset regex rejects — also not an enforced property. No core table can be inside `plg_`, so the syntactic check now carries the guarantee its own doc comment claimed. Free to tighten today: nothing calls `grant()` yet, so no deployment holds a row under the looser rule. 2. Migrations are time-bounded, and the runner is bounded too. `runMigrations()` was awaited BEFORE `withTimeout(activateFn, 10_000)`, so the activate cap never covered it, and `lock_timeout` bounds only lock ACQUISITION. A migration that took its locks and then ran forever held the advisory lock and blocked boot for every replica. Adds a server-side `statement_timeout` and wraps the call in its own `MIGRATION_TIMEOUT_MS`. The test that claimed "the lock wait budget stays inside the 10s activate() cap" asserted a relationship between two numbers that were never related — it compared one constant to a literal and passed regardless. Replaced with the ordering that does matter (lock wait << statement budget) plus a case that reads both budgets back out of the live transaction. 3. `graphPool` is handed to plugins BORROWED, not owned. `services.get` returned the raw `pg.Pool` — the same one core writes user data through — so `ctx.services.get<Pool>('graphPool')?.end()` in any plugin tore down the whole middleware's connection pool. This is the #665 class of bug and `adminEmbeddingProvider.ts` records it having already happened once. `query`/`connect`/`release` pass through (arbitrary SQL is the granted, intended behaviour); `end` and the listener removers throw, and the `connect().pool` escape is closed. Applied at the plugin-facing seam, so core keeps the real object. 4. Read-once grant semantics are documented and pinned. Revoking a grant does not disarm a plugin that is already running — the gate closes over a boolean taken at activate. That is defensible (see the sync `services.get` argument) but was nowhere stated. Documented on `revoke()` and on `sqlGranted`, and pinned by a test so drift in either direction has to be deliberate. 5. `pluginSqlPermission.test.ts` is text again. It contained a literal NUL byte, so git classified the file as binary: the diff read `Bin 0 -> 11636 bytes` and the entire 11.6 KB of the test proving the injection defence was invisible in the PR, unreviewable and un-blameable. Now escaped. 6. Timeout conditions surface as typed errors. `55P03` / `57014` reached the caller as bare pg errors, so an activation log blamed the package for what is usually another replica holding the lock. Both are re-typed as SqlMigrationError; every other pg error still propagates raw, because a syntax error in a plugin's own SQL is most useful unwrapped. Verified: build, typecheck, typecheck:test (406 baseline, no regressions), lint all pass. Full suite 7298 tests / 7286 pass / 0 fail. pg suite 22/22, re-run 3x. check-core-decoupling held at 3300 (the +3 my new tests cost was reworded away, not raised).
6 tasks
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 free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
What & why
Epic #470 C4 / H1 — manifest-declared, operator-consented public-path grants, exclusive
prefix ownership, and the terminating early mount. Closes gap G6: a plugin could not
contribute an auth exemption, and core could not revoke one on uninstall.
Per
implementation.md§1 row 1 and §3, andplan.md§3 (G-gaps) / §4.Design — why
publicPathsstays frozenThe obvious fix, making
publicPathsa dynamic set, rebuilds the hole it is meant to close.requireAuthruns before routing: it sees a URL and nothing else, so it structurallycannot know which router will finally answer. An entry there says "this URL needs no
session" — it does not say "and only plugin A may answer it". Plugin A gets a grant for a
prefix, does not handle some subpath, and whatever else matches answers it with no session.
So
auth/publicPaths.tsstays a frozen, core-owned literal and both static exemptionsstay (they leave in C12, not here). The grant instead lives in a mount slot placed before
requireAuthwhich terminates:An unhandled path under a granted prefix is answered 404 from the mount and goes no
further. The granted prefix is a closed world owned by exactly one plugin.
Fail-closed by construction. Every "I don't know" is a
next()intorequireAuth: nogrants loaded, store unreachable, registry unwired, plugin not activated, plugin deactivated.
There is no failure mode of this machinery that produces less authentication than a build
without it.
Three independent gates
permissions.public_pathsin the manifestplugin_public_path_grantsSchema + migration
Migration
0044_plugin_public_path_grants.sql(next free number;0043was the max —note
0040is already doubled upstream, this does not add to that).permissions.public_paths: string[]is zod-validated (publicPathEntrySchema). Rejected:wildcards, percent-encoding (
%2e%2eis..once Express decodes), query/fragment, dotsegments, one-segment claims, core-reserved roots (
/api/v1/admin,/api/chat, …), andanything already matched by a static core exemption. Accepted only under
/api/plugins/orunder a prefix the plugin actually registers a router at — checked against the live route
registry, so no core file has to name any particular plugin's paths (this is what keeps the
decoupling ratchet flat and what makes the C12 handover a config change, not a validator
rewrite).
Unknown
permissions.*keys now warn instead of vanishing silently (implementation.md§2.5 — a plugin declaring
public_pathsagainst an unpatched core used to activate with nogrant and no error). Retired keys are deliberately not exempted from the warning: "core removed
this, your manifest still asks for it" is signal the author needs.
Consent UI + admin API
GET /api/v1/admin/runtime/installed/:id/public-paths— declared vs granted, reported perpath rather than two lists to diff; orphaned grants surfaced, not hidden.
PUT …/public-paths— the complete consented set, not one path at a time. Consent cannever exceed the declaration; without that check the consent endpoint would itself be a way
to make an arbitrary URL public. Revokes are written before grants so a half-failed write
leaves the more restrictive state, and the live registry updates so a revoke closes the
surface immediately rather than at next boot.
permission chip — it is the most consequential thing a plugin can request. Copy states
plainly that declaring is not granting and that consent is withdrawn on deactivate.
en.jsonandde.json. No hardcoded strings.Tests + counter-proof
middleware/test/publicPathGrants.test.ts— 27 tests, built on the production mount order(
express.json→cookieParser→ mount →requireAuth→ routers), never a bareexpress().That matters here specifically: this epic's own router once passed an e2e test against a bare
app while 401'ing in production behind the blanket
/apigate.Required cases, all green:
requireAuthdispose()anddisposeBySource) → 404Plus: rollback of partial claims, idempotent re-activation,
/api/plugins/acme-evilnotmatching
/api/plugins/acme, consent unable to invent ownership, static core public pathsstill reachable.
Counter-proof. The mount takes a
terminateoption that production never sets. The testruns the same fixture both ways and asserts the two outcomes differ: with termination the
unhandled path is 404; with it disabled the request escapes into the authenticated stack (401).
A test that cannot fail when the mechanism is removed is not evidence.
Mutation checks (applied, observed, reverted):
terminateforcedfalseresolve()Commands run
Node v22.22.3 (nvm).
origin/mainmerged in before this was opened.middleware: npm install && npm run buildmiddleware: npm run typecheckmiddleware: npm run typecheck:testmiddleware: npm run lintmiddleware: npm test(pg at127.0.0.1:55438)web-ui: npm run lintweb-ui: npm run typecheckweb-ui: node scripts/i18n-validate.mjsweb-ui: node scripts/i18n-literal-scan.mjstranslatebucket 0;store/[id]/page.tsxunchanged at 4node scripts/check-core-decoupling.mjsThe 4 failures are
neonDatasetFilterEscapingandpalaiaHybridRetrievalNeon— allrelation "graph_nodes" does not existagainst an unmigrated local test DB. Verifiedpre-existing: the same files fail 4/4 on a detached
origin/maincheckout with identicalerrors. Nothing in this PR touches them.
Decoupling ratchet held at 3296 deliberately — two accidental new references (a
dev_repo_plugin_grantsmention in a doc comment, and a retired permission key in anallowlist) were both reworded/removed rather than raising the baseline.
Not in this PR
being proven on a live phone-home (P5).
granting/revoking is operable via the runtime API above. Wiring a toggle belongs with the
surface that first needs it.
revokeAllForPlugin) but is not yet called from the uninstall path— deactivate releases ownership, so no prefix is served; the row would only matter on a
same-id reinstall.
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Cross-family review (Forge)
Adversarial review of the auth boundary, driven against the code and a running Express
stack rather than the description. Express version in this tree is 5.2.1. Three defects
fixed on the branch; commits
d5e95b0e,2a3552f7,66501ee2.Findings
PUT …/public-pathswas fail-open on revoke. It wrote revokes to the table, then everygrant(), and only then calledsetGranted(). The in-memory registry — not the table — is what the terminating mount consults, so agrant()throwing after the revokes landed left every revoked prefix still answering without a session until process restart. The comment "the surviving state is the more restrictive one" was true of the table and false of the thing that gates requests.req.path". It does not —req.pathisparseurl(req).pathname, raw and undecoded (verified:/api/plugins/acme/%68ellostays%68ello). Behaviour was safe, the stated invariant was not; a maintainer trusting it could relax the ban.web-uicheck red: the four codes the new endpoints emit had noerrorHelpcopy and were missing fromERROR_HELP_CODES, failingerrorHelpCoverage.test.ts(16 missing keys).what/nextcopy in en + de. 24 files / 235 tests pass.mainlanded0044_sandbox_registry.sqlwhile this PR was open, and0045is claimed in flight. Two0044_*files in a forward-only, filename-ordered series.0046_plugin_public_path_grants.sql; both references updated.listForPlugin()and then claims; a revoke landing in that window is overwritten by the stale read until the next consent change. Direction is less-restrictive.req.pathand confirms which prefixes are granted. JSON content type, so not XSS.CORE_RESERVED_ROOTSis a denylist, not an allowlist./api/usageand/api/v1/dev-platformare core mounts after the public mount and are not reserved (the latter looks deliberate for the C12 handover). A granted prefix also inverts route precedence — the plugin router runs ahead of core for authenticated requests too. Bounded by operator consent and by the plugin having registered a router there.The seven questions, answered against the code
..client-side, sofetchcannot test this): literal../..,%2e%2e,..%2f,..;/, doubled slashes, trailing slash, bare prefix,%00, andTRACE/FOO/OPTIONS— every one either terminates 404 at the mount or falls closed torequireAuth(401). None reached the core router. Case variants and an encoded prefix (/API/…,/api/plugins/%61cme/…) fail closed. Express strips the mount prefix and will not traverse out of it.publicPaths()and the reserved roots at write time. Cross-plugin exclusivity holds inclaim(), which is fully synchronous — noawaitinside — so two concurrent activations cannot interleave inside it. The ownership claim is atomic w.r.t. the event loop.disposeBySource()andreleaseBySource()in the same breath; dispatch re-resolves the live router per request and the wrapper re-checksentry.disposed, so a plugin deactivated between resolve and dispatch still stops serving. Uninstall not callingrevokeAllForPluginis acknowledged in the body and is inert — deactivate releases ownership, so nothing serves.CREATE TABLE IF NOT EXISTS, and the runner (harness-orchestrator/src/registry/migrator.ts) already holds a session-scopedpg_try_advisory_lockwith per-file transactions and a bookkeeping table — the feat(plugins): allow .sql in packages + make the 8 migrators concurrency-safe #552 pattern./api/v1/admin/runtimebehindrequireAuth, alongsidePATCH …/audit-mode. No separate admin role and no CSRF token exists anywhere in this repo, and nocors()is configured, so a JSONPUTis preflighted and blocked cross-origin. Not a regression, but this endpoint makes a URL unauthenticated, so a role gate is worth considering separately.claim()runs afteractivate()against the live registry, and dispatch filters onentry.source === pluginId, so a plugin can never receive another plugin's granted traffic. Registering later only widens what that same plugin serves under its own already-granted prefix.permissions.*keys warn? Yes —warnOnUnknownPermissionKeys, allowlist of 11 keys, warns without rejecting the manifest. There is no plugin manifest JSON-schema file to keep in step.Verification
middleware: npm run buildmiddleware: npm run typecheckmiddleware: npm run typecheck:testmiddleware: npm run lintmiddleware: tsx --test test/publicPathGrants.test.tsweb-ui: vitest run app/_lib/__tests__/web-ui: npm run typecheckweb-ui: npm run lintweb-ui: i18n-validate.mjsweb-ui: i18n-literal-scan.mjstranslatebucket 0;store/[id]/page.tsxunchanged at 4node scripts/check-core-decoupling.mjsMutation check on the new tests — restoring the pre-fix ordering (no pre-narrowing, no
re-sync) fails tests 1 and 3 (28/30). The happy-path test survives by design; it is not the
regression test.
Verdict: MERGE. The core design holds under adversarial probing: the terminating mount is
genuinely a closed world, and every failure mode I could construct degrades to more
authentication. The one real hole was on the revoke path, and it is fixed and evidenced.