Skip to content

fix(githubreview): 证据的 redacted 与 truncated 标记各自跟随自己的真实工作 - #484

Open
Qiyuanqiii wants to merge 2 commits into
mainfrom
Qiyuanqiii/github-review-redacted-flag
Open

Qiyuanqiii wants to merge 2 commits into
mainfrom
Qiyuanqiii/github-review-redacted-flag

Conversation

@Qiyuanqiii

@Qiyuanqiii Qiyuanqiii commented Oct 23, 2026 •

Copy link
Copy Markdown
Member

SanitizeRemoteText 用 redacted != outputsafe.Sanitize([]byte(value)) 决定 TextEvidence.Redacted(model.go:285 的 json:"redacted",随 PR 标题、评论正文、Actions 日志一起持久化)。这个比较的两端基线不一致:左边是经过“远程语法转义 + 去 CR + 裁剪 + 截断 + 二次脱敏”的文本,右边是 Sanitize 的产物,而 Sanitize 内部本来就调用 redact.String(internal/outputsafe/stream.go:369-372)。所以它度量的是“转义管线有没有改动文本”,而不是“有没有脱敏”。

两个方向都会错,合成数据实测:

  • 假阴性:SanitizeRemoteText("ghp_abcdefghijklmnopqrstuvwxyz0123456789", 100) 的 Text 已经是 [REDACTED:github-token],但两侧都脱敏过、字面相等,于是 Redacted 报 false——真被脱敏的证据自称没被脱敏。
  • 假阳性:see [docs](https://example.com/doc) 和 200 个 a 的正文都不含任何机密,却因为链接被换成占位符、文本被截短而报 true。

修法是把“脱敏是否改动了文本”交回给真正做脱敏的那个函数:outputsafe 新增 SanitizeWithRedaction(data) (string, bool),先取控制字符剥离后的 inert,再比较 redact.String(inert);Sanitize 改成它的薄包装,返回文本按构造完全不变。SanitizeRemoteText 由此能分别拿到第一次(Sanitize 内部)与第二次(转义截断之后补打)脱敏的增量,Redacted 取两者之或。

没有动 Truncated。它当前的含义是“存下来的字节少于原始字节”,占位符替换也算缩短,这与 Redacted 的语义不是一回事;改它会改变既有证据的 Truncated/SHA256 组合,超出本 PR 范围。Redacted 目前也没有判定型消费方(只被序列化进 API 视图),所以本修复不改变任何门禁、授权或执行行为,只纠正证据元数据。

验证:

  • 新增表驱动用例先红后绿。在 origin/main 上三条全红:token only: Redacted=false, want true、inert link: Redacted=true, want false、oversized plain text: Redacted=true, want false;修复后 go test ./internal/githubreview ./internal/outputsafe -count=1 通过。outputsafe 侧另加一条用例,同时断言“含机密时 redacted 为真”和“仅控制字符被剥离时 redacted 为假”,并断言 SanitizeWithRedaction 的文本与 Sanitize 逐字相等。
  • 回归面核对:全仓只有新增用例断言 TextEvidence.Redacted(internal/httpapi/ui_evidence_test.go 里的 Redacted: true 属于 uievidence.ArtifactMetadata,不是同一结构体),没有既有断言依赖旧值;outputsafe.Sanitize( 的非测试调用点共 14 处、分布在 7 个文件(toolgateway 5、runner 3、browserruntime 3、githubreview 1、application 1),它们拿到的文本不变。
  • gofmt -l internal/githubreview internal/outputsafe 无输出;go vet ./internal/githubreview ./internal/outputsafe 通过;git diff --check 干净。本机 Go 全量套件有与本次无关的环境性缺口,见 fix(ui): 终端关闭响应迟到时保留新的会话绑定 #479 正文记录。
  • OCR delegate 复核(开 PR 前执行):ocr delegate preview --from origin/main --to HEAD 给出 2 个可复核文件 / 共 4 个改动文件(merge_base 513c9510,+49/-4),两个 _test.go 被默认路径规则排除,已另行人工复核。按 system / **/*.go 规则组重点核对:无被忽略的 error、无 nil 接口与锁拷贝(Stream 只有 state/pending 两个字段,每次调用新建实例,返回的是字符串副本,不存在跨调用的可变缓冲)、无数值边界或并发状态改动、测试沿用同文件既有的合成 token 而非真实凭据。无发现。

无 Surface 变化;docs/openapi.json 无需再生成(只改字段取值,未改 DTO 结构);不含凭据与本地运行数据;不触碰权限、Scope、沙箱与持久化边界。

同一函数里 Truncated 还带着同类问题,我按同样的判据一起修了(第二个提交 22081d5c)。internal/githubreview/sanitize.go 结尾原本写作 truncated = truncated || secondTruncation || originalBytes > len([]byte(clean)),第三个 OR 项的含义是"存下来的比原载荷短",而不是"撞到了字节上界"。脱敏本身就是缩短文本的:github=ghp_<36 位合成 token> 共 47 字节,[REDACTED:github-token] 之后只剩 30 字节,于是这条 47 字节的证据在对 4096 上界毫无压力的情况下报告 Truncated=true。实测(-run TestSanitizeRemoteTextTruncatedFlagTracksTheByteBound,加在本 PR 已有的 sanitize_test.go 上):

--- FAIL: TestSanitizeRemoteTextTruncatedFlagTracksTheByteBound
    in-bounds redaction reported truncation: TextEvidence{Text:"github=[REDACTED:github-token]",
    OriginalBytes:47 StoredBytes:30 Truncated:true Redacted:true}

新用例把三种情形分开了:界内脱敏(Truncated=false、Redacted=true、StoredBytes < OriginalBytes)、单次上界截断(true)、脱敏标记撑过上界后的第二次截断(true)。前两条在本 PR 的既有断言里已经钉住(sanitize_test.go:15 与 :69 都对确实越界的载荷要求 Truncated),删掉这一项不会让真实截断变成 false;两条界内/界外断言在改动前后都绿,只有界内脱敏那条从红转绿,说明本项确实是多余的。OriginalBytes 与 StoredBytes 一并序列化,任何"存下来的比原来少"的判断都能直接比较这两个数,而转义丢失本来就在正文里留下 [remote link omitted] 之类的标记,所以收窄 Truncated 不会把信息缺口藏起来。我也确认没有消费方按 Truncated 分支:网页侧只是带着这个字段——它出现在生成的 web/src/api/schema.d.ts:9942 和两个测试夹具里(github-review-panel.test.tsx:43-44/62、task-review-tools.test.tsx:52),web/src/api/client/responses 下没有它的解析器(original_bytes 0 命中),github-review-panel.tsx 与 task-review-tools.tsx 对 truncated 0 引用;Go 侧读这条证据的文件(internal/application/github_review_service.go、pr_followup.go、pr_followup_changes.go、internal/app/github_review_command.go、agent_code_tools.go)对 Truncated 0 处引用,internal/httpapi 不引用 TextEvidence 类型,全仓 grep 里 internal/githubreview 之外没有针对该字段的断言或分支。

第二个提交后重跑:go test -count=1 ./internal/githubreview/ 通过(0.907s);gofmt -l 对本 PR 四个文件无输出;go vet ./internal/githubreview/ 通过;git diff --check 干净。ocr delegate preview --from 513c9510 --to HEAD → 仍是 2 个可复核文件 / 共 4 个改动文件(+70/-5),两个 _test.go 依旧被 default_path 排除并已人工复核。新增改动的规则组核对:契约变化只收窄一个布尔标记的含义且已在上文给出影响面证据;originalBytes 变量仍用于 OriginalBytes 字段,无未使用变量;无新增 error 路径、无锁与共享状态、无数值边界。无发现。

SanitizeRemoteText decided its Redacted flag by comparing the redacted text with
outputsafe.Sanitize of the raw input, but Sanitize already runs redact.String and
the escaping pipeline had rewritten the other side. A redacted GitHub token
therefore reported Redacted=false while a benign link or an oversized plain body
reported true. Let outputsafe say whether redaction changed the text and keep
Sanitize's output identical by construction.
…lied

SanitizeRemoteText also ORed "the stored text is shorter than the input" into
Truncated, so a 47-byte payload holding one credential reported Truncated=true
against a 4096-byte bound simply because masking made it shorter. The two real
truncation passes already track the boundary, and OriginalBytes plus StoredBytes
carry any shrinkage, so reserve Truncated for the limit actually being reached.
@Qiyuanqiii Qiyuanqiii changed the title fix(githubreview): 证据的 redacted 标记只跟随脱敏,不再跟随转义 fix(githubreview): 证据的 redacted 与 truncated 标记各自跟随自己的真实工作 Oct 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants