Skip to content

Commit 5bdb456

Browse files
jkyberneeesclaude
andcommitted
fix: skip empty untrusted-content sources in audit aggregation
Self-review follow-up. When unwrapUntrusted/untrustedSource (singular, with an `src != ""` call-site guard) were replaced by untrustedSourcesAll, the empty-source filter was dropped. A wrapper with source="" (reachable when a wrapUntrusted caller passes an empty source, e.g. a browser snapshot with an empty URL) then entered untrustedSources. In the audit divergence check, isSource() does strings.HasPrefix(resource, ""), which is always true, so a single empty-source blob marked every resource as a "source", emptied untrustedResSet, and silently suppressed reused-resource injection detection for the whole turn — defeating the multi-blob hardening the change introduced. Restore the empty-source skip in untrustedSourcesAll and add a regression test. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 56be5d3 commit 5bdb456

2 files changed

Lines changed: 33 additions & 1 deletion

File tree

cmd/odek/untrusted.go

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -219,7 +219,14 @@ func untrustedSourcesAll(s string) []string {
219219
rep := strings.NewReplacer("'", `"`, "‹", "<", "›", ">")
220220
srcs := make([]string, 0, len(matches))
221221
for _, m := range matches {
222-
srcs = append(srcs, rep.Replace(m[1]))
222+
src := rep.Replace(m[1])
223+
// Skip empty sources. An empty source would match every resource as a
224+
// prefix in the audit divergence check (strings.HasPrefix(r, "")), which
225+
// would blind the reused-resource injection heuristic for the whole turn.
226+
if src == "" {
227+
continue
228+
}
229+
srcs = append(srcs, src)
223230
}
224231
return srcs
225232
}

cmd/odek/untrusted_test.go

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,3 +89,28 @@ func TestWrapUntrusted_EmptyInputBypasses(t *testing.T) {
8989
t.Errorf("wrapUntrusted(_, \"\") = %q, want \"\"", got)
9090
}
9191
}
92+
93+
// TestUntrustedSourcesAll_SkipsEmptySource verifies that a wrapper with an
94+
// empty source attribute does not contribute an empty string to the source
95+
// list. An empty source would match every resource via strings.HasPrefix(r, "")
96+
// in the audit divergence check, blinding the reused-resource heuristic.
97+
func TestUntrustedSourcesAll_SkipsEmptySource(t *testing.T) {
98+
// A blob with no source, concatenated with a blob that has a real source.
99+
combined := wrapUntrusted("", "anonymous body") + wrapUntrusted("https://evil.example/x", "named body")
100+
101+
srcs := untrustedSourcesAll(combined)
102+
for _, s := range srcs {
103+
if s == "" {
104+
t.Fatalf("untrustedSourcesAll returned an empty source: %#v", srcs)
105+
}
106+
}
107+
if len(srcs) != 1 || srcs[0] != "https://evil.example/x" {
108+
t.Fatalf("untrustedSourcesAll = %#v, want exactly [https://evil.example/x]", srcs)
109+
}
110+
111+
// Both bodies must still be aggregated (the empty-source blob is not dropped).
112+
bodies := unwrapUntrustedAll(combined)
113+
if len(bodies) != 2 {
114+
t.Fatalf("unwrapUntrustedAll returned %d bodies, want 2: %#v", len(bodies), bodies)
115+
}
116+
}

0 commit comments

Comments
 (0)