Prompt-analysis read failures are discarded, so an unanalyzed prompt can still publish a warning-free result

[davidslater]

Created by GitHub Ace · View Session

Follow-up to #954 / #963.

#963 closed the reporting gap for warnDegradedPromptAnalysis: prompt-analysis degradation now reaches warnings instead of existing only as a ::warning:: annotation. It also added a readability probe in pkg/artifacts so an unopenable prompt.txt or patch is detected at load time rather than passing a stat-only check.

A narrower case survives both fixes. BuildPromptAnalysis discards the error from every file it reads, so a read that fails after artifacts.Load has already passed produces no signal at all.

The three discards

pkg/detector/static.go:

data, err := os.ReadFile(arts.PromptTemplatePath)
if err == nil && strings.TrimSpace(string(data)) != "" { analysis.PromptTemplate = string(data) }

data, err := os.ReadFile(arts.PromptImportTreePath)
if err == nil && strings.TrimSpace(string(data)) != "" { analysis.ImportTree = string(data) }

data, err := os.ReadFile(arts.PromptFilePath)
if err == nil { rendered = string(data) }

The first two are now adequately covered: a failure there leaves PromptTemplate/ImportTree empty, which warnDegradedPromptAnalysis reports as "Missing or unusable". It cannot distinguish unreadable from absent, but it does say so.

The third is not covered. A failed read of the rendered prompt yields rendered = "", and nothing reports it.

Reproduction

Stage a readable prompt.txt, prompt-template.txt and prompt-import-tree.json, let Load succeed, then make prompt.txt unreadable before BuildPromptAnalysis runs:

loader warnings at Load time:  0
PromptTemplate present:        true
ImportTree present:            true
UntrustedInputsIndeterminate:  true
eligibility prompt_injection:  true
total loader warnings:         0

Zero warnings. The loader's probe already passed, and warnDegradedPromptAnalysis returns nothing because the template and import tree are both present — it only fires on those two. So the run publishes a detection_result.json with warnings: [] even though the rendered prompt was never read and untrusted-input extraction never ran.

Severity

This is a reporting gap, not a verdict fail-open. An empty rendered takes the else branch in BuildPromptAnalysis and sets UntrustedInputsIndeterminate = true, which TD-10g's eligibility maps to uninspectable: true, keeping prompt_injection eligible. So a staging failure still cannot suppress a genuine finding — that half works correctly.

What is lost is exactly what #954 set out to make visible: the host reading the uploaded result cannot tell that the prompt channel went unexamined. It looks like a clean, complete inspection.

Reachability is narrow after #963 — it needs a TOCTOU window between Load and BuildPromptAnalysis, or a non-permission I/O error (media error, a file replaced or truncated mid-run, an NFS/overlay hiccup). It is not reachable by a simple chmod 000 before the run, since the loader probe now catches that. Low likelihood, but it is the one remaining path by which the detector can analyze less than it reports.

Suggested fix

Have BuildPromptAnalysis surface its read failures rather than discarding them, and route them into Artifacts.Warnings (or into the reported set assembled in run()) so they land in warnings like every other degraded-inspection condition. TD-10h already requires this in general terms:

Every degraded-inspection condition the implementation reports as an annotation MUST also be recorded in warnings when a result is written.

Two constraints carried over from #963:

• Keep the classification right. prompt-template.txt and prompt-import-tree.json are optional, so their findings must stay RequiredInput: false or TD-18c will promote them and strict mode will start refusing runs of hosts that never staged them. A failure to read prompt.txt, by contrast, concerns a required input.
• Don't double-count into eligibility. Prompt-injection inspectability is already derived from the analysis fields directly; adding a second signal for the same condition would be redundant.

Distinguishing "unreadable" from "absent" in the template and import-tree messages would be a worthwhile side effect — the current wording has to hedge with "Missing or unusable" precisely because the error is thrown away.

Work

• Return or record read errors from the three os.ReadFile calls in BuildPromptAnalysis
• Surface them in warnings, with prompt as a required input and the two optional files advisory
• Split the "Missing or unusable" message into distinct absent vs unreadable diagnostics
• Test: a rendered-prompt read failure with template and import tree present produces a warning, still exits 0, and does not change the verdict