Repository navigation
fix(mothership): redact file text before grep clips or splits it - #8929
waleedlatif1 wants to merge 1 commit into
Conversation
Model egress redacts whole secrets only, so grep's 2,000-character line clip and its per-line split could hand it a fragment of a known secret it cannot recognize. Grep now redacts each resource's full text through the invocation's provenance before splitting, matching and clipping, and the clip never cuts through a redaction marker. JSON and JSONL parse errors no longer quote a clipped excerpt of the file.
There was a problem hiding this comment.
2 issues found across 6 files
Confidence score: 2/5
- In
agent-cli/types.ts, grep can run without provenance, andredactForModelleaves text unchanged when the registry is absent, so clipped output can expose secret fragments. Fail closed when provenance is missing. - In
json-parser.ts,describeSyntaxErrorcan still send the first character of a secret to Chat in V8’sUnexpected tokenprefix. Return a generic syntax diagnostic instead.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/sim/lib/mothership/agent-cli/types.ts">
<violation number="1" location="apps/sim/lib/mothership/agent-cli/types.ts:44">
P1: This optional field lets grep run without provenance, and `redactForModel` returns text unchanged when the registry is absent, so clipping can emit secret fragments. Fail closed when provenance is missing or require and populate the registry before this engine runs.</violation>
</file>
<file name="apps/sim/lib/file-parsers/json-parser.ts">
<violation number="1" location="apps/sim/lib/file-parsers/json-parser.ts:225">
P2: `describeSyntaxError` removes the quoted excerpt but leaves V8’s `Unexpected token 's'` prefix, so malformed JSON starting with a secret still sends its first character to Chat. Return a generic syntax diagnostic instead.</violation>
</file>
Heads up: you’ve reached your flex budget. Increase your flex budget or wait for usage to reset.
Turn on auto-fix | Re-trigger cubic
| * The invocation's secret provenance, which model egress redacts the result with. Egress | ||
| * matches whole secrets, so an engine that clips or splits text it read redacts it here first. | ||
| */ | ||
| resolvedSecretTraceRegistry?: ResolvedSecretTraceRegistry |
There was a problem hiding this comment.
P1: This optional field lets grep run without provenance, and redactForModel returns text unchanged when the registry is absent, so clipping can emit secret fragments. Fail closed when provenance is missing or require and populate the registry before this engine runs.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At apps/sim/lib/mothership/agent-cli/types.ts, line 44:
<comment>This optional field lets grep run without provenance, and `redactForModel` returns text unchanged when the registry is absent, so clipping can emit secret fragments. Fail closed when provenance is missing or require and populate the registry before this engine runs.</comment>
<file context>
@@ -36,6 +37,11 @@ export interface AgentCliRuntime {
+ * The invocation's secret provenance, which model egress redacts the result with. Egress
+ * matches whole secrets, so an engine that clips or splits text it read redacts it here first.
+ */
+ resolvedSecretTraceRegistry?: ResolvedSecretTraceRegistry
signal?: AbortSignal
}
</file context>
| * through a secret would carry the part that survived the cut. | ||
| */ | ||
| function describeSyntaxError(error: unknown): string { | ||
| return getErrorMessage(error, 'Unknown error').replace(QUOTED_INPUT_EXCERPT, '') |
There was a problem hiding this comment.
P2: describeSyntaxError removes the quoted excerpt but leaves V8’s Unexpected token 's' prefix, so malformed JSON starting with a secret still sends its first character to Chat. Return a generic syntax diagnostic instead.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At apps/sim/lib/file-parsers/json-parser.ts, line 225:
<comment>`describeSyntaxError` removes the quoted excerpt but leaves V8’s `Unexpected token 's'` prefix, so malformed JSON starting with a secret still sends its first character to Chat. Return a generic syntax diagnostic instead.</comment>
<file context>
@@ -213,6 +213,18 @@ export function stripJsonComments(text: string): string {
+ * through a secret would carry the part that survived the cut.
+ */
+function describeSyntaxError(error: unknown): string {
+ return getErrorMessage(error, 'Unknown error').replace(QUOTED_INPUT_EXCERPT, '')
+}
+
</file context>
| return getErrorMessage(error, 'Unknown error').replace(QUOTED_INPUT_EXCERPT, '') | |
| return 'Invalid JSON syntax' |
|
| ) | ||
| continue | ||
| } | ||
| const lines = text.split('\n') |
There was a problem hiding this comment.
Replacing a multi-line secret with one marker removes its newlines before grep numbers the lines. In the new fixture, searching for config end reports line 3 instead of line 6. Every later match shifts too, and context windows cover different source lines. Following a reported location can therefore read the wrong part of the file.
Keep redaction before matching, but preserve the original line positions or map the redacted positions back to them.
|
Closing: this fix is being consolidated with the other file-provenance fixes into a single PR with one shared abstraction. The branch is kept for reference. |
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Summary
grepclipped each line at 2,000 characters and split files into lines before egress ran, so a known secret that straddled the clip leaked its prefix, and each line of a multi-line secret (a PEM key, for example) crossed as an unrecognizable fragment.grepnow redacts each resource's full text through the invocation's provenance (the same propagated set egress uses) before splitting, matching and clipping. The clip also backs off to the start of a redaction marker, so it never leaves half a{{NAME}}or[REDACTED_SECRET].grep --count <guess>can no longer be used to confirm a secret value one prefix at a time.resolvedSecretTraceRegistry.Type of Change
Testing
file-provenance.test.tsrun through the real CLI handler andinspectToolResultForCopilot: a secret straddling the clip, a secret whose marker would straddle it, a secret wholly past it, several secrets around the cut, and a multi-line secret. They check that no 4-character prefix or suffix and no partial marker survives. The straddling, marker, multiple-secret and multi-line cases fail on the old code; the wholly-past case passed before too and guards the boundary.json-parser.test.tscases: invalid JSON and JSONL never quote the file. Both fail on the old code.bunx vitest run lib/mothership/agent-cli lib/file-parsers lib/workspace-filespasses (1,120 tests).bun run testpassed except 3 files that hit 10s/60s timeouts under full parallel load. Those 3 files pass when run alone and are unrelated.bun run lint,bun run type-check,bun run check:audits(59 audits), and the block-registry check all pass.Checklist
test-auditauthoring gate)