From 3401d63063fc9a07abc0e8cc5ba43a4056dbcdc1 Mon Sep 17 00:00:00 2001 From: asepharyana Date: Thu, 16 Jul 2026 13:32:33 +0700 Subject: [PATCH] docs(shell): perbaiki doc comment shell_filter yang menyesatkan soal credential-read --- CLAUDE.md | 2 +- crates/zesdex-backend/src/tool/shell.rs | 22 ++++++++++++------- .../src/tool/shell_filter/credentials.rs | 8 ++++++- 3 files changed, 22 insertions(+), 10 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 5d06970..90edce4 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -30,7 +30,7 @@ Detailed architecture documentation is in `docs/CODEMAPS/`: - **Error handling** — `anyhow::Result` and `anyhow::bail!` throughout. No custom error types. - **Static strings** — MCP tool descriptions use `Box::leak` + `OnceLock` cache. - **Tools** — `trait Tool { fn name() -> &str, fn run() -> Result }`, 28 impls, gated by `Harness`. -- **Shell safety** — `tool/shell_filter/` blocks credential leaks and destructive git commands. +- **Shell safety** — `tool/shell_filter/` blocks destructive git commands (`shell_filter::git::check_git_destructive`, called from `tool/shell.rs::Bash::run`). It also contains a `check_credential_read` detector for credential-file reads, but that one is intentionally NOT wired into `Bash::run` today — see the doc comment on `Bash::run` for why. ### Hive-Mind Orchestration (Machine Intelligence) diff --git a/crates/zesdex-backend/src/tool/shell.rs b/crates/zesdex-backend/src/tool/shell.rs index 4eb6fd9..7807e01 100644 --- a/crates/zesdex-backend/src/tool/shell.rs +++ b/crates/zesdex-backend/src/tool/shell.rs @@ -44,16 +44,22 @@ impl Tool for Bash { }) } - /// Run a bash command (foreground or background) with safety filters and a timeout. + /// Run a bash command (foreground or background) with a safety filter and a timeout. /// - /// Flow: extract args → run `check_credential_read` then `check_git_destructive` - /// (bail if either rejects) → branch on `run_in_background`: if true, hand off - /// to the bg-bash subsystem and return the job ID; else spawn `bash -c`, - /// poll with `try_wait`, kill on timeout, format combined stdout+stderr. + /// Flow: extract args → run `check_git_destructive` (bail if it rejects) → branch on + /// `run_in_background`: if true, hand off to the bg-bash subsystem and return the + /// job ID; else spawn `bash -c`, poll with `try_wait`, kill on timeout, format + /// combined stdout+stderr. /// - /// Why: the safety filters run unconditionally so background jobs are also gated; - /// the timeout is enforced by polling the child rather than relying on a libc alarm - /// so cleanup stays in Rust. + /// Why: only destructive git operations are gated here — credential-file reads + /// (`~/.ssh/id_rsa`, `.netrc`, etc.) are deliberately NOT blocked, since the agent + /// often needs to read local config for legitimate debugging; the real leak vector + /// (committing secrets to a remote) is handled by git hooks/user review, not this + /// tool. `shell_filter::credentials::check_credential_read` exists but is + /// intentionally not called from here — see its module doc comment. The safety + /// filter runs unconditionally so background jobs are also gated; the timeout is + /// enforced by polling the child rather than relying on a libc alarm so cleanup + /// stays in Rust. /// /// Return: exit-code + elapsed-seconds summary line (plus captured output) for /// foreground runs, or the job ID for background runs. diff --git a/crates/zesdex-backend/src/tool/shell_filter/credentials.rs b/crates/zesdex-backend/src/tool/shell_filter/credentials.rs index 4ce2c1a..653ea96 100644 --- a/crates/zesdex-backend/src/tool/shell_filter/credentials.rs +++ b/crates/zesdex-backend/src/tool/shell_filter/credentials.rs @@ -1,4 +1,10 @@ -//! Block shell commands that try to read common credential files or secrets. +//! Credential-file-read detection. +//! +//! Not currently called from `tool::shell::Bash::run` — see that function's +//! doc comment for why credential reads are intentionally allowed. This +//! module is kept for callers that DO want to block credential reads (e.g. +//! a future sandboxed/untrusted-tool execution path) and is covered by its +//! own inline tests below. use anyhow::Result; /// Reject shell commands whose lowercased form contains any known credential-read pattern.