fix(cosh-ng): block path traversal in readonly - #2708
Conversation
![qoderai[bot]](https://avatars.githubusercontent.com/in/1396158?s=60&v=4){kind=small}
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2f5d3c168e
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
PR number: #2708 Findings
结构评审确认项（无 blocking 组织问题）
剩余风险
未跑验证（以 CI 实测为准）
|
2f5d3c1 to
0ae466c
Compare
|
Both findings addressed in 0ae466c (single commit, force-pushed): P1 (relative traversal) — fixed fail-closed and cwd-independently: after lexical normalization, a leading P2 (CI vs body) — the fast-checks failure was the test-inventory overlap ratchet: the three new tests had landed in |
|
PR number: #2708 Findings
结构评审确认项（无 blocking 组织问题）未发现 blocking package/module/public API 组织问题：
剩余风险
未跑验证（以 CI 实测为准）
|
0ae466c to
eb1cb67
Compare
|
Thanks for the follow-up monitor pass. The
On the |
|
PR number: #2708 Findings
结构评审确认项（无 blocking 组织问题）未发现 blocking package/module/public API 组织问题：
剩余风险
未跑验证（以 CI 实测为准）
|
Clippy 1.98 on stable rolls the new chunks_exact_to_as_chunks lint onto pre-existing decode_hex code, turning the fast-checks job on #2708 red. as_chunks::<2>().0.iter() is semantics-preserving: same full chunks, same dropped tail remainder as chunks_exact(2). Fixes: 1a93589 ("feat(cosh-ng): [core,shell] add audit log") Assisted-by: Qoder Signed-off-by: Jiangtian Feng <jiangtianf97@163.com>
|
PR number: #2708 PR #2708 验证报告：fix(cosh-ng): block path traversal in readonly验证范围
结果分类两个 focused scope 首跑即通过，无需复跑：
stderr 证实两次运行均重新编译了当前 worktree 的 Findings 验证[P1] 残余绕过（裸相对拼写 +
|
|
Reviewed against Verified fixed
P1: residual bypasses in the same family
Reproduce by extracting the four functions from this PR's Two notes on reachability:
Why this is a design gap, not a missing case: the doc comment on Suggested convergence (one change, not four cases): drop the P2: the cross-layer
|
is_blocked_special_path matched /proc, /dev and /sys by raw prefix only, so spellings like /../proc/version bypassed the blocklist. Normalize lexically (no filesystem access) before matching, keep the raw-prefix pass first (fail-closed), and block relative spellings whose first surviving component is proc/dev/sys; a leading ~ popped by .. is unresolvable and fails closed. $-quoting forms stay intercepted upstream by the exec layer. Traversal tests moved to a lib-only module (overlap ratchet). Closes #2184 Assisted-by: Qoder Signed-off-by: Jiangtian Feng <jiangtianf97@163.com>
eb1cb67 to
8af1480
Compare
|
Thanks for the thorough verification round — the OS-level reproductions (cwd= Landed in 8af1480 (single commit, force-pushed onto latest
Anti-false-positive matrix re-run: Discriminative two-direction check (ECS, toolchain 1.89.0): on the previous head
|
|
PR number: #2708 Findings
前轮 findings 闭环（静态推演确认修复有效）
结构评审确认项（无 blocking 组织问题）未发现 blocking package/module/public API 组织问题：
剩余风险
未跑验证（以 CI 实测为准）
|
|
PR number: #2708 PR #2708 验证报告（T2）验证范围
结果分类
Findings 验证
补充测试（评审证据，未提交）写入 #[test]
fn probe_tokenizer_strips_quotes_and_backslash_escapes() {
let quoted = super::command_risk_parser::parse_command("cat '/proc/version' && pwd");
assert_eq!(quoted.segments, vec![
vec![vec!["cat".to_string(), "/proc/version".to_string()]],
vec![vec!["pwd".to_string()]],
]);
let escaped = super::command_risk_parser::parse_command("cat /pro\\c/version && pwd");
assert_eq!(escaped.segments, vec![
vec![vec!["cat".to_string(), "/proc/version".to_string()]],
vec![vec!["pwd".to_string()]],
]);
}
#[test]
fn probe_quoted_or_escaped_special_paths_stay_ineligible() {
for command in [
"cat /proc/version && pwd",
"cat '/proc/version' && pwd",
"cat \"/proc/version\" && pwd",
"cat /pro\\c/version && pwd",
] {
assert!(build_readonly_compound_plan(command).is_none(),
"{command} must stay ineligible");
}
}运行结果：容器内 证据索引本作业为单元测试级验证，无 PTY/渲染界面，未产生截图，evidence_dir 为空。证据载体为各 run_dir 下的 未运行 scope全量 结论Go —— PR 变更面对应的 focused scope 全绿，唯一待决的 P2 经容器内探针实锤为 shell_words 式分词、无安全缺口，静态评审结论无需修正； |
Clippy 1.98 on stable rolls the new chunks_exact_to_as_chunks lint onto pre-existing decode_hex code, turning the fast-checks job on #2708 red. as_chunks::<2>().0.iter() is semantics-preserving: same full chunks, same dropped tail remainder as chunks_exact(2). Fixes: 1a93589 ("feat(cosh-ng): [core,shell] add audit log") Assisted-by: Qoder Signed-off-by: Jiangtian Feng <jiangtianf97@163.com>
8af1480 to
736b551
Compare
Clippy 1.98 on stable rolls the new chunks_exact_to_as_chunks lint onto pre-existing decode_hex code, turning the fast-checks job on alibaba#2708 red. as_chunks::<2>().0.iter() is semantics-preserving: same full chunks, same dropped tail remainder as chunks_exact(2). Fixes: 1a93589 ("feat(cosh-ng): [core,shell] add audit log") Assisted-by: Qoder Signed-off-by: Jiangtian Feng <jiangtianf97@163.com>
|
P2 确认（分词行为）：compound 链的分词是 shell 式的，引号和反斜杠在 token 化阶段就已解析，到达路径检查的都是解析后的字面量——无缺口。证据：
因此 |
|
PR number: #2708 Findings
前轮 findings 闭环（与上一轮引用逐条比对，无回退）
结构评审确认项未发现 blocking package/module/public API 组织问题：
剩余风险
未跑验证（以 CI 实测为准）
|
|
PR number: #2708 PR #2708 验证报告（T1）1. 验证范围
2. 结果分类
3. Findings 验证
4. 补充测试无（T1 作业，未新增测试）。 5. 证据索引
6. 未运行 scope
7. 结论Go。PR 引入的行为面（穿越阻断 6 新例 + |
2 participants
Why
is_blocked_special_pathmatched/proc,/devand/sysby raw prefix only, so spellings like/../proc/versionbypassed the blocklist and were allowed through the readonly auto-execution chain — a real bypass of the readonly safety check.What changed
$-quoting forms remain intercepted upstream by the exec layer; noted in a code comment.Related issue
closes #2184
User / Agent impact
Commands whose path arguments lexically resolve into
/proc,/devor/sys— previously executed via the readonly chain — are now blocked. Legitimate paths are unaffected.Risk and compatibility
Security tightening: the newly intercepted set is exactly the spellings that lexically resolve into
/proc,/devor/sys, all of which were genuine bypasses before the fix. An anti-false-positive matrix pins legitimate paths that must keep passing.Validation
cargo fmtandclippyclean with zero warnings.Documentation and rollback
Single commit — revert it directly to restore the previous matching behavior if ever needed.