QA follow-up: fix(qa): sanitize evidence_field diagnostics against terminal escape injection (#379) #387

Post-merge QA follow-up (#379)

Source: #379 — fix(qa): sanitize evidence_field diagnostics against terminal escape injection

Lens: evidence + test design. The merge does what #378 asked for the evidence_field line: sanitizeDiagnosticField keeps [A-Za-z0-9_.-] and collapses the rest, the regression case reproduces the escape payload and pins the sanitized rendering, and the focused suites plus go vet are stated as run. It does not close the class the issue names in its second acceptance criterion ('The same rule applies to any future diagnostic fields derived from caller-supplied input'): the same code path still prints evidence_detail with %s, and for expected_array rejections that Detail is built from caller-supplied JSON object keys joined with %s, so terminal escape bytes still reach stderr through the sibling diagnostic line. The PR body states that evidence_detail is unaffected 'because it already quotes values with %q', which is not true for the object-keys construction in validateEvidence, and the new test asserts only the field line, so nothing in the suite would catch the surviving path. Test design gap: the new case checks that the sanitized field line is present but never asserts that no raw ESC byte appears anywhere in the stderr stream, which is the property the issue actually asks for.

Blocking findings

• Escape injection survives the fix via evidence_detail built from caller-supplied JSON keys (internal/qapi/evidence.go): validateEvidence builds the expected_array diagnostic as fmt.Sprintf("top-level must be a JSON array of records, not an object with keys: %s", strings.Join(keys, ", ")), where keys are the caller's own JSON object key names, and internal/cli/doctor.go prints that text through fmt.Fprintf(stderr, "evidence_detail=%s\n", evidenceErr.Detail). fix(qa): sanitize evidence_field diagnostics against terminal escape injection #379 sanitizes only evidenceErr.Field and leaves this pair untouched, so the payload class from evidence_field diagnostic prints attacker-controlled JSON key names unescaped (terminal escape hygiene) #378 still lands raw on the operator terminal. This contradicts the merged PR's own claim that evidence_detail 'already quotes values with %q', and contradicts acceptance criterion 2 of evidence_field diagnostic prints attacker-controlled JSON key names unescaped (terminal escape hygiene) #378: 'The same rule applies to any future diagnostic fields derived from caller-supplied input'. The added test case asserts only that the sanitized evidence_field line is present, so the suite stays green with the leak open.
  • Failure scenario: Write an evidence file whose top-level JSON is an object with an escape sequence in a key, e.g. {"\u001b[2J\u001b[31mowned\u001b[0m": 1}, and run cortex qa run --role requirements-analyst --request req.txt --catalog ./catalog --evidence evidence.json. Unmarshalling into []json.RawMessage fails, unmarshalling into map[string]json.RawMessage succeeds, the keys are joined verbatim into Detail, and stderr receives evidence_detail=top-level must be a JSON array of records, not an object with keys: <ESC>[2J<ESC>[31mowned<ESC>[0m — raw ESC bytes, the exact injection evidence_field diagnostic prints attacker-controlled JSON key names unescaped (terminal escape hygiene) #378 was opened to stop.

Acceptance criteria

• Fixed: Escape injection survives the fix via evidence_detail built from caller-supplied JSON keys (internal/qapi/evidence.go). The scenario below no longer reproduces: Write an evidence file whose top-level JSON is an object with an escape sequence in a key, e.g. {"\u001b[2J\u001b[31mowned\u001b[0m": 1}, and run cortex qa run --role requirements-analyst --request req.txt --catalog ./catalog --evidence evidence.json. Unmarshalling into []json.RawMessage fails, unmarshalling into map[string]json.RawMessage succeeds, the keys are joined verbatim into Detail, and stderr receives evidence_detail=top-level must be a JSON array of records, not an object with keys: <ESC>[2J<ESC>[31mowned<ESC>[0m — raw ESC bytes, the exact injection evidence_field diagnostic prints attacker-controlled JSON key names unescaped (terminal escape hygiene) #378 was opened to stop.