fix: a crashed prompt test is not a leaking prompt filter - #348
Merged
Conversation
The effective-prompt harness imports dm-context-filter.ts directly. node cannot load TypeScript — it exits ERR_UNKNOWN_FILE_EXTENSION before rendering a single prompt — and the runner reported that crash as "dm-context-filter may be leaking banned phrases". So h44lacrosse.com warned about a possible prompt leak on every single upgrade, for months, while the filter was clean. Verified under bun on that host: OK — 2 scenarios passed, 0 leaks. An alarm that is always wrong is worse than silence, because it teaches everyone to scroll past the one time it is right. I scrolled past it a dozen times today before reading it. Same shape as the declare -F guard fixed in #343: a failure being reported as something it is not. There the error was silent, here it is loud and inaccurate. Both come from a caller assuming it knows why the thing below it failed. The runner now resolves a runtime that can actually import TypeScript — bun on PATH, then the service home, then root's, because a service-identity migration leaves the toolchain behind while the upgrade still runs as root, which is precisely h44's shape. When none exists it says the filter is UNVERIFIED and explicitly disclaims alleging a leak, rather than guessing. A real failure with a working runtime still reports as a possible leak, which is the case that message was written for.
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.
The alarm was always wrong
h44lacrosse.com has warned this on every upgrade:
It isn't. The filter is clean — verified on that host under
bun:The harness imports
dm-context-filter.tsdirectly, and node cannot load TypeScript:It exits before rendering a single prompt. The runner is
if node run.mjs; then ... else "may be leaking banned phrases", so a crash, a missing dependency, or a syntax error all get reported as a possible security leak.Why this matters more than the bug
An alarm that is always wrong is worse than silence, because it teaches everyone to scroll past the one time it's right. I scrolled past this a dozen times today before actually reading it — on a live payments site, in a warning about the agent's prompt filter.
It's the same shape as the
declare -Fguard fixed in #343: a caller assuming it knows why the thing below it failed. There the error was silent; here it's loud and inaccurate. Both are worse than an honest "I could not tell."The fix
Resolve a runtime that can actually import TypeScript —
bunon PATH, thenSERVICE_HOME, then root's. That last one matters: a service-identity migration leaves the toolchain behind while the upgrade still runs as root, which is exactly h44's shape (bunat/root/.bun, not on the upgrade's PATH).When none exists:
Unverified, explicitly disclaiming a leak. Same principle as
verify.sh: an invariant that could not be checked has not been shown to hold, and must not be reported as either passing or failing.A genuine failure with a working runtime still reports as a possible leak — that's the case the message was written for.
Coverage
Added to
tests/bridge-render.sh: prefersbunon PATH, falls back to the service home, returns nothing when no TS runtime exists, the skip path exists and disclaims a leak, and the harness is no longer invoked under node. The/rootfallback got a test seam so a host with no bun anywhere can be modelled — this dev box really does have/root/.bun, which made the first version of that assertion pass for the wrong reason.