fix: freeze external plugin content (F19) - #262
Merged
Conversation
…n the gate Audit F19: capture a SHA-256 of each plugin entry and its relative import graph at load, freeze the registered pack's rulesConfig, and re-verify the fingerprint before write-time lint and every command-gate cycle. Drift hard-fails with a restart instruction instead of silently applying weaker on-disk rules under the same pack id.
The freeze itself was sound; every path that APPLIES it was not. Write-time lint re-verified inside `if (engine === null)`, the one-time ESLint construction branch, and inside the `try` whose `catch` returns `[]`. So the check ran once per linter and, when it did fire, a drift was reported as a clean file. `runWriteGuard` then swallowed it a second time. The check now runs on every lint, outside both catches, and drift travels as `ExternalPackDriftError` so the best-effort handlers on the write path rethrow it while still absorbing ordinary linter faults. An unreadable entry yielded a valid-looking digest over zero files: reachable, since entry resolution does no extension/index lookup, and every such plugin shares that one constant forever. A graph past the file/byte cap was silently truncated, leaving everything past the cut editable under an unchanged digest. Both now fail closed, and a re-hash that cannot be computed counts as drift rather than as proof of no change. Also: fingerprint the real path, so a symlinked entry pins the dependency graph Node actually loads; follow bare `import "./x"` specifiers, which bound no names and so were never walked at all; re-hash after import, so a plugin that rewrites itself while loading is refused instead of pinned to bytes that never ran; freeze each rule module, since a retained reference could swap `create` for a no-op with no on-disk change to detect; and carry the pack and its fingerprint in one map entry, so an unpinned external pack can no longer be registered. Tests drive the real entry points — write-time linter, write guard, command gate, the session's auto re-detecting gate — since every original defect lived at those seams and not in the check.
Round 2 of the panel found the freeze covering less than the code that runs. `isUnderRoot` dropped every relative import resolving above the entry's own directory — silently, on both the queue push and the walk. A plugin at `plugins/pack.ts` importing `../shared.ts` executed that file on every lint with none of it pinned, and shared severity config is exactly what lives one level up. Containment is gone: a relative import is workspace code wherever it lands. Package specifiers were not walked at all. In a monorepo a linked workspace package or a path alias is imported BY NAME and still lives in the repo, as editable as the plugin itself. Those now resolve and join the graph; anything whose real path sits under `node_modules` stays out, since installed dependencies are not the surface this pins and walking them would drag in the tree. The cap check asked whether the queue was empty, but `candidatePaths` queues up to 16 spellings per specifier and at most one exists — so a graph that fit was refused for the phantoms trailing behind it. Counting now happens after a successful read, against limits raised to 512 files / 8 MiB, with a separate bound on the speculative queue. A plugin that genuinely cannot be pinned still fails rather than being hashed in part. A second load of changed content in one process is refused: the ESM cache is keyed by resolved path and Bun ignores the query string, so `import` returns the first module while the fingerprint describes the new bytes — a pack whose rules and whose freeze disagree, undetectably. Plugins are also imported by resolved path, so a bare specifier cannot select a different file than the one hashed. A configured plugin that registers no pack now fails the run instead of leaving it with fewer rules than tsforge.config.json declares, announced by one line of report output. Rule modules freeze all the way down, since rewriting `meta.messages` or widening `meta.schema` changes what a rule enforces as surely as swapping `create`. Documented, not fixed: only source files are hashed, so a plugin reading its severities from a JSON file beside it can still change behavior with no digest change. Knowing which data files a plugin reads is not something the import graph can answer.
Round 3 passed, so none of these blocked; all four are real. A relative import of a data file was dropped from the graph. `candidatePaths` speculated code spellings for anything whose extension was not in CODE_EXT, so `import severities from "./severities.json"` looked for `severities.json.ts` and found nothing — a pack's own severity table, imported and editable, outside the freeze. Any specifier that already names a file now resolves to that file; guessing is for extensionless specifiers only. `freezeRulePack` re-read `rulesConfig` after `isRulePack` had validated an earlier read. A getter can answer the two reads differently, and the second read is the one that becomes the registered pack — so the pack that runs could be weaker than the pack that passed validation, with no on-disk change for the fingerprint to catch. The copy now re-validates as it goes, through a widened view, since the declared union describes what the validator saw rather than anything the object is obliged to keep saying. Registration happened before the "did every configured plugin produce a pack" check, so a partial failure threw with the registry already populated: a caller that catches proceeds with exactly the packs the error was meant to stop. The check now runs first. The queue bound was applied once per file, after `enqueueImports` had walked every specifier in it — one file can hold as many imports as someone cares to write, each queuing 16 spellings and costing a resolve, so the bound was checked only after it had been blown past. It is now checked per specifier. Also: two test comments and one test name described the opposite of what they asserted, or numbers from before the limits were raised.
Both were introduced by the previous commit; two reviewers agreed on each. `IMPORTED_AT` was recorded BEFORE the post-import re-hash, so refusing a self-rewriting plugin left the entry holding the pre-swap digest. The refused module had already run, and the ESM cache keeps it for the life of the process — so restoring the original bytes made the digest match again and admitted exactly the module the refusal was about. A rejected entry is now marked refused rather than fingerprinted, and no content can produce that marker, so the path cannot load again in this process. Treating any extension-bearing specifier as literal-only broke NodeNext. TypeScript sources import each other by the extension of the emitted file, so `./dep.js` is how a plugin refers to `dep.ts` and no `.js` file ever exists — the real source fell out of the graph. Emitted extensions now also queue the TypeScript spellings they stand for. Re-validating `rulesConfig` during the freeze only narrowed the double-read problem to INVALID severities: a getter answering "error" to the validator and "warn" to the copy is two valid reads and passes. Plugin exports are now snapshotted into plain data before validation, so there is no second read to disagree with the first. Also: the freeze-limit message named files and bytes even when the speculative queue bound was what tripped, and the reload refusal said "restart the session" for something scoped to the process.
A rule can be an accessor. `Object.freeze` makes an accessor property
non-configurable and leaves the getter running, so `rules["no-foo"].create`
could still hand ESLint a different function on every read — and since the file
on disk never changes, no fingerprint sees it. The snapshot only reached the
pack shell, so the rule modules under it were still the plugin's live objects.
Plugin exports are now copied into frozen plain data, reading each property
once. That fixes a second problem in the same place: freezing in place reached
whatever the plugin's objects happened to reference — a shared constant, a
library object from `@typescript-eslint/utils`, any singleton it imported —
and froze things belonging to someone else. Functions stay by reference; a
rule's `create` is the implementation.
The "configured plugin registered no pack" check compared plugin PATHS, so
`{ path, packs: ["strict", "extra"] }` exporting only `strict` satisfied it and
`extra` — a typo or a renamed export — stopped being enforced with no failure.
It now checks each declared name.
Reading an export runs the plugin's getters, and that happened outside the
per-plugin catch, so one throwing getter took down every other plugin's load.
Each candidate export is now guarded and named in its own failure.
Also: drift errors carry the original as `cause`, since "cannot be verified" and
"the entry is gone" are different problems with different fixes.
…match
The re-hash after import can THROW rather than mismatch — the entry deleted, or
grown past the freeze limits. That went to the outer catch, which reports
"failed to load" and knows nothing about the marker, so the entry kept its
pre-import fingerprint while the module it rejected sat in the ESM cache for the
life of the process. Restoring the original bytes then made the digest match and
re-admitted it. The entry is now marked the instant the module runs and cleared
only once every check has passed, so the failure paths cannot miss it.
Materialization was rebuilding every object from its own enumerable string keys,
which is not what an object is. Non-enumerable properties and symbol keys were
dropped, and anything with a prototype — a RegExp in a schema, a Map, an instance
of the plugin's own class — came out as `{}` with its methods gone. Non-plain
objects are now passed through as they are (they are leaves in practice, and a
leaf we pass through beats a rule we silently break), and plain objects are
rebuilt from every own key.
Enumerating a module namespace and indexing into it both run plugin code, and
that happened in a for-of header outside every catch — so one throwing getter
ended the load for every plugin after it too.
One round-6 finding is not fixed because it is not real: the panel reported that
the accessor test passes with its fix reverted. Reverting materialization and
running it turns it red, so it does falsify.
Round 7 was right that the accessor tests read `create` once, which passes
against a live getter as long as the test is its first reader. They read twice
now. That exposed the real problem: a rule written as `class R { get create() }`
keeps its getter on the PROTOTYPE, so no copy of the object graph pins it —
copying own keys drops it, and copying the prototype with it hands the getter
straight back.
A rule module is `{ meta, create }`. Reading those two properties once and
building that object pins the rule whatever shape it arrived in, and needs no
guesses about which parts of an arbitrary graph are safe to rebuild. The graph
copy stays for `rulesConfig` and `meta`, which are data by ESLint's own
contract.
`IMPORTED_AT` was keyed on the lexical path from the config while both things it
guards key on the real one: the fingerprint resolves symlinks before hashing,
and the module cache is keyed by resolved URL. Two spellings of one entry were
two keys, so a refusal recorded against the first did not cover the second. The
record and the registered entry path are now the real path.
The write path's rethrow is covered end to end: a drift during a write ends the
send rather than being swallowed. That was worth proving rather than asserting —
the rethrow is only load-bearing if every caller above it propagates, and those
callers are not in this diff.
The freeze exists because a plugin FILE edited mid-session could quietly weaken the gate. The content fingerprint enforces that, and it is the part that works. The materialization added over the last rounds was aimed at a different thing: a plugin that hands back one set of rules when it is inspected and another when it is used. That cannot be closed here. The plugin's code runs in this process, so it decides what to answer, and every shape pinned so far — a getter on the pack, a getter on a rule, an accessor on a prototype — has another shape behind it. It also buys little: whoever writes the plugin is whoever configured it to load, and they can ship weak rules openly. What stays is what the audit asked for and what tests can hold: the fingerprint covering the editable surface a plugin executes, re-verification on every path that applies external rules, and failing closed when anything cannot be verified.
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.
Summary
Closes audit F19: workspace-local external plugins could change on disk mid-session under the same pack id. Content is now frozen at load and re-checked before the gate and write-time lint.
plugin-fingerprint.ts)rulesConfigon registrationassertExternalPacksFrozen()hard-fails on driftcommandGate, interactive auto-gate runner, andmakeFileLinterNote: external pack implementations still do not load inside the eslint gate subprocess (deliberate — avoids
process.exit(0)from untrusted workspace code). Orchestrator-side freeze covers the registered packs used for write-time lint and the gate cycle entry.Test plan
bun test packages/core/tests/external-plugins.test.ts(incl. mid-session edit → throw)