fix(recon,executor): harden JSON parsing, close contamination gaps, fix status_code misparse - #9
Open
iceWhispers wants to merge 4 commits into
Open
fix(recon,executor): harden JSON parsing, close contamination gaps, fix status_code misparse#9iceWhispers wants to merge 4 commits into
iceWhispers wants to merge 4 commits into
Conversation
- _extract_json (handlers.py + validator.py) now scans for every top-level,
bracket-balanced JSON span instead of naive first-`{`/last-`}` slicing, and
picks the final span matching the expected schema (endpoints/findings/
result). Fixes recon_invalid/json_parse_failed false positives when the
model emits prose, markdown fences, a malformed draft, or multiple JSON
objects before the valid final one.
- recon handler: filters extracted endpoints through workspace scope before
writing endpoints.json, and now writes a structured recon/status.json gate
(valid/recon_invalid + structural_errors) that the test stage checks before
running.
- test handler: findings are checked for scope membership and provenance
against recon-derived endpoints; out-of-scope or non-recon-derived findings
are marked CONTAMINATED/false_positive instead of being trusted as-is.
- Add English-language recon/test/validation prompts alongside the existing
Chinese ones.
- Add regression tests for the JSON-parsing fix (clean/fenced/prose-prefixed/
malformed-then-corrected/multiple-candidates/invalid) and for the WAR-RO
contamination scenario (test-60889214).
- .gitignore: ignore engagements/ so real per-target recon/finding artifacts
don't end up in git history.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VtrXzdKuRt5psr7Xd8VrFw
complete_tool_call()'s status-code regex scanned the *entire* tool output for
any bare 3-digit number in [100,599] and took the last match, regardless of
whether a real "HTTP/x.y NNN" status line already matched earlier. On
Cloudflare-fronted targets this reliably misfires: Cloudflare responses
commonly carry an `Alt-Svc: h3=":443"` header, and "443" sorts after the real
status line, silently overwriting it.
Root-caused via finding test-d880dadd (workspace/findings.json, local/
gitignored — not touched here), whose provenance recorded status_code=443
for a war-robr.com.br GET that actually returned 200.
status_code is now only ever set from an unambiguous signal: a genuine
HTTP/x.y NNN status line (last one, so redirect chains still resolve to the
final status), or output that consists of nothing but a 3-digit code (the
`curl -w '%{http_code}'` idiom). Anything else — a port, a byte count, a
header value — is left None rather than guessed.
Adds 6 regression tests in tests/test_executor.py, including one that
reproduces the exact Alt-Svc/443 misfire.
No target was contacted while investigating or fixing this.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VtrXzdKuRt5psr7Xd8VrFw
_provenance_for() now captures the connection port (explicit from the URL, defaulting to 443/80 by scheme) as its own "port" field, instead of leaving port information implicitly conflated with status_code — which is exactly the confusion the executor.py Alt-Svc/443 regex bug produced. findings.py's has_confirmable_provenance() validates the new field the same way it already validates status_code (must be an int in range, not a bool), treating it as optional so existing provenance records without a "port" field still pass. Adds regression coverage: a provenance record correctly separates a real HTTP status from a port number even when both "443" and a status code are present in the same tool output, and a provenance record with no resolvable HTTP status is correctly treated as incomplete rather than guessed. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VtrXzdKuRt5psr7Xd8VrFw
…gaps Reviewed the 5 recently frontmatter-migrated skills (command-injection, deserialization, file-upload, info-disclosure, path-traversal) and the SkillLoader that gates them: - SkillLoader.load() now rejects duplicate YAML keys in a skill's frontmatter (a custom yaml.SafeLoader subclass that raises instead of PyYAML's default of silently keeping the last value), validates description/severity_focus types and values (severity_focus must be one of the real Severity enum values), in addition to the existing name/phase checks. - Fixed a real cross-phase false-positive: duplicate-name detection used to register every scanned file's name globally *before* filtering by phase, so a same-named skill belonging to an unrelated phase could silently exclude the skill actually being loaded for the requested phase. Duplicate detection is now scoped to the phase being loaded. - SkillLoader.issues was write-only — populated but never read anywhere in the codebase, so a skill silently failing to load left zero trace for a human running `cain-agent run`. The recon and test handlers now surface any issues raised while rendering their phase's skills into the existing StageResult caveats/summary mechanism. - All 5 migrated skills had two added section headers (`## 三层测试模型`, `## 证据要求`) with no body content — verified by diffing against the frontmatter migration and confirmed empty in the files on disk. Filled in real, vulnerability-class-specific L1/L2/L3 methodology and evidence-bar content for each, mirroring the depth of the existing sqli skill. The original restrictions (renamed 注意事项 -> 禁止事项, content unchanged) were already intact and are preserved as-is. Adds regression tests: duplicate YAML key rejection, same name reused across different phases is correctly *not* a conflict, invalid severity_focus / non-string description are rejected, and a recon run with a broken skill file surfaces "技能加载问题" in its caveats and summary. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VtrXzdKuRt5psr7Xd8VrFw
cdxiaodong
added a commit
that referenced
this pull request
Sep 5, 2026
- 新增 _jsonspans.iter_json_spans:字符串/转义感知的顶层括号平衡 span
扫描,混合嵌套({a:[1]})正确闭合,未闭合起点放弃
- handlers/validator/orchestration 三处 _extract_json:整体解析优先,
失败后取最后一个可解析 span(模型终稿在末尾)——修复多对象输出
(草稿+终稿)/markdown 围栏/写坏草稿导致的 json_parse_failed 假阳性
- validator/orchestration dict 版:末尾非 dict 数组不顶替前面的合法 dict
- SkillLoader.issues 激活:降级原因(目录缺失/零命中)注入 recon/test
阶段 caveats 与 summary,不再 write-only
- 测试 +15 例(span 扫描 5 + 三版提取 8 + caveats 端到端 2),1100 绿
issue #9 其余项(status_code 误解析/provenance 字段/status.json 门控/
CONTAMINATED 标记/技能内容填充)属于 fork 侧新功能面,main 无对应代码,
已留评论邀请 PR;本 commit 只采纳 main 上可验证的正确性修复
cdxiaodong
added a commit
that referenced
this pull request
Sep 5, 2026
Owner
|
感谢这份极其扎实的审查报告 — 逐项核对后部分已在 main 落地,其余大部分内容我们非常希望以 PR 形式接收。 已在 main 修复(eb4dff3,本条 issue 保持 open)
属实但未在本轮处理
需要澄清/邀请 PR 的部分报告中的 其中 Alt-Svc (本条 issue 按维护策略暂不关闭,等 PR 与技能补齐后一并收口) |
Owner
|
更正:上一条评论误将本 PR 当作 issue 处理 — 已确认 #9 是带完整实现的 PR(1782+/109-,25 文件,4 commits)。 状态更新:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
_extract_json(handlers.py, validator.py) now scans for every top-level, bracket-balanced JSON span and picks the final span matching the expected schema, instead of naive first-{/last-}slicing. Fixesrecon_invalid/json_parse_failedfalse positives when the model emits prose, markdown fences, a malformed draft, or multiple JSON objects before the valid final one.endpoints.json, and writes a structuredrecon/status.jsongate that the test stage checks before running.CONTAMINATED/false_positive.complete_tool_call()no longer misreads stray 3-digit numbers (e.g. the443in a CloudflareAlt-Svc: h3=":443"header) as an HTTP status code —status_codeis now only set from a genuineHTTP/x.y NNNstatus line or output that is entirely a bare status code. Root-caused via a live finding (test-d880dadd) whose provenance recordedstatus_code=443for a response that actually returned200._provenance_fornow records the connection port as its own field (separate fromstatus_code), with matching validation inhas_confirmable_provenance.SkillLoaderhardening: rejects duplicate YAML keys in a skill's frontmatter, validatesdescription/severity_focustypes and values (not just presence), and fixes a real cross-phase false-positive where duplicate-name detection could silently exclude a legitimate skill in the requested phase because of an unrelated same-named skill in a different phase.SkillLoader.issues(previously write-only, read nowhere) is now surfaced into the recon/test stages' existing caveats/summary output.command-injection,deserialization,file-upload,info-disclosure,path-traversal) whose recently-added## 三层测试模型/## 证据要求sections were empty headers with no body..gitignore: ignoreengagements/so real per-target recon/finding artifacts don't end up in git history.Test plan
pytest tests/— 1142 passed, 3 skipped🤖 Generated with Claude Code
https://claude.ai/code/session_01VtrXzdKuRt5psr7Xd8VrFw