Skip to content

💥 Reject secrets before the journal by default - #329

Merged
taras merged 4 commits into
mainfrom
feat/issue-199-secret-policy
Aug 5, 2026
Merged

💥 Reject secrets before the journal by default#329
taras merged 4 commits into
mainfrom
feat/issue-199-secret-policy

Conversation

@taras

@taras taras commented Aug 4, 2026

Copy link
Copy Markdown
Owner

Why

Journals carry prompts, tool results, process output, and component captures. #198
built the pre-persistence boundary and #199's first slice built the offline scanner,
safe findings, and the reusable file scan — but nothing installed them, and the
specification still said the opposite of what should be true: "Journal policy lives
outside execute."

This is #199's second slice. It makes secret detection the default policy of a
programmatic execution and gives trusted runtime packages an authenticated read of it.
#199 stays open for the CLI opt-out and the remaining integration slices.

What changes

Before: execute() journaled whatever a document produced. A host that wanted
credentials refused had to wrap its own stream with guardDurableStream and bring its
own gate.

After: every execution refuses to persist a durable event carrying a credential. The
journal is selected before the durable run starts, so the root component import is
already covered. secretDetection: false from the trusted host is the only way off —
💥 this is a behavioral break for every existing caller of execute(), including
xmd.

How it works

execute(options) → useSecretDetection(request, stream) → guarded stream → durableRun
                        └→ private policy bound for the execution's own scope

useSecretDetection normalizes the request (requested !== false), builds exactly one
scanner when enabled, binds the private policy, and returns the stream to journal
through. execute receives only that stream — never the policy.

The settled rule is that document code cannot disable, mutate, counterfeit, retain, or
substitute an execution's detection. It has two independent parts, and each needs its
own mechanism:

The journal. The gate is a closure over the private scanner, held by the stream
durableRun already has. It reads no context after installation, so nothing a document
does to contextual state can change what the journal is held to.

The contextual surface. Effection resolves contexts by name
(buildScopeInternal stores Object.create(parent.contexts) and reads
contexts[context.name]), so any code can construct the same name and bind something
else for its own descendants — keeping the descriptor module-private proves nothing.
What is authenticated is the value: a private class field no other module can
produce. A counterfeit therefore fails the read instead of downgrading it. A name
collision can deny detection information; it can never weaken detection.

The private policy never leaves the module — a brand proves module origin, not
execution ownership, so an authentic value that escaped could be replayed into a later
execution. secretPolicy() answers with a frozen description carrying only enabled,
and scanSecrets() resolves the running execution's scanner each time it is
interpreted.

Review guide

Start with: packages/core/src/secrets/policy.ts — the module docs state the
threat model the rest of the file implements.

Then review:

  1. SecretPolicy, secretPolicy(), scanSecrets() — the whole public surface.
  2. useSecretDetection() and guardWithSecretDetection() — normalization, one
    scanner, and the gate over the unchanged generic guard.
  3. packages/core/src/secrets/secretlint.ts — the Secretlint boundary and why the
    profiler is stubbed there rather than per run.
  4. packages/core/src/execute.ts — where the journal is selected, and why there.
  5. specs/executable-mdx-spec.md §7/8.1.

Look carefully at:

  • ExecutionDetection.authentic() — the object guard exists because #brand in value
    throws on a primitive rather than answering false.
  • The disabled path builds no scanner and returns the original stream untouched.
  • scanSecrets is a generator so the policy resolves when the operation runs, not when
    it is created.

What must stay true

  • The gate never consults context after installation — enforced by the gate being a
    closure over the scanner; checked by does not weaken the journal gate when the
    context is counterfeited
    , which counterfeits the binding, proves the read it governs
    fails, and shows the canary still rejected.
  • Only the host request disables detection — enforced by requested !== false over
    a value only ExecuteOptions carries; checked by the three host-only control tests
    (root prop, frontmatter, component-set context) and disables detection only for an
    explicit host false
    .
  • No private value escapes — enforced by returning frozen descriptions; checked by
    hands back a frozen description carrying nothing but enabled, rejects a description
    retained from an earlier execution
    , and leaves no retained value able to scan.
  • One scanner per execution — enforced by building it once in useSecretDetection;
    checked by creates one scanner per execution and gives concurrent executions
    distinct scanners and fingerprint keys
    .
  • Replay does not rescan — enforced by guardDurableStream delegating readAll;
    checked by does not rescan a journal it replays.
  • Nothing leaks the matched value — enforced by slice 1's normalization; checked by
    leaks the canary through no completion, output, or persisted channel and the
    detector-failure test.

How to verify it

Canaries are assembled at run time from parts, so no usable-looking literal enters the
repository and repository scanning does not detect the fixtures themselves.

  • Rejects a canary in the root import event proves the gate precedes the first
    durable event, and fails if the guard is installed after durableRun starts.
  • Admits the later safe close as an independent append proves a rejection is
    per-event, and fails if a rejection were treated as emptying the journal.
  • Agrees with the journal gate on the same execution's scanner compares fingerprints
    — HMACs under a per-scanner key — so it fails if the gate and scanSecrets ever use
    different scanners.
  • Resolves its policy when interpreted, not when created builds the operation inside a
    live execution and runs it after teardown; it fails if the policy is captured early.
  • Fails closed when the detector itself fails proves an unusable detector is not an
    open gate, with no raw cause and no canary in the failure.

Mutation evidence

Fourteen mutations, each applied to a file copy of the implementation, focused suite run,
then restored. All fourteen are killed:

Mutation Reddens
omit the guard 12 tests, incl. all four persistence-boundary assertions
install it after durable work begins 5, incl. keeps the offending import event out of the backend
create a scanner per append 3, incl. creates one scanner per execution
ignore explicit false 3, incl. creates no scanner and installs no guard
rescan replayed entries does not rescan a journal it replays
raw detector error / source escapes fails closed when the detector itself fails
return the private policy from secretPolicy() 5, incl. hands back a frozen description
expose a scanner-bound closure publicly 4, incl. leaves no retained value able to scan
accept an unbranded context value 3, all counterfeit tests
accept a public description as the internal policy 3, all counterfeit tests
capture the policy when scanSecrets() is created 4, incl. resolves its policy when interpreted
leave the shared profiler unstubbed scans without emitting profiler marks
swap the shared profiler per run instead of once never changes the shared profiler while executions overlap
disabled scanSecrets() returns clean findings throws rather than reporting clean content

Local results

Red-first: the suite was written against the final API and run before any
implementation existed — Cannot find module .../secrets/policy.ts plus five
'secretDetection' does not exist in type errors.

  • deno task fmt / lint — 0 errors (1096 pre-existing warnings, unchanged)
  • deno task check — clean
  • deno task test350 passed, 0 failed
  • deno task check:jsrSuccess Dry run complete
  • pnpm exec tsc --project tsconfig.node.json --noEmit — clean
  • git diff --check — clean

Node, Bun, and the compiled-binary smoke run as their own CI jobs on this PR — all green.

Cost

Default-on means every execution scans its root source and every event. The first
measurement showed +29 s user CPU over the suite (+14%). The cause turned out not to be
scanning, and fixing it removed the regression.

user CPU tests
base 511776e 207.19 s / 206.90 s 349
detection on, profiler as shipped 238.16 s / 235.35 s 350
detection on, profiler stubbed 207.34 s / 212.66 s / 216.39 s 350

@secretlint/profiler exports a process-global singleton that every lintSource() call
marks through — 66 marks and 33 measures per scan for this rule set. A
PerformanceObserver pushes each mark into an array nothing ever clears, and for every
::end mark it scans that array for the matching start and discards the result. Dead
work, linear in every mark the process has emitted, so each scan costs more than the one
before it. Measured over buckets of 100 scans with one scanner: 1.19 → 2.09 → 1.54 →
2.13 → 6.68 → 10.52 ms, mark count past 39,000. Stubbed, flat at 0.045 ms.

This is also what broke test-bun. Bun runs a whole corpus in one process, so file
b starts with file a's marks still counted (measured: 3366 → 6732 across two files);
Deno gives each file its own process and starts at 0. By the time use-testing.test.ts
ran, scans were slow enough to blow that test's 200 ms budget — which is why only Bun
failed, and failed consistently while main passed 8/8. No test was modified.

The mechanism. packages/core/src/secrets/secretlint.ts is the only module that
reaches Secretlint. It replaces secretLintProfiler.mark with a no-op once, at module
evaluation, and never restores it
, then re-exports lintSource — so the stub is in
place before any scan runs and the used export keeps the module from being dropped by a
bundler. Nothing switches per run, so nothing can observe a switch and no execution owns
any part of it.

Alternatives were measured and rejected. Clearing the performance timeline leaves the
array alone — it belongs to the profiler. An install-time npm override reaches Node but
not Deno (0 marks against 62 in the same probe), and would only apply to this
repository's builds: anything installed from JSR or npm would resolve the real package
and carry the defect. Replacing the method in source travels with the code, so published
consumers run the same stub.

The per-scan baseline recorded in slice 1 is unchanged and no threshold was added.

No corpus false positives. All 349 pre-existing tests pass unmodified: a finding
throws and rejects the append, so a green suite is direct evidence that no existing
fixture is credential-shaped.

Scope

Included

  • secretDetection?: boolean on ExecuteSettings, normalized in one place.
  • Default-on gate installation before durableRun.
  • secretPolicy() and scanSecrets(), plus the private policy they read.
  • Stubbing Secretlint's profiler at the dependency boundary — required work, not a
    follow-up: default-on scanning is what makes that defect expensive, and it is what
    broke test-bun.
  • Specification §7/8.1.

Intentionally unchanged

  • guardDurableStream stays generic and policy-neutral — the secret policy is a gate
    it is given, not behavior inside it.
  • scanFiles() is untouched. Future Workflow code composes it as
    scanFiles(root, { scan: scanSecrets }) without receiving a scanner.
  • No CLI --no-secret-detection, CLI warning, or run/test option plumbing — the
    next Reject secrets before journal persistence by default #199 slice. xmd inherits the new default because it calls execute().
  • No snapshots or Git objects, allowlists, sanitization, repair, approval, elicitation,
    or general append middleware.
  • site/routes/docs/journal.tsx documents the generic library guard and stays true.

New abstractions

  • SecretPolicy exists because trusted runtime packages must branch on whether an
    execution scans, and the future snapshot path is its consumer. It carries enabled
    and nothing else.
  • The private ExecutionDetection hierarchy exists because a name-keyed context cannot
    authenticate its own value; the brand is what a counterfeit cannot reproduce.
  • secrets/secretlint.ts exists to give the package one place that reaches Secretlint,
    so the profiler stub is applied before any scan and the import cannot be dropped.
  • Each new abstraction has multiple concrete uses or a clear justification.
  • No speculative functionality is included.

New dependencies

  • Package: @secretlint/profiler@13.0.4
  • Used for: reaching the profiler singleton @secretlint/core already marks through, to
    replace its mark with a no-op once at import.
  • Why existing dependencies are insufficient: @secretlint/core imports the singleton
    directly and offers no injection point, and it is already in the tree at this exact
    version as a transitive dependency of @secretlint/core.

Risks and limitations

  • Breaking by design. Any host relying on execute() journaling credential-shaped
    content must now pass secretDetection: false. Rollback is that one flag.
  • A document can deny its own contextual read by binding the policy name — the read
    fails closed. It cannot make the read report a weaker policy, and the journal is
    unaffected either way.
  • The profiler stub replaces a method on a package-global once at import. It is not
    scoped to an execution and is never restored, which is what keeps it out of execution
    lifetime — no run owns it, none of them coordinate over it, and mark is identical by
    reference before, during and after overlapping runs (asserted). The cost is that a
    process importing this package has Secretlint profiling off; nothing reads it, since it
    exists for Secretlint's --profile CLI. Worth reporting upstream: the entries array
    is unbounded and the find whose result is discarded is pure waste.

Scope confirmation

  • Every changed file supports the purpose described above.
  • Unrelated cleanup and formatting changes are excluded.
  • Generated or mechanical changes are clearly identified — there are none.
  • The description matches the final diff and test results.

Every programmatic execution now scans each durable event before it
persists. `execute()` selects its journal before `durableRun` starts, so
the root component import is already covered, and `secretDetection: false`
from the trusted host is the only way off.

The rule is that document code cannot disable, mutate, counterfeit,
retain, or substitute an execution's detection, and it has two parts. The
journal gate is a closure over the execution's private scanner and reads
no context after installation, so nothing a document does to contextual
state can reach it. The contextual surface authenticates instead: Effection
resolves contexts by name, so any code can bind the same name, and what
proves a policy genuine is a private class field no other module can
produce. A counterfeit therefore fails the read rather than downgrading it.

The private policy never escapes. `secretPolicy()` answers with a frozen
description carrying only `enabled`, and `scanSecrets()` resolves the
running execution's scanner each time it is interpreted — so an operation
built during a run and performed after it has ended finds no policy and
fails, and no retained value can scan.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Found 3 redundant comments. Inline suggestions to remove them below.

Comment thread packages/core/mod.ts
// The running execution's policy. Read-only by construction: what a caller can
// reach is a detached description and an execution-bound scan. The policy
// itself, its scanner, and the context it is bound in stay private, so nothing
// here can disable, replace, or outlive the detection an execution runs under.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant comment — restates what the code does.

Suggested change
// here can disable, replace, or outlive the detection an execution runs under.

// document, frontmatter, prop, component, or eval code exists — so the
// root component import is already behind the gate. What comes back is
// the stream to journal through; the policy itself stays inside the
// execution that owns it.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant comment — restates what the code does.

Suggested change
// execution that owns it.

abstract readonly enabled: boolean;

static authentic(value: unknown): value is ExecutionDetection {
// `#brand in value` throws on a primitive rather than answering false.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant comment — restates what the code does.

Suggested change
// `#brand in value` throws on a primitive rather than answering false.

@github-actions

github-actions Bot commented Aug 4, 2026

Copy link
Copy Markdown

PR #329: 💥 Reject secrets before the journal by default

12 files, +1148 / -11

Scope

🔴 PR has 1159 lines changed. Split into focused PRs.

🟡 1159 lines changed. PRs under 400 receive more thorough review.

🟡 PR mixes config and source changes.

Structural

✅ No structural bloat detected.

Slop

✅ Slop indicators look low.

Static Analysis

✅ Oxlint found no issues.

Correctness

No extraneous code patterns detected.

taras added 3 commits August 4, 2026 18:47
Default-on scanning made a defect in `@secretlint/profiler` expensive.
The package exports a process-global singleton that every `lintSource()`
call marks through — 66 marks and 33 measures per scan for the rule set
this package pins. A PerformanceObserver pushes each mark into an array
nothing ever clears, and for every `::end` mark it scans that array for
the matching start and discards the result. The scan is dead work, and it
is linear in every mark the process has emitted, so each scan costs more
than the one before it.

Measured over buckets of 100 scans, one scanner: 1.19 → 2.09 → 1.54 →
2.13 → 6.68 → 10.52 ms per scan, with the mark count passing 39,000.
Silenced, the same measurement is flat at 0.045 ms. Clearing the
performance timeline does not help — the array belongs to the profiler.

The silence is bound to the execution rather than switched on for the
process: what it quiets is global, so the last run using it puts back what
the first one found. A counter is what makes that correct when executions
overlap, which they do — a run that ended would otherwise un-silence one
still going.

The suite's cost returns to where it started: 207.3s user CPU against
206.9s on the base commit, from 235-238s before this.
The counter was module state, which the state-ownership rule does not
allow, and it turned out not to be needed. A run that finds the profiler
already silent leaves it alone and restores nothing, so exactly one run
ever holds the real `mark` and the process ends as noisy as it started
however the runs interleave. What that run found lives in its own frame
and goes away with it.

The cost is that a run finishing while another is still going hands
profiling back early. The other run pays for that in speed and never in
correctness, which is the right side to fail on for bookkeeping nothing
reads.
Replacing the profiler for an execution's lifetime was shared mutable
state: it changed a process-global that no execution owns, concurrent runs
had to agree about who restored it, and an unrelated Secretlint user in
the process would see profiling appear and disappear under them.

The replacement happens once, where this package imports Secretlint, and
never again. `secretlint.ts` is now the only module that reaches
`@secretlint/core`, and it re-exports `lintSource` so the stub is in place
before any scan runs and no bundler can drop it. Nothing switches per run,
so nothing can observe a switch, and no execution owns any part of it.

Install-time alternatives were measured and rejected. An npm override
reaches Node but not Deno — 0 marks against 62 in the same probe — and
would only apply to this repository's own builds, so anything installed
from JSR or npm would resolve the real package and carry the defect.
Replacing the method in source travels with the code, so a consumer of
this package runs the same stub we do.

Two tests replace the lifecycle ones: `mark` is identical by reference
before, during and after overlapping executions, and a run emits no
profiler marks at all. Both are discriminating — the first reddens against
a per-run swap, the second against no stub.
@taras
taras merged commit 209bf21 into main Aug 5, 2026
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant