Skip to content

Commit f34af2b

Browse files
anandgupta42claude
andcommitted
fix: redact non-SQL content echoed in safety threat messages
The main-only Sanity (Verdaccio) security test 6/15 (`altimate check ../../../../etc/passwd` must not reflect file contents) fails since #1090: surfacing real `ThreatFinding` messages exposed that the engine's `multi_statement` rule quotes the raw "statement type" token verbatim — for non-SQL input that is the file's content (`Disallowed statement type: ROOT:X:0:0:…`), an information-disclosure echo. The sanity job is skipped on PRs, so #1090's CI could not catch it. Fix at the dispatcher boundary (covers CLI, tools, and review): - `EngineCoerce.redactThreatText`: keeps SQL-keyword-like statement types (`DROP`, `ALTER TABLE`) and replaces arbitrary content with `<non-SQL content redacted>` in threat message/detail. - `multi_statement` threats also redact `matched_pattern` — for that rule the pattern IS the raw input line. Injection rules keep their SQL-shaped patterns. Diff-scoping keys stay consistent (base and head redact identically). - Applied in the `altimate_core.safety` handler and the composite check. Tests: unit coverage for the redaction (keyword kept / content redacted / unrelated messages untouched) plus real-engine tests proving passwd-like input no longer echoes through either handler while `DROP` survives in messages. Verified the exact sanity scenario locally: CLI output now shows `<non-SQL content redacted>`. Filed upstream for the engine-side echo: to be linked. Closes #1110 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent da952c1 commit f34af2b

3 files changed

Lines changed: 106 additions & 2 deletions

File tree

‎packages/opencode/src/altimate/native/altimate-core.ts‎

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,20 @@ function fail(error: unknown): AltimateCoreResult {
4242
return { success: false, data: {}, error: String(error) }
4343
}
4444

45+
/** Redact raw-input echoes from a scan result's threats (see EngineCoerce.redactThreatText). */
46+
function redactScan(scan: Record<string, unknown>): Record<string, unknown> {
47+
const threats = (scan.threats as any[] | undefined)?.map((t: any) => ({
48+
...t,
49+
message: typeof t.message === "string" ? EngineCoerce.redactThreatText(t.message) : t.message,
50+
detail: typeof t.detail === "string" ? EngineCoerce.redactThreatText(t.detail) : t.detail,
51+
// For multi_statement the matched pattern IS the raw input line — an
52+
// arbitrary-file content echo. Injection rules keep their SQL-shaped
53+
// patterns (useful, and inherently query-derived).
54+
matched_pattern: t.rule === "multi_statement" ? "<redacted>" : t.matched_pattern,
55+
}))
56+
return threats ? { ...scan, threats } : scan
57+
}
58+
4559
// ---------------------------------------------------------------------------
4660
// IFF / QUALIFY transpile transforms (ported from Python guard.py)
4761
// ---------------------------------------------------------------------------
@@ -114,7 +128,7 @@ export function registerAll(): void {
114128
register("altimate_core.safety", async (params) => {
115129
try {
116130
const raw = core.scanSql(params.sql)
117-
const data = toData(raw)
131+
const data = redactScan(toData(raw))
118132
return ok(true, data)
119133
} catch (e) {
120134
return fail(e)
@@ -193,7 +207,7 @@ export function registerAll(): void {
193207
// occurrence, so a PR that ADDS a second identical injection still
194208
// reports it. Recompute safe/risk_score from the surviving threats so a
195209
// fully pre-existing threat set doesn't leave a stale unsafe verdict.
196-
let safety: Record<string, unknown> = toData(core.scanSql(params.sql))
210+
let safety: Record<string, unknown> = redactScan(toData(core.scanSql(params.sql)))
197211
if (params.base_sql) {
198212
try {
199213
const baseCounts = new Map<string, number>()

‎packages/opencode/src/altimate/native/engine-coerce.ts‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,4 +59,23 @@ export function piiColumnsFromReport(piiData: unknown): Array<Record<string, any
5959
return columns.filter((c) => c && c.classification !== "None")
6060
}
6161

62+
/**
63+
* Redact raw input echoed inside engine threat messages.
64+
*
65+
* The engine's `multi_statement` rule quotes the offending "statement type"
66+
* verbatim — for non-SQL input (e.g. `altimate check /etc/passwd`) that
67+
* reflects arbitrary file content back into CLI/tool output. Keep the token
68+
* only when it looks like a SQL keyword phrase; otherwise redact.
69+
*/
70+
const SQL_KEYWORD_LIKE = /^[A-Za-z_][A-Za-z0-9_$ ]{0,31}$/
71+
export function redactThreatText(text: string): string {
72+
return text
73+
.replace(/(Disallowed statement type: )(.+)$/i, (_m, prefix: string, tok: string) =>
74+
SQL_KEYWORD_LIKE.test(tok.trim()) ? `${prefix}${tok}` : `${prefix}<non-SQL content redacted>`,
75+
)
76+
.replace(/(Statement type ')([^']*)(')/i, (_m, a: string, tok: string, c: string) =>
77+
SQL_KEYWORD_LIKE.test(tok) ? `${a}${tok}${c}` : `${a}<non-SQL content redacted>${c}`,
78+
)
79+
}
80+
6281
export * as EngineCoerce from "./engine-coerce"
Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
import { describe, expect, test, beforeAll } from "bun:test"
2+
import { EngineCoerce } from "../../src/altimate/native/engine-coerce"
3+
4+
let coreAvailable = false
5+
try {
6+
require.resolve("@altimateai/altimate-core")
7+
coreAvailable = true
8+
} catch {}
9+
const describeIf = coreAvailable ? describe : describe.skip
10+
11+
describe("redactThreatText", () => {
12+
test("keeps SQL-keyword-like statement types", () => {
13+
expect(EngineCoerce.redactThreatText("Disallowed statement type: DROP")).toBe("Disallowed statement type: DROP")
14+
expect(EngineCoerce.redactThreatText("Statement type 'ALTER TABLE' is not in the allowed list")).toBe(
15+
"Statement type 'ALTER TABLE' is not in the allowed list",
16+
)
17+
})
18+
19+
test("redacts non-SQL content (e.g. /etc/passwd lines)", () => {
20+
const msg = "Disallowed statement type: ROOT:X:0:0:ROOT:/ROOT:/BIN/BASH"
21+
expect(EngineCoerce.redactThreatText(msg)).toBe("Disallowed statement type: <non-SQL content redacted>")
22+
const detail = "Statement type 'ROOT:X:0:0:ROOT:/ROOT:/BIN/BASH' is not in the allowed list: [\"SELECT\"]"
23+
expect(EngineCoerce.redactThreatText(detail)).toContain("'<non-SQL content redacted>'")
24+
expect(EngineCoerce.redactThreatText(detail)).not.toContain("ROOT:X:0")
25+
})
26+
27+
test("leaves unrelated messages untouched", () => {
28+
expect(EngineCoerce.redactThreatText("Tautology attack detected (numeric constant comparison)")).toBe(
29+
"Tautology attack detected (numeric constant comparison)",
30+
)
31+
})
32+
})
33+
34+
// Sanity security test [6/15] regression: `altimate check /etc/passwd` must not
35+
// reflect file contents in threat messages (found by the main-only Verdaccio
36+
// sanity job after PR #1090 surfaced real threat messages).
37+
describeIf("safety handlers redact raw-input echoes (real engine)", () => {
38+
beforeAll(async () => {
39+
process.env.ALTIMATE_TELEMETRY_DISABLED = "true"
40+
const { registerAll } = await import("../../src/altimate/native/altimate-core")
41+
registerAll()
42+
})
43+
44+
const PASSWD = "root:x:0:0:root:/root:/bin/bash\ndaemon:x:1:1:daemon:/usr/sbin:/usr/sbin/nologin\n"
45+
46+
test("altimate_core.safety does not echo non-SQL file content", async () => {
47+
const { Dispatcher } = await import("../../src/altimate/native")
48+
const r = await Dispatcher.call("altimate_core.safety", { sql: PASSWD })
49+
const text = JSON.stringify((r.data as any).threats ?? [])
50+
expect(text.toLowerCase()).not.toContain("root:x:0")
51+
expect(text).toContain("redacted")
52+
})
53+
54+
test("composite check does not echo non-SQL file content in threats", async () => {
55+
const { Dispatcher } = await import("../../src/altimate/native")
56+
const r = await Dispatcher.call("altimate_core.check", { sql: PASSWD })
57+
const threats = JSON.stringify(((r.data as any).safety?.threats ?? []))
58+
expect(threats.toLowerCase()).not.toContain("root:x:0")
59+
})
60+
61+
test("real SQL statement types are preserved in messages", async () => {
62+
const { Dispatcher } = await import("../../src/altimate/native")
63+
const r = await Dispatcher.call("altimate_core.safety", { sql: "SELECT 1; DROP TABLE users;" })
64+
const threats = ((r.data as any).threats ?? []) as any[]
65+
const messages = threats.map((t) => `${t.message} ${t.detail}`).join(" ")
66+
// SQL-keyword statement types stay useful in messages/details
67+
// (matched_pattern is always redacted for multi_statement by design).
68+
expect(messages).toContain("DROP")
69+
expect(messages).not.toContain("redacted")
70+
})
71+
})

0 commit comments

Comments
 (0)