Repository navigation
fix: sanitize hidden terminal strings in web evidence - #705
outlier27-cell wants to merge 10 commits into
Conversation
github-actions
Bot
added
the
documentation
|
@tt-a1i 请审核此 PR。最新提交已通过完整 GitHub CI，当前可合并；如有需要调整的地方，我会在原分支及时修复。 |
testikun
left a comment
testikun
left a comment
There was a problem hiding this comment.
@outlier27-cell 已审查当前 Web 改动，head 为 6315bd9abd60699e2f14dd4700a76a8124e9530b，基线为 3cb2ecfe1bfbb98252651885311d309f83749428。
发现一项 P2：显示文本的清理改变了可点击文件引用，导致预览打开另一份文件，详见行内评论。
验证：macOS / Node 24.20.0；现有 8 项 evidence 测试通过。额外使用真实临时文件、生产投影函数和 ArtifactReader 比较基线与当前 head，确认上述回归。没有修改原工作区、调用模型或使用私人 Session；本结论来自生产函数与文件预览读取链路。
6315bd9 to
319aa39
Compare
|
Fixed in 319aa39.
Regression coverage uses real temporary files to verify that |
|
@testikun 已按审核意见修复路径身份与展示文本混用的问题，最新提交和完整 CI 均已通过，烦请复核。 |
There was a problem hiding this comment.
Exact reviewed head: 319aa3980f13b5fba8a238413a06ed56796422fe.
Spec:
- [P2] Bound raw path identity before evidence admission: The old Unicode path-identity issue is fixed, and its thread is now resolved. A separate 1 MiB path fixture grows serialized production evidence from 24,491 bytes on main to 1,060,779 bytes on this head: bounded displayPath does not bound raw path/resolvedPath. Preserve exact identity for admitted paths, validate its byte budget separately, and report an oversized path as non-actionable rather than rewriting identity or removing the bound.
Verification: prior exact-head Standards and Spec evidence reused because this head is unchanged; current review/CI/integration metadata refreshed. No complete-suite rerun, installed-runtime/provider run, or visual acceptance performed in this update.
Maintainer fix follow-up
The path admission fix is implemented in 2286726d808a0ea4189d4b92ee9b9d928ebe4ffa, appended to the original contributor branch by non-force updates. Raw and trusted resolved identities are independently admitted at 4,096 UTF-8 bytes, exact identity is preserved, oversized identity is display-only, and fallback cannot restore a rejected action target. Raw callers cannot provide trusted derived metadata. Invisible admitted names keep a safe UI label. The Windows true-tab filesystem fixture was corrected without skipping identity/denial assertions or widening production policy. The committed Web app was regenerated and verified reproducible.
Independent automated Spec and Standards reviews have no open scoped findings. Focused Node 13/13 and UI 9/9, bun run check and the final exact-head bun run test passed: Node 2,319 passed / 9 skipped, UI 1,160 passed. Earlier local timeout, unavailable first terminal receipt and Windows fixture failure remain historical evidence.
Current-head remote CI remains FAILED: all Runtime/Windows/UI jobs pass, but message-rerun.e2e.ts:209 fails. This run's own trace reaches a successful fork and correct child-4 snapshots at 390px, then reports confirmation uncertainty before prompt acceptance. A competing refresh diagnosis is an inference; #718 addresses that seam. The code finding is addressed, while overall readiness still requires the current CI gate and valid last-push approval. After a maintainer authorizes merging #718, synchronize and revalidate the new head; do not mix that separate Web fix into this path PR or treat the red gate as passed.
…inal-strings # Conflicts: # tests/web/evidence-view.spec.ts # web/dist/app.js # web/ui/src/features/transcript/ToolEvidence.tsx
The merged summary renders the display path, while the artifact action still opens the original resolved path.
…inal-strings # Conflicts: # web/dist/app.js # web/protocol/evidence.ts # web/ui/src/features/transcript/ToolEvidence.tsx
Apply the repository formatter to the merged evidence component and refresh the committed web bundle.
|
@tt-a1i 当前 head 相对 main 的改动仍限于 evidence 协议、转录展示和对应测试。GitHub 上 18 项检查均成功，0 失败，PR 可合并。 |
…inal-strings # Conflicts: # web/dist/app.js
This branch has not been deployed
3 participants
Problem
Fixes #700.
Value
This is an evidence-backed source-boundary fix with deterministic regressions; malformed input no longer becomes a misleading successful projection.
Approach
Admit raw and trusted resolved file identities independently at 4,096 UTF-8 bytes. Keep exact admitted identity, bounded display, display-only oversized paths and safe fallback behavior. Preserve raw-versus-trusted metadata ownership and invisible-filename labels. Real tab-file setup/readback is POSIX-only; identity/display and Artifact denial assertions remain enabled on all platforms. The committed Web app build was updated and verified reproducible.
Require the existing canonical types and limits at the Web boundary. Add regression tests that fail on origin/main and pass after the fix. See the linked docs/research record in this branch.
Validation
Latest maintainer follow-up (
2286726d808a0ea4189d4b92ee9b9d928ebe4ffa)Focused Node 13/13 and UI 9/9; check passed. Final exact-head full test exit 0: Node 2,319 passed / 9 skipped and UI 1,160 passed. Earlier local timeout, initial missing terminal receipt and the original Windows fixture failure remain historical evidence. New remote CI has all Runtime/Windows/UI green but one Web E2E failure at message-rerun:209, so overall CI remains FAILED. Its own trace confirms fork success and matching child snapshot followed by confirmation uncertainty before prompt acceptance. A competing refresh diagnosis is an inference; #718 addresses that seam and still needs a maintainer merge decision before downstream synchronization.
Independent automated Spec and Standards reviews have no open scoped findings. Original contributor commits and branch are preserved with non-force updates. Required current-head CI, strict main synchronization and valid last-push approval remain distinct merge gates.
Earlier contributor validation
Impact
Existing valid behavior is preserved. Malformed evidence/configuration is rejected or remains unknown. No provider, Session, or orchestration surface is added.