feat(core)!: retire PluginSecurityScanner — plugin security scanning is not a platform capability - #15930
Conversation
…14919) ADR-0049 enforce-or-remove; maintainer ruling 2026-09-05 (director summon #14, decision batch #42). The class was a shell that reported success: four of its five private scanners returned an empty issue list unconditionally, and the fifth matched against an in-memory vulnerability database whose only writer had zero callers -- so every scan() ever performed answered status: 'passed' with a perfect score, for a malicious plugin as readily as a benign one. A security control that cannot fail is worse than none, because callers rely on it. - delete packages/core/examples/phase2-integration.ts (the only constructor) - delete src/security/security-scanner.ts; drop its export block from src/security/index.ts, leaving a tombstone naming the retirement - rewrite PHASE2_IMPLEMENTATION.md section 6 to state plainly that plugin security scanning is NOT a platform capability, and drop the two capability claims elsewhere in the same document that outlived their subject - delete the FOLLOW-UPS.md row, repair the paragraph that existed only to compound it, and correct the neighbouring row whose evidence the deletion falsified - pin the retirement as an export-list assertion on both barrels Repair was refused by name: a real vulnerability scanner is a feature with a design surface, not a defect fix. There is no replacement export. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ
📓 Docs Drift CheckThis PR changes 2 package(s): 27 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: ⛔ 1 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails. What this run could not see
Coarse fallback — 135 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 3bd95c5c5e6b2d4f98cf5da03fa76e27737191a3 && git checkout 3bd95c5c5e6b2d4f98cf5da03fa76e27737191a3
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin f1e91595f706b7eefb73457754f4eb05fa13e362 67e559697ada8e10715b6dfa73cadf71446164de && git checkout -B drift-repro f1e91595f706b7eefb73457754f4eb05fa13e362 && git merge --no-ff 67e559697ada8e10715b6dfa73cadf71446164de
node scripts/docs-audit/affected-docs.mjs --json f1e91595f706b7eefb73457754f4eb05fa13e362
|
…0087 ledger (#14919) check-adr-0087-registration refused the previous disposition, correctly: the changeset carries a real consumer prescription (delete the import and every call), so `not-required (no-migration-prescription)` was a self-contradiction. Every other not-required category is false too -- @objectstack/core publishes, so `unpublished` is out; `already-registered` has no entry to name; `type-surface-only` needs an any/unknown-to-concrete narrowing this is not; and `runtime-interface-only` explicitly inherits the same prescription refusal (#8299). The only truthful disposition left is `registered`. That is also the repo's settled convention for this exact shape -- a published TS symbol with no spec schema, no stored source and no tombstone, where the ledger is the only channel that reaches an upgrader. contracts.IDataDriver.findStream and actor-user-roles-to-positions are both registered on those grounds. D3 semantic, not a D2 conversion: the class has no spec schema, so there is no authorable key to tombstone and no stored sys_metadata row to rewrite -- a scanner was constructed per call and every result lived in a per-instance Map discarded with the object, so applyConversionsToStoredItem has no seam that would ever see one. This is what the ruling's "no metadata migration" excludes, and it is excluded. - add entries/semantic/18.plugin-security-scanner-retired.ts (one file, per the entries README kit -- no hand edit inside registry.ts's generated markers) - regenerate registry.ts via gen:migration-registry (157 semantic entries) - flip the core changeset's marker to `registered plugin-security-scanner-retired`, keeping the BREAKING banner, the no-replacement statement and the NOT MEASURED paragraph untouched - add the @objectstack/spec patch changeset, mirroring the #6138 backfill Measured and recorded in that changeset: the regeneration lap the entries README warns about did not materialise. check:generated reports all 15 artifacts up to date, and running gen:spec-changes and gen:upgrade-guide explicitly moved neither file -- a major-18 semantic entry is not yet projected into either. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ
…ionale into the body check-adr-0087-registration parses everything after `registered` as a comma/space-separated id list, so the trailing `why` prose that the `not-required (...)` forms accept was read as 131 nonexistent migration ids. The asymmetry is real and AGENTS.md spells it: `registered SOME-MIGRATION-ID` carries no `why`, the three `not-required` forms do. The rationale is unchanged, only relocated into the changeset body where a reader gets it anyway. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ
|
Contract review (clause ②) PASS at head Landing follows Implemented-by: Generated by Claude Code |
|
PM note on this PR's clause-② status — recorded so a cleared label is not later read as a review that happened. Measured just now, not inferred:
The one thing that should not be misread. This card was declared So the carrier now reads clear, but it reads clear because it was cleared — not because a review passed. If this PR lands from here, it lands on maintainer authority. That is the maintainer's call to make; this comment exists only so it is made knowingly, and so nobody reading the labels in a month concludes a review is on file. This seat has not armed this PR and will not arm it, consistent with the park recorded on the card. No action is requested. Two things still on offer, if wanted:
Generated by Claude Code |
|
Correction to the PM note above (5553093398) — a clause-② review at The open scope question the note repeats was ruled in that PASS (row 4): the Generated by Claude Code |
Closes #14919
Retires
PluginSecurityScannerfrom@objectstack/coreunder ADR-0049enforce-or-remove, across all four surfaces the ruling names, plus the ADR-0087
ledger entry the disposition turned out to owe.
Why removal beat repair
The class was a shell that reported success.
scan()composed five privatescanners:
scanCode,scanMalware,scanLicenses,scanConfigurationeach allocatedan empty issue array, logged, and returned it with no code in between. None
could report a finding for any input.
scanDependenciesran a real loop, but matched only againstvulnerabilityDb,an in-memory Map whose sole writer was the public
addVulnerability— whichhad zero callers in this repo, in objectui at the pinned sha, and in the
example itself.
updateVulnerabilityDatabase()logged twice and fetchednothing.
So the database was empty on every code path that has ever executed, no issue
was ever produced, the score stayed 100, and the result was
status: 'passed'for every plugin the scanner was ever handed — a malicious one included. A
security control that cannot fail is worse than no control, because callers rely
on it. Repair was refused by name: a real vulnerability scanner is a feature with
a design surface, not a defect fix. There is no replacement export.
What changed
packages/core/examples/phase2-integration.tspackages/core/src/security/security-scanner.tspackages/core/src/security/index.tspackages/core/PHASE2_IMPLEMENTATION.mddocs/qa/platform-checklist/FOLLOW-UPS.mdpackages/core/src/security/security-scanner-retirement.pin.test.tspackages/spec/src/migrations/entries/semantic/18.plugin-security-scanner-retired.tspackages/spec/src/migrations/registry.tsgen:migration-registry(157 semantic entries).changeset/plugin-security-scanner-retired.md@objectstack/coreminor, BREAKING banner.changeset/plugin-security-scanner-ledger-entry.md@objectstack/specpatch, the ledger halfThree exports leave the public surface, not one:
ScanTargetandSecurityIssueexisted only to feed the class and were exported beside it. The changeset names
all three.
The pin is an export-list assertion, not a grep
Object.keys()over both barrel namespaces, per the ruling. A grep cannot answerthis: the name legitimately survives in the tombstone comment, in the pin's own
header and in the retired doc section, so a grep pin would go red on the
tombstones that exist to explain the retirement, and would stay green if the
class were re-exported under a different local name.
The pin carries a control assertion —
PluginSandboxRuntime, the exportblock immediately above the retired one, reaching the root barrel by the same
export *line the scanner used. Without it, a barrel that failed to load wouldanswer "absent" for every name and pass forever.
Reverse verification. Re-adding a
PluginSecurityScannerdeclaration to thesecurity barrel turned both retirement assertions red while the control
stayed green —
expected [ 156 names ] to not include 'PluginSecurityScanner'on the root barrel and
[ 82 names ]on the security barrel. Mutation confirmedon disk before the run; restore proven byte-exact afterwards (worktree
git hash-objectequal to the HEAD blob,git diff HEADempty). Both barrelsare reached by relative specifiers into this package's own
src/, so nodistleg is involved and no rebuild gates the result.
The ADR-0087 disposition, and what the gate actually wanted
The first push carried
not-required (no-migration-prescription)andcheck:adr-0087-registrationrefused it. The gate was right, and the reading isworth recording because the fix was not the obvious one.
What it wanted. Not a missing marker — an untruthful one. The changeset
carries a real consumer prescription (delete the import and every call), so
claiming no consumer has to rewrite anything is a self-contradiction the gate
checks statement-against-statement. Every other
not-requiredcategory is falsehere too:
@objectstack/corepublishes, sounpublishedis out;already-registeredhad no entry to name;type-surface-onlyneeds anany/unknown-to-concrete narrowing this is not; andruntime-interface-onlyexplicitly inherits the same prescription refusal rather than escaping it.
So the only truthful disposition was
registered, which required actuallywriting the ledger entry.
That is also the repo's settled convention for this exact shape — a published TS
symbol with no spec schema, no stored source and no tombstone, where the ledger
is the only channel that reaches an upgrader.
contracts.IDataDriver.findStreamand
actor-user-roles-to-positionsare both registered on those grounds, and thegate's own header names the predicament verbatim.
D3 semantic, not a D2 conversion — so this is not the metadata migration the
ruling excludes. The class has no spec schema, so there is no authorable key to
tombstone and no stored
sys_metadatarow to rewrite: a scanner was constructedper call and every result lived in a per-instance Map discarded with the object,
leaving
applyConversionsToStoredItemno seam that would ever see one.Two more things the gate taught, both measured rather than assumed:
nothing; the gate re-read the old text from
HEADuntil the edit wascommitted.
registeredtakes ids only — no trailingwhy. The threenot-requiredforms accept prose after the category;
registeredparses everything after itas a comma/space-separated id list, so a rationale sitting there was read as
131 nonexistent migration ids. The rationale now lives in the changeset body,
where a reader gets it anyway. That asymmetry is real and AGENTS.md spells it.
The regeneration lap did not materialise, and that is measured. The entries
README warns that
spec-changes.jsonanddocs/protocol-upgrade-guide.mdareprojections that must be regenerated when an entry lands. Here
check:generatedreports all 15 artifacts up to date, and running
gen:spec-changesandgen:upgrade-guideexplicitly moved neither file — a major-18 semantic entryis not yet projected into either.
registry.tsis the whole generated diff.Beyond the four named sites, and why
Two edits go past the ruling's literal enumeration. Both are the same defect
class the ruling closes — a shipped document teaching a capability that does not
exist — and leaving either would have produced exactly the dangling reference the
ruling forbids.
PHASE2_IMPLEMENTATION.mdcarried two more capability claims that name noclass and so were invisible to a grep for the symbol: "Security scanner
integrates with CVE databases" (Security) and "Security scanning can be run
asynchronously" (Performance). The first is replaced by a statement that the
platform performs no plugin security scanning; the second is dropped.
FOLLOW-UPS.md's row had two dependents. The paragraph beginning"Compounding the first row" existed only to elaborate it, and cites two files
this PR deletes; after the deletion "the first row" would also point at a
different surface. It is rewritten to record the closure and to preserve the
half that survives. The neighbouring row's evidence column said its "only
consumer is the dead scanner" — this PR makes that false, so it now reads
zero consumers.
Verification
All at head
67e559697. Exit codes captured by redirect before any pipe.pnpm --filter @objectstack/core exec vitest run --maxWorkers=2Test Files 50 passed (50)·Tests 1204 passed (1204)pnpm --filter @objectstack/core run typechecktsc --noEmit+tsc -p tsconfig.examples.json+check:test-typecheck: OKpnpm --filter @objectstack/spec run check:generatedAll 15 generated artifacts are up to datenode scripts/check-adr-0087-registration.mjs --base origin/mainregistered plugin-security-scanner-retired (new here: plugin-security-scanner-retired)node scripts/check-published-readme-exports.mjs60 published document(s) across 79 workspace package(s)scripts/pm/dispatch-gates.mjsderives for this diffpackages/spec(54 before)The examples program still has an input after the deletion
(
kernel-features-example.ts), and--listFilesconfirms the new pin is insidetsconfig.test.json's program while the deleted source is inside neither — sothe typecheck green is a measurement over the new file, not a green over a file
nothing read.
Two derivation notes, since the derived list is a lead rather than a
specification.
check:migration-registry— the gate that provesregistry.tsstill matches the entries directory, and the one most obviously implicated by
this diff — is not in the derived 75; it was run anyway, via spec's
check:generated. Andcheck:api-surfaceentered the derived set only once thediff reached
packages/spec; on the core-only diff it was named by the rulingand by nothing else.
Parked — do not land
Clause-2 card (a public export is removed), reviewable only at
CONTRACT_REVIEW_TIER, which is quota-exhausted. The dispatch exemption coversdispatch only, never contract review.
needs:contract-reviewis on this PR andon the card. Green CI on this PR is not landability.
Generated by Claude Code