Skip to content

fix(security): unified sensitive path protection across shell and file tools - #1713

Closed
zmanian wants to merge 3 commits into
stagingfrom
fix/unified-sensitive-path-protection
Closed

zmanian wants to merge 3 commits into
stagingfrom
fix/unified-sensitive-path-protection

Conversation

@zmanian

@zmanian zmanian commented Mar 27, 2026

Copy link
Copy Markdown
Collaborator

Summary

Closes the security gap where the shell tool could bypass file-level protections (e.g., cat ~/.ssh/id_rsa worked even though ReadFileTool would block ~/.ssh/id_rsa).

Adds a shared SENSITIVE_PATH_PATTERNS list in path_utils.rs used by both shell and file tools:

  • Shell tool: command_references_sensitive_path() scans commands for sensitive file references
  • File tools: is_sensitive_path() blocks ReadFileTool, WriteFileTool, ListDirTool, ApplyPatchTool
  • ListDirTool: Skips sensitive subdirectories during recursive traversal

Protected paths

SSH keys, GPG, AWS/Azure/GCP credentials, Kubernetes config, GitHub CLI tokens, Terraform credentials, Docker config, Vault tokens, shell history, .env files, git credentials, system shadow files, and sensitive key file extensions (.pem, .key, .p12, .pfx, .jks, .keystore).

Safe suffixes (.example, .sample, .template) are excluded to avoid blocking template files.

Previous state

  • Shell tool had 5 hardcoded patterns in DANGEROUS_PATTERNS (/etc/passwd, /etc/shadow, ~/.ssh, .bash_history, id_rsa)
  • File tools had zero sensitive path protection
  • Commands like cat ~/.aws/credentials or read_file ~/.config/gh/hosts.yml were unblocked

Test plan

  • cargo clippy --all --benches --tests --examples --all-features -- zero warnings
  • 21 new tests: path detection, command scanning, safe suffixes, normal file allowlisting
  • Existing path_utils tests pass (sandbox validation, traversal rejection)
  • cargo check --no-default-features --features libsql -- compiles

…e tools

Add a shared SENSITIVE_PATH_PATTERNS list in path_utils.rs that protects
credentials, secrets, and private keys consistently across all tool types:

- Shell tool: command_references_sensitive_path() scans commands for
  references to sensitive files (cat ~/.ssh/id_rsa, etc.)
- File tools: is_sensitive_path() blocks ReadFileTool, WriteFileTool,
  ListDirTool, and ApplyPatchTool from accessing sensitive paths
- ListDirTool: skips sensitive subdirectories during recursive traversal

Previously, the shell tool had a small hardcoded list (5 patterns) in
DANGEROUS_PATTERNS while file tools had no sensitive path protection at
all. This created an asymmetric security model where file tools were
more permissive than the shell tool.

The shared list covers: SSH keys, GPG, AWS/Azure/GCP credentials,
Kubernetes config, GitHub CLI tokens, Terraform credentials, Docker
config, Vault tokens, shell history, .env files, git credentials,
system shadow files, and sensitive key file extensions (.pem, .key,
.p12, .pfx, .jks, .keystore). Safe suffixes (.example, .sample,
.template) are excluded.

21 tests covering path detection, command scanning, safe suffixes,
and normal file allowlisting.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@github-actions github-actions Bot added scope: tool/builtin Built-in tools size: L 200-499 changed lines risk: medium Business logic, config, or moderate-risk modules contributor: core 20+ merged PRs labels Mar 27, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request centralizes sensitive path protection by introducing a shared utility to identify and block access to credentials, SSH keys, and system secrets across file and shell tools. It updates file operations (read, write, list, patch) and shell command execution to utilize these checks. Feedback indicates that the shell command scanner should be refined to use word-boundary or token-based matching instead of simple substring containment to minimize false positives.

Comment on lines +108 to +112
for pattern in SENSITIVE_PATH_PATTERNS.iter() {
// For path patterns, check case-insensitively
if normalized.contains(&pattern.to_lowercase()) {
return Some(pattern);
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

security-medium medium

The current implementation uses simple substring containment to detect sensitive paths, which leads to false positives (e.g., matching 'id_rsa' inside other words). Per repository security guidelines, please replace simple substring checks with token-based or word-boundary checks to improve precision and reduce false positives.

References
  1. When detecting commands or keywords in a string, use token-based or word-boundary checks instead of simple substring containment to avoid false positives.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9fb704a213

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/tools/builtin/path_utils.rs Outdated
}

// Check sensitive path patterns
if SENSITIVE_PATH_PATTERNS.iter().any(|p| path_str.contains(p)) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Normalize separators before checking sensitive path patterns

is_sensitive_path does a raw contains match against slash-delimited patterns (e.g., /.ssh/, /.aws/credentials) without normalizing path separators. On Windows, Path::to_string_lossy() yields backslash paths, so sensitive targets like C:\Users\...\.ssh\id_rsa do not match and the file-level protection is bypassed; because the same pattern set is reused for shell scanning, backslash-based shell paths are similarly missed.

Useful? React with 👍 / 👎.

Comment on lines +75 to +77
let path_str = match path.canonicalize() {
Ok(p) => p.to_string_lossy().to_string(),
Err(_) => path.to_string_lossy().to_string(),

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Canonicalize parent path when target file does not yet exist

When canonicalization fails, is_sensitive_path falls back to the unresolved input path, which allows symlink aliases to hide sensitive destinations for new-file writes. For example, if /tmp/link is a symlink to ~/.ssh, writing /tmp/link/new_config is treated as non-sensitive (no /.ssh/ in the alias string) even though write_file will create the file inside ~/.ssh, leaving a bypass for protected directories.

Useful? React with 👍 / 👎.

Comment thread src/tools/builtin/path_utils.rs Outdated
Comment on lines +115 to +119
// Check for sensitive extensions in file arguments
SENSITIVE_EXTENSIONS
.iter()
.find(|ext| normalized.contains(*ext))
.copied()

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Apply safe-suffix exemptions in shell sensitive-path scan

command_references_sensitive_path checks sensitive patterns/extensions directly but never applies the SAFE_SUFFIXES allowlist that is_sensitive_path uses. This causes shell commands against template files (for example .env.example or .pem.template) to be blocked while file tools allow them, creating inconsistent behavior and breaking workflows the safe-suffix policy was added to preserve.

Useful? React with 👍 / 👎.

@serrrfirat serrrfirat left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review: fix(security): unified sensitive path protection across shell and file tools

Good direction — unifying sensitive path detection across shell and file tools closes a real gap. The pattern list is comprehensive and the test coverage for the happy paths is solid. However, there are several issues with the substring-matching approach that create both false positives and bypasses.


HIGH — H1: command_references_sensitive_path has no safe-suffix exemption

is_sensitive_path() correctly exempts .env.example, .env.sample, .env.template via SAFE_SUFFIXES. But command_references_sensitive_path() does pure substring matching with no suffix check. This means:

  • cat /app/.env.example — allowed by ReadFileTool, blocked by shell tool
  • source /app/.envrc — blocked by shell tool (.envrc is direnv code, not secrets)

The asymmetry between file tools and shell tool for the same paths is confusing and creates inconsistent UX.

Fix: Apply SAFE_SUFFIXES exemption in command_references_sensitive_path(), or extract file-path-like tokens from the command and run is_sensitive_path() on each.

HIGH — H2: Extension matching via contains() on full command strings causes false positives

command_references_sensitive_path() checks normalized.contains(".key") against the entire lowercased command string. This matches:

  • cat config.keys — blocked (.key substring in .keys)
  • cat .keychain — blocked
  • grep pattern file.keynote — blocked
  • cat /tmp/my.keyboard.cfg — blocked
  • cat /tmp/template.pemx — blocked (.pem in .pemx)

These are not sensitive files. The extension check needs word-boundary awareness — either split the command into tokens and check file extensions on each token, or use a regex like \\.key(?:\s|$).

HIGH — H3: is_sensitive_path pattern check is case-sensitive — bypass on macOS

Line 85: SENSITIVE_PATH_PATTERNS.iter().any(|p| path_str.contains(p)) checks against path_str (original case), not lower. The lower variable is created but only used for safe-suffix checking.

On macOS (case-insensitive HFS+/APFS), if canonicalize() fails (file doesn't exist — relevant for WriteFileTool), the fallback uses the original input case. So write_file("/app/.SSH/id_rsa", ...) bypasses the check because "/.SSH/" doesn't match pattern "/.ssh/".

Fix: Use lower.contains(p) instead of path_str.contains(p) on line 85. The lower variable is already computed two lines above.

MEDIUM — M1: /.env pattern is overly broad — blocks .envrc, .environment

The pattern "/.env" blocks any path containing that substring:

  • /.envrc — direnv config (code, not secrets, version-controlled)
  • /.environment — systemd unit files
  • /.env.development, /.env.production, /.env.local — intentionally blocked per the tests, which is fine

.envrc is the main false positive here. Consider changing the pattern to "/.env" only matching when followed by nothing, a dot, or end-of-path. Or add ".envrc" to an exclusion list.

MEDIUM — M2: Sensitive path check bypasses allow_dangerous flag — behavior change

Previously, path patterns (/etc/shadow, ~/.ssh, id_rsa, .bash_history) were in DANGEROUS_PATTERNS and gated by !self.allow_dangerous. The new command_references_sensitive_path() check at line 617 runs unconditionally, outside the allow_dangerous gate.

This is arguably a security hardening (sensitive paths should never be accessible even with allow_dangerous), but it's an undocumented behavior change. If any workflow relies on allow_dangerous=true to access these paths (e.g., a sandboxed tool builder reading its own .env), it will break silently.

Fix: At minimum, document this in the PR description. Consider whether the intent is correct.

LOW — L1: /etc/passwd was removed from protection

The old DANGEROUS_PATTERNS included /etc/passwd. The new SENSITIVE_PATH_PATTERNS includes /etc/shadow and /etc/gshadow but not /etc/passwd. While /etc/passwd is world-readable on most systems, the removal should be intentional and documented.

LOW — L2: Symlink bypass when canonicalize fails (codex-bot also flagged this)

When canonicalize() fails (file doesn't exist), is_sensitive_path falls back to the raw input path. A symlink like /tmp/link -> ~/.ssh/ wouldn't be resolved. However, for WriteFileTool this is the relevant case — writing to a path that symlinks into a sensitive location. The validate_path function upstream does handle some symlink resolution, so the practical risk depends on ordering. Worth a note but likely low-risk given the defense-in-depth.

NIT — N1: LazyLock<Vec<&'static str>> could be &[&str]

The vectors are compile-time constants. LazyLock<Vec<...>> allocates on the heap at first access. A const array or static slice would be simpler and zero-allocation:

static SENSITIVE_EXTENSIONS: &[&str] = &[".pem", ".key", ".p12", ".pfx", ".jks", ".keystore"];

Summary

The unification concept is sound and the pattern list is well-researched. The main issues are:

  1. H2 (extension false positives via contains()) will cause user friction in normal development
  2. H3 (case-sensitive check) is a bypass on macOS
  3. H1 (missing safe-suffix in command scanner) creates inconsistent behavior between file and shell tools

I'd recommend fixing H1-H3 before merge. The medium and low findings are worth addressing but aren't blockers.

Comment thread src/tools/builtin/path_utils.rs Outdated
}

// Check sensitive path patterns
if SENSITIVE_PATH_PATTERNS.iter().any(|p| path_str.contains(p)) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

HIGH (H3): Pattern check is case-sensitive but lower is already computed two lines above. On macOS (case-insensitive filesystem), if canonicalize() fails (file doesn't exist — relevant for WriteFileTool), the fallback uses the original input case. So write_file("/app/.SSH/id_rsa", ...) bypasses the check because "/.SSH/" doesn't match pattern "/.ssh/".

Fix: Change path_str.contains(p) to lower.contains(p) — the lower variable is already computed on line 79.

SENSITIVE_EXTENSIONS
.iter()
.find(|ext| normalized.contains(*ext))
.copied()

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

HIGH (H2): Extension matching via contains() on the full lowercased command string causes false positives. Examples: cat config.keys, cat .keychain, grep pattern file.keynote, cat /tmp/my.keyboard.cfg all match .key. Similarly cat /tmp/template.pemx matches .pem.

Fix: Split the command into whitespace-delimited tokens and check file extensions on each token individually, or use a regex like \.key(?:\s|$).

/// Scan a shell command string for references to sensitive paths.
/// Returns the first matched pattern, or None if the command is clean.
/// Used by the shell tool to block `cat ~/.ssh/id_rsa` etc.
pub fn command_references_sensitive_path(command: &str) -> Option<&'static str> {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

HIGH (H1): This function does pure substring matching with no safe-suffix exemption, unlike is_sensitive_path() which checks SAFE_SUFFIXES first. This means cat /app/.env.example is allowed by ReadFileTool but blocked by shell tool. Also blocks source /app/.envrc (direnv code, not secrets).

Fix: Apply SAFE_SUFFIXES exemption here too, or extract file-path tokens from the command and delegate to is_sensitive_path().

@zmanian zmanian left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Security Review: Unified Sensitive Path Protection

What was done well

  • The core idea is sound: unifying sensitive path detection across shell and file tools closes a real gap where cat ~/.ssh/id_rsa via shell would succeed while ReadFileTool would block it.
  • Good test coverage for the happy paths (21 tests).
  • The SENSITIVE_PATH_PATTERNS list is comprehensive and well-organized by category.
  • The safe-suffix exemption for .example/.sample/.template is a thoughtful UX consideration.
  • Correct use of ToolError::NotAuthorized (not ExecutionFailed) for security denials.

Critical Issues (Must Fix)

C1: Symlink bypass in is_sensitive_path -- incomplete canonicalization fallback

When path.canonicalize() fails (file does not exist yet, relevant for WriteFileTool), the function falls back to the raw path string. This means a symlink can hide a sensitive destination:

ln -s ~/.ssh /tmp/innocent
write_file("/tmp/innocent/new_config", "malicious")

The path string /tmp/innocent/new_config does not contain /.ssh/, so the check passes, but the file gets created inside ~/.ssh/. The existing validate_path function already handles symlink resolution for sandbox escapes (lines 86-113 of path_utils.rs), but is_sensitive_path is called on the path AFTER validate_path resolves it. The problem is that validate_path also falls back to lexical normalization when canonicalize fails. For WriteFileTool specifically, the target file does not exist, so canonicalize fails on the full path. The ancestor-walking logic in validate_path would resolve the symlink, but is_sensitive_path receives the result of the initial resolution (line 62-75), not the check_path.

Fix: is_sensitive_path should walk up to the nearest existing ancestor, canonicalize that, and re-append the tail -- exactly like validate_path's check_path logic. Or better: call is_sensitive_path on the check_path computed inside validate_path, not on the resolved path returned to the caller.

C2: Case-sensitivity bug on macOS

is_sensitive_path computes lower = path_str.to_lowercase() for safe-suffix checking but then does the pattern match against the ORIGINAL path_str: SENSITIVE_PATH_PATTERNS.iter().any(|p| path_str.contains(p)). On macOS (case-insensitive HFS+), if canonicalize fails (file does not exist), a path like /app/.SSH/id_rsa bypasses the check because /.SSH/ does not match /.ssh/. The lower variable is already computed -- use it for pattern matching too.

C3: /etc/passwd was removed from DANGEROUS_PATTERNS with no replacement

The old DANGEROUS_PATTERNS included /etc/passwd. This PR removed it (along with /etc/shadow, ~/.ssh, .bash_history, id_rsa) and moved the logic to command_references_sensitive_path. However, /etc/passwd is NOT in the new SENSITIVE_PATH_PATTERNS list -- only /etc/shadow and /etc/gshadow are. This is a regression: cat /etc/passwd is no longer blocked by the shell tool. While /etc/passwd is world-readable, blocking access to it was an intentional security decision in the original code, and removing it silently is concerning. Either add it back to SENSITIVE_PATH_PATTERNS or document the intentional removal.

Important Issues (Should Fix)

I1: command_references_sensitive_path lacks safe-suffix exemption

is_sensitive_path() exempts .env.example etc. via SAFE_SUFFIXES, but command_references_sensitive_path() does pure substring matching with no such exemption. Result: cat /app/.env.example is allowed by ReadFileTool but blocked by shell tool. This asymmetry is confusing and defeats the purpose of unification.

I2: Extension matching via contains() causes false positives in shell scanning

command_references_sensitive_path checks normalized.contains(".key") against the entire lowercased command string. This matches:

  • cat config.keys (.key is a substring of .keys)
  • cat .keychain
  • grep pattern file.keynote
  • cat /tmp/my.keyboard.cfg

Similarly .pem matches cat template.pemx. The fix is to either split the command into whitespace-delimited tokens and check extensions on each token, or use a regex boundary like \.key(\s|$).

I3: Windows path separator bypass

is_sensitive_path matches against forward-slash patterns (/.ssh/, /.aws/credentials). On Windows, Path::to_string_lossy() yields backslash paths, so C:\Users\...\.ssh\id_rsa would not match. While IronClaw may be Linux/macOS-focused, this should at minimum be documented, or the path string should be normalized to forward slashes before matching.

I4: Overlap with PR #1675

PR #1675 (also open) creates crates/ironclaw_safety/src/sensitive_paths.rs -- a proper module in the ironclaw_safety crate. This PR adds the same logic inline in path_utils.rs. Per the project's CLAUDE.md: "new code should import from ironclaw_safety directly." These two PRs will conflict. The sensitive path logic should live in the safety crate (as PR #1675 proposes), not in path_utils.rs. This PR should either be rebased on top of 1675 or the two should be merged.

Suggestions (Nice to Have)

S1: LazyLock<Vec<&str>> could be &[&str]

The SENSITIVE_PATH_PATTERNS, SENSITIVE_EXTENSIONS, and SAFE_SUFFIXES are all compile-time-known constant data. They do not need LazyLock<Vec<...>> -- a simple const or static slice would be simpler and avoid the lazy initialization overhead. Example: static SENSITIVE_EXTENSIONS: &[&str] = &[".pem", ".key", ".p12", ".pfx", ".jks", ".keystore"];

S2: Missing tests for bypass vectors

For a security feature, the test suite should include adversarial cases:

  • Symlink-through-path test (C1)
  • Case variation test on macOS (/.SSH/id_rsa) (C2)
  • Safe-suffix via shell command (cat .env.example) -- should pass (I1)
  • False positive test (cat config.keys) -- should pass (I2)
  • Path with /../ normalization (/home/user/innocent/../.ssh/id_rsa) through file tools

S3: The /.env pattern is overly broad

"/.env" matches /.env, /.env.local, /.env.production, /.envrc (direnv config), /.environment, etc. Consider using "/.env" with an explicit check that the next character is either end-of-string, ., or / to avoid blocking legitimate dotfiles.

TOCTOU Assessment

The file tools call validate_path (which may canonicalize) and then separately call is_sensitive_path on the result. Between these two calls, a symlink could be modified. However, this is a minor concern given the typical threat model (the attacker is the LLM, not a concurrent process). The more pressing issue is C1 (symlink bypass via incomplete canonicalization), which is a static bypass, not a race condition.

Verdict

The core approach is right but C1 (symlink bypass), C2 (case-sensitivity), and C3 (/etc/passwd regression) need to be addressed before merge. The overlap with PR #1675 should also be resolved to avoid architectural divergence.

- Fix .env pattern matching .envrc/.environment (direnv config files
  are not sensitive). Now /.env matches exactly or /.env.* but not
  /.envrc or /.environment.
- Fix extension check in command scanner: require sensitive extensions
  (.key, .pem, etc.) at end of whitespace-delimited tokens instead of
  substring matching, preventing false positives like --key-file.
- Add doc comment to command_references_sensitive_path documenting
  that the command scanner is defense-in-depth and cannot catch
  symlinks, encoding tricks, variable expansion, or glob bypasses.
- Add regression tests for all three issues.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@zmanian

zmanian commented Mar 29, 2026

Copy link
Copy Markdown
Collaborator Author

Note: This PR conflicts with #1675 which implements the same sensitive path protection but places it in crates/ironclaw_safety/ (architecturally correct per the project's crate extraction direction). However, this PR has a more complete path list (shell history, key extensions like .pem/.key/.p12, individual key files like id_ed25519/id_ecdsa, /etc/gshadow).

Recommendation: use #1675 as the base (safety crate is the right home), absorb this PR's more complete pattern list into it, and close this PR as superseded.

@ilblackdragon ilblackdragon left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Additional Review Findings

Beyond zmanian's existing feedback (symlink bypass, case-sensitivity fixed in 3rd commit, /etc/passwd regression, overlap with #1675):

Critical

  1. command_references_sensitive_path lacks safe-suffix exemption — is_sensitive_path() exempts .env.example, .env.sample, .env.template via SAFE_SUFFIXES. But the shell tool check does not. Result:

    • read_file("/app/.env.example") → ALLOWED
    • cat /app/.env.example via shell → BLOCKED
      This defeats the stated "unified" protection goal.
  2. Quoted arguments bypass shell check — check_segment_file_commands splits on whitespace but doesn't strip quotes. cat "/home/user/.ssh/id_rsa" passes quotes through to is_sensitive_path, which tries to match "/home/user/.ssh/id_rsa" (with quotes). canonicalize() fails, raw quoted string won't match patterns. This is a bypass vector.

High

  1. allow_dangerous behavior change undocumented — sensitive path patterns were previously inside DANGEROUS_PATTERNS gated by !self.allow_dangerous. The new check runs UNCONDITIONALLY. If any workflow relies on allow_dangerous=true (e.g., sandboxed tool builder), it breaks silently.

  2. LazyLock<Vec<&'static str>> should be &[&str] — these are compile-time constant data. Static slices avoid unnecessary heap allocation.

Per zmanian's recommendation

Agree that #1675 should be the base (safety crate is the right home). Absorb this PR's more complete pattern list into #1675 and close this PR as superseded.

j-bloggs added a commit to j-bloggs/ironclaw that referenced this pull request Mar 29, 2026
…#1713 patterns

Absorb nearai#1713's sensitive path patterns into ironclaw_safety crate:
- Add shell history files, SSH key types, /etc/gshadow
- Add sensitive file extensions (.pem, .key, .p12, .pfx, .jks, .keystore)
- Add .dist safe suffix; smart .env matching (excludes .envrc, .environment)
- Directory-level blocking for .aws/, .docker/, .kube/ (not just specific files)
- Trailing-slash matching so bare directory paths trigger detection

Shell tool hardening:
- Strip surrounding quotes from tokens before sensitive path check
- Add grep, awk, sed to FILE_READ_COMMANDS
- Scan ALL redirect operators in a segment, not just the first

Other fixes:
- Add trailing \b to Telegram bot token regex to prevent over-matching
- Update error messages to reference secret_list/secret_create
- Strengthen ListDirTool test with tempfile-based .ssh directory

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@zmanian

zmanian commented Mar 31, 2026

Copy link
Copy Markdown
Collaborator Author

Closing as superseded per ilblackdragon's recommendation. The pattern list will be absorbed into #1675 (safety crate is the right home for unified sensitive path detection).

@zmanian zmanian closed this Mar 31, 2026
ilblackdragon pushed a commit that referenced this pull request Apr 7, 2026
)

* fix(safety): add credential patterns and sensitive path blocklist

Addresses critical credential leakage found by security testing
(~/.ironclaw/tests/SECURITY_REPORT.md, test ce-02).

Leak detector (crates/ironclaw_safety/src/leak_detector.rs):
- Add OpenRouter API key pattern (sk-or-v1-<hex>)
- Add Anthropic OAuth token pattern (sk-ant-oat<NN>-<base64url>)
- Add Telegram bot token pattern (word-bounded, 8-12 digit bot ID)
- Add Groq API key pattern (gsk_<alphanumeric>)
- 12 new tests with synthetic keys (positive, false-positive, integration)

File tools (src/tools/builtin/file.rs):
- Add sensitive path blocklist to ReadFileTool, WriteFileTool, and
  ApplyPatchTool (defense-in-depth for all file access vectors)
- Blocks: .env (and .env.local/.env.production/etc.), .ssh/, .aws/,
  .netrc, .pgpass, .npmrc, .pypirc, .docker/config.json, .kube/config,
  .git-credentials, .gcloud/, .config/gcloud/, .gnupg/, .vault-token,
  .ironclaw/secrets/
- Allows .env.example, .env.template, .env.sample (safe suffixes)
- Case-insensitive; resolves symlinks via canonicalize() before check
- 8 new tests covering blocking, safe suffixes, .env variants, case

Known gap: shell tool can still `cat ~/.env` — different security
domain (denylist-gated in autonomous mode, user-initiated in
interactive mode). Tracked for follow-up.

Note: new patterns use .unwrap() on Regex::new() matching the
established convention of the 16 existing patterns in this file
(all use // safety: hardcoded literal). Follow-up to address the
existing .unwrap() debt across all patterns.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* Update src/tools/builtin/file.rs

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

* Update crates/ironclaw_safety/src/leak_detector.rs

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

* Update crates/ironclaw_safety/src/leak_detector.rs

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

* Update crates/ironclaw_safety/src/leak_detector.rs

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

* fix(safety): address review feedback on credential patterns and path blocklist

- Move is_sensitive_path to ironclaw_safety crate for shared use
- Guard ListDirTool with sensitive path checks (including recursive traversal)
- Add missing sensitive paths: ~/.config/gh/hosts.yml, /etc/shadow,
  ~/.terraform.d/credentials.tfrc.json, ~/.azure/
- Add path traversal regression test and ListDirTool blocking test

Addresses review feedback from zmanian, gemini-code-assist, and copilot.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(tools): annotate sensitive dirs as blocked in recursive listing

When ListDirTool's recursive traversal encounters a sensitive directory,
annotate it with [sensitive - access blocked] so users understand why
its contents are suppressed.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(tools): integrate is_sensitive_path into shell tool file-access checking

Add defense-in-depth check for shell commands that read sensitive
credential files (cat, head, tail, less, cp, etc.). Extracts file
path arguments from known file-reading commands and checks them
against the shared is_sensitive_path function from ironclaw_safety.

This is best-effort — shell-level bypass via aliases, variable
expansion, or encoding is still possible. Full mitigation requires
filesystem-level sandboxing (seccomp/landlock).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(tools): add output redirection checks, fix segment splitting

Address zmanian's re-review findings:
- Check > and >> redirection targets against is_sensitive_path
  (write-path equivalent of read-path protection)
- Replace single-char & splitting with proper &&/|| aware parser
  to avoid fragmenting double operators
- Document subshell/command-substitution gap (partially covered by
  detect_command_injection upstream)
- Extract helpers: split_shell_segments, check_segment_file_commands,
  check_redirect_target, expand_tilde
- Add tests for output redirection, chained commands, segment splitting

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(safety): address PR #1675 review feedback and absorb #1713 patterns

Absorb #1713's sensitive path patterns into ironclaw_safety crate:
- Add shell history files, SSH key types, /etc/gshadow
- Add sensitive file extensions (.pem, .key, .p12, .pfx, .jks, .keystore)
- Add .dist safe suffix; smart .env matching (excludes .envrc, .environment)
- Directory-level blocking for .aws/, .docker/, .kube/ (not just specific files)
- Trailing-slash matching so bare directory paths trigger detection

Shell tool hardening:
- Strip surrounding quotes from tokens before sensitive path check
- Add grep, awk, sed to FILE_READ_COMMANDS
- Scan ALL redirect operators in a segment, not just the first

Other fixes:
- Add trailing \b to Telegram bot token regex to prevent over-matching
- Update error messages to reference secret_list/secret_create
- Strengthen ListDirTool test with tempfile-based .ssh directory

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(safety): close 4 adversarial bypass vectors in leak detector

- Fix .env suffix check: use exact remainder matching instead of
  ends_with, so .env.production.dist is no longer allowed through
- Detect process substitution <(...) in redirect checks and scan
  inner tokens for sensitive paths
- Add missing /id_rsa to SENSITIVE_PATH_PATTERNS (other SSH key
  types were already present)
- Check --flag=value tokens for sensitive paths instead of skipping
  all tokens starting with -

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* Fix 3 items from zmanian re-review on leak detector

1. Document blocking canonicalize() in async context: added comment
   explaining the trade-off (sub-ms on local FS, could block on NFS)
   with guidance to make async if needed.

2. Fix overly broad standalone key patterns: moved id_rsa, id_ed25519,
   id_ecdsa, id_dsa, authorized_keys, known_hosts from substring-based
   SENSITIVE_PATH_PATTERNS to exact filename matching via SENSITIVE_FILENAMES.
   This prevents false positives on paths like /project/grid_rsa_data while
   still blocking /project/test_fixtures/id_rsa.

3. Remove duplicate is_sensitive_path unit tests from file.rs: these
   belong in sensitive_paths.rs which already has comprehensive coverage.
   Kept integration-level tests that exercise tool execute() methods.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: j-bloggs <j-bloggs@users.noreply.github.com>
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
drchirag1991 pushed a commit to drchirag1991/ironclaw that referenced this pull request Apr 8, 2026
…arai#1675)

* fix(safety): add credential patterns and sensitive path blocklist

Addresses critical credential leakage found by security testing
(~/.ironclaw/tests/SECURITY_REPORT.md, test ce-02).

Leak detector (crates/ironclaw_safety/src/leak_detector.rs):
- Add OpenRouter API key pattern (sk-or-v1-<hex>)
- Add Anthropic OAuth token pattern (sk-ant-oat<NN>-<base64url>)
- Add Telegram bot token pattern (word-bounded, 8-12 digit bot ID)
- Add Groq API key pattern (gsk_<alphanumeric>)
- 12 new tests with synthetic keys (positive, false-positive, integration)

File tools (src/tools/builtin/file.rs):
- Add sensitive path blocklist to ReadFileTool, WriteFileTool, and
  ApplyPatchTool (defense-in-depth for all file access vectors)
- Blocks: .env (and .env.local/.env.production/etc.), .ssh/, .aws/,
  .netrc, .pgpass, .npmrc, .pypirc, .docker/config.json, .kube/config,
  .git-credentials, .gcloud/, .config/gcloud/, .gnupg/, .vault-token,
  .ironclaw/secrets/
- Allows .env.example, .env.template, .env.sample (safe suffixes)
- Case-insensitive; resolves symlinks via canonicalize() before check
- 8 new tests covering blocking, safe suffixes, .env variants, case

Known gap: shell tool can still `cat ~/.env` — different security
domain (denylist-gated in autonomous mode, user-initiated in
interactive mode). Tracked for follow-up.

Note: new patterns use .unwrap() on Regex::new() matching the
established convention of the 16 existing patterns in this file
(all use // safety: hardcoded literal). Follow-up to address the
existing .unwrap() debt across all patterns.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* Update src/tools/builtin/file.rs

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

* Update crates/ironclaw_safety/src/leak_detector.rs

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

* Update crates/ironclaw_safety/src/leak_detector.rs

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

* Update crates/ironclaw_safety/src/leak_detector.rs

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

* fix(safety): address review feedback on credential patterns and path blocklist

- Move is_sensitive_path to ironclaw_safety crate for shared use
- Guard ListDirTool with sensitive path checks (including recursive traversal)
- Add missing sensitive paths: ~/.config/gh/hosts.yml, /etc/shadow,
  ~/.terraform.d/credentials.tfrc.json, ~/.azure/
- Add path traversal regression test and ListDirTool blocking test

Addresses review feedback from zmanian, gemini-code-assist, and copilot.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(tools): annotate sensitive dirs as blocked in recursive listing

When ListDirTool's recursive traversal encounters a sensitive directory,
annotate it with [sensitive - access blocked] so users understand why
its contents are suppressed.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(tools): integrate is_sensitive_path into shell tool file-access checking

Add defense-in-depth check for shell commands that read sensitive
credential files (cat, head, tail, less, cp, etc.). Extracts file
path arguments from known file-reading commands and checks them
against the shared is_sensitive_path function from ironclaw_safety.

This is best-effort — shell-level bypass via aliases, variable
expansion, or encoding is still possible. Full mitigation requires
filesystem-level sandboxing (seccomp/landlock).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(tools): add output redirection checks, fix segment splitting

Address zmanian's re-review findings:
- Check > and >> redirection targets against is_sensitive_path
  (write-path equivalent of read-path protection)
- Replace single-char & splitting with proper &&/|| aware parser
  to avoid fragmenting double operators
- Document subshell/command-substitution gap (partially covered by
  detect_command_injection upstream)
- Extract helpers: split_shell_segments, check_segment_file_commands,
  check_redirect_target, expand_tilde
- Add tests for output redirection, chained commands, segment splitting

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(safety): address PR nearai#1675 review feedback and absorb nearai#1713 patterns

Absorb nearai#1713's sensitive path patterns into ironclaw_safety crate:
- Add shell history files, SSH key types, /etc/gshadow
- Add sensitive file extensions (.pem, .key, .p12, .pfx, .jks, .keystore)
- Add .dist safe suffix; smart .env matching (excludes .envrc, .environment)
- Directory-level blocking for .aws/, .docker/, .kube/ (not just specific files)
- Trailing-slash matching so bare directory paths trigger detection

Shell tool hardening:
- Strip surrounding quotes from tokens before sensitive path check
- Add grep, awk, sed to FILE_READ_COMMANDS
- Scan ALL redirect operators in a segment, not just the first

Other fixes:
- Add trailing \b to Telegram bot token regex to prevent over-matching
- Update error messages to reference secret_list/secret_create
- Strengthen ListDirTool test with tempfile-based .ssh directory

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(safety): close 4 adversarial bypass vectors in leak detector

- Fix .env suffix check: use exact remainder matching instead of
  ends_with, so .env.production.dist is no longer allowed through
- Detect process substitution <(...) in redirect checks and scan
  inner tokens for sensitive paths
- Add missing /id_rsa to SENSITIVE_PATH_PATTERNS (other SSH key
  types were already present)
- Check --flag=value tokens for sensitive paths instead of skipping
  all tokens starting with -

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* Fix 3 items from zmanian re-review on leak detector

1. Document blocking canonicalize() in async context: added comment
   explaining the trade-off (sub-ms on local FS, could block on NFS)
   with guidance to make async if needed.

2. Fix overly broad standalone key patterns: moved id_rsa, id_ed25519,
   id_ecdsa, id_dsa, authorized_keys, known_hosts from substring-based
   SENSITIVE_PATH_PATTERNS to exact filename matching via SENSITIVE_FILENAMES.
   This prevents false positives on paths like /project/grid_rsa_data while
   still blocking /project/test_fixtures/id_rsa.

3. Remove duplicate is_sensitive_path unit tests from file.rs: these
   belong in sensitive_paths.rs which already has comprehensive coverage.
   Kept integration-level tests that exercise tool execute() methods.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: j-bloggs <j-bloggs@users.noreply.github.com>
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
JZKK720 pushed a commit to JZKK720/ironclaw that referenced this pull request Apr 13, 2026
…arai#1675)

* fix(safety): add credential patterns and sensitive path blocklist

Addresses critical credential leakage found by security testing
(~/.ironclaw/tests/SECURITY_REPORT.md, test ce-02).

Leak detector (crates/ironclaw_safety/src/leak_detector.rs):
- Add OpenRouter API key pattern (sk-or-v1-<hex>)
- Add Anthropic OAuth token pattern (sk-ant-oat<NN>-<base64url>)
- Add Telegram bot token pattern (word-bounded, 8-12 digit bot ID)
- Add Groq API key pattern (gsk_<alphanumeric>)
- 12 new tests with synthetic keys (positive, false-positive, integration)

File tools (src/tools/builtin/file.rs):
- Add sensitive path blocklist to ReadFileTool, WriteFileTool, and
  ApplyPatchTool (defense-in-depth for all file access vectors)
- Blocks: .env (and .env.local/.env.production/etc.), .ssh/, .aws/,
  .netrc, .pgpass, .npmrc, .pypirc, .docker/config.json, .kube/config,
  .git-credentials, .gcloud/, .config/gcloud/, .gnupg/, .vault-token,
  .ironclaw/secrets/
- Allows .env.example, .env.template, .env.sample (safe suffixes)
- Case-insensitive; resolves symlinks via canonicalize() before check
- 8 new tests covering blocking, safe suffixes, .env variants, case

Known gap: shell tool can still `cat ~/.env` — different security
domain (denylist-gated in autonomous mode, user-initiated in
interactive mode). Tracked for follow-up.

Note: new patterns use .unwrap() on Regex::new() matching the
established convention of the 16 existing patterns in this file
(all use // safety: hardcoded literal). Follow-up to address the
existing .unwrap() debt across all patterns.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* Update src/tools/builtin/file.rs

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

* Update crates/ironclaw_safety/src/leak_detector.rs

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

* Update crates/ironclaw_safety/src/leak_detector.rs

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

* Update crates/ironclaw_safety/src/leak_detector.rs

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

* fix(safety): address review feedback on credential patterns and path blocklist

- Move is_sensitive_path to ironclaw_safety crate for shared use
- Guard ListDirTool with sensitive path checks (including recursive traversal)
- Add missing sensitive paths: ~/.config/gh/hosts.yml, /etc/shadow,
  ~/.terraform.d/credentials.tfrc.json, ~/.azure/
- Add path traversal regression test and ListDirTool blocking test

Addresses review feedback from zmanian, gemini-code-assist, and copilot.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(tools): annotate sensitive dirs as blocked in recursive listing

When ListDirTool's recursive traversal encounters a sensitive directory,
annotate it with [sensitive - access blocked] so users understand why
its contents are suppressed.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(tools): integrate is_sensitive_path into shell tool file-access checking

Add defense-in-depth check for shell commands that read sensitive
credential files (cat, head, tail, less, cp, etc.). Extracts file
path arguments from known file-reading commands and checks them
against the shared is_sensitive_path function from ironclaw_safety.

This is best-effort — shell-level bypass via aliases, variable
expansion, or encoding is still possible. Full mitigation requires
filesystem-level sandboxing (seccomp/landlock).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(tools): add output redirection checks, fix segment splitting

Address zmanian's re-review findings:
- Check > and >> redirection targets against is_sensitive_path
  (write-path equivalent of read-path protection)
- Replace single-char & splitting with proper &&/|| aware parser
  to avoid fragmenting double operators
- Document subshell/command-substitution gap (partially covered by
  detect_command_injection upstream)
- Extract helpers: split_shell_segments, check_segment_file_commands,
  check_redirect_target, expand_tilde
- Add tests for output redirection, chained commands, segment splitting

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(safety): address PR nearai#1675 review feedback and absorb nearai#1713 patterns

Absorb nearai#1713's sensitive path patterns into ironclaw_safety crate:
- Add shell history files, SSH key types, /etc/gshadow
- Add sensitive file extensions (.pem, .key, .p12, .pfx, .jks, .keystore)
- Add .dist safe suffix; smart .env matching (excludes .envrc, .environment)
- Directory-level blocking for .aws/, .docker/, .kube/ (not just specific files)
- Trailing-slash matching so bare directory paths trigger detection

Shell tool hardening:
- Strip surrounding quotes from tokens before sensitive path check
- Add grep, awk, sed to FILE_READ_COMMANDS
- Scan ALL redirect operators in a segment, not just the first

Other fixes:
- Add trailing \b to Telegram bot token regex to prevent over-matching
- Update error messages to reference secret_list/secret_create
- Strengthen ListDirTool test with tempfile-based .ssh directory

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(safety): close 4 adversarial bypass vectors in leak detector

- Fix .env suffix check: use exact remainder matching instead of
  ends_with, so .env.production.dist is no longer allowed through
- Detect process substitution <(...) in redirect checks and scan
  inner tokens for sensitive paths
- Add missing /id_rsa to SENSITIVE_PATH_PATTERNS (other SSH key
  types were already present)
- Check --flag=value tokens for sensitive paths instead of skipping
  all tokens starting with -

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* Fix 3 items from zmanian re-review on leak detector

1. Document blocking canonicalize() in async context: added comment
   explaining the trade-off (sub-ms on local FS, could block on NFS)
   with guidance to make async if needed.

2. Fix overly broad standalone key patterns: moved id_rsa, id_ed25519,
   id_ecdsa, id_dsa, authorized_keys, known_hosts from substring-based
   SENSITIVE_PATH_PATTERNS to exact filename matching via SENSITIVE_FILENAMES.
   This prevents false positives on paths like /project/grid_rsa_data while
   still blocking /project/test_fixtures/id_rsa.

3. Remove duplicate is_sensitive_path unit tests from file.rs: these
   belong in sensitive_paths.rs which already has comprehensive coverage.
   Kept integration-level tests that exercise tool execute() methods.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: j-bloggs <j-bloggs@users.noreply.github.com>
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
(cherry picked from commit e7fa167)
theredspoon pushed a commit to theredspoon/ironclaw that referenced this pull request Jun 21, 2026
…arai#1675)

* fix(safety): add credential patterns and sensitive path blocklist

Addresses critical credential leakage found by security testing
(~/.ironclaw/tests/SECURITY_REPORT.md, test ce-02).

Leak detector (crates/ironclaw_safety/src/leak_detector.rs):
- Add OpenRouter API key pattern (sk-or-v1-<hex>)
- Add Anthropic OAuth token pattern (sk-ant-oat<NN>-<base64url>)
- Add Telegram bot token pattern (word-bounded, 8-12 digit bot ID)
- Add Groq API key pattern (gsk_<alphanumeric>)
- 12 new tests with synthetic keys (positive, false-positive, integration)

File tools (src/tools/builtin/file.rs):
- Add sensitive path blocklist to ReadFileTool, WriteFileTool, and
  ApplyPatchTool (defense-in-depth for all file access vectors)
- Blocks: .env (and .env.local/.env.production/etc.), .ssh/, .aws/,
  .netrc, .pgpass, .npmrc, .pypirc, .docker/config.json, .kube/config,
  .git-credentials, .gcloud/, .config/gcloud/, .gnupg/, .vault-token,
  .ironclaw/secrets/
- Allows .env.example, .env.template, .env.sample (safe suffixes)
- Case-insensitive; resolves symlinks via canonicalize() before check
- 8 new tests covering blocking, safe suffixes, .env variants, case

Known gap: shell tool can still `cat ~/.env` — different security
domain (denylist-gated in autonomous mode, user-initiated in
interactive mode). Tracked for follow-up.

Note: new patterns use .unwrap() on Regex::new() matching the
established convention of the 16 existing patterns in this file
(all use // safety: hardcoded literal). Follow-up to address the
existing .unwrap() debt across all patterns.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* Update src/tools/builtin/file.rs

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

* Update crates/ironclaw_safety/src/leak_detector.rs

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

* Update crates/ironclaw_safety/src/leak_detector.rs

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

* Update crates/ironclaw_safety/src/leak_detector.rs

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

* fix(safety): address review feedback on credential patterns and path blocklist

- Move is_sensitive_path to ironclaw_safety crate for shared use
- Guard ListDirTool with sensitive path checks (including recursive traversal)
- Add missing sensitive paths: ~/.config/gh/hosts.yml, /etc/shadow,
  ~/.terraform.d/credentials.tfrc.json, ~/.azure/
- Add path traversal regression test and ListDirTool blocking test

Addresses review feedback from zmanian, gemini-code-assist, and copilot.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(tools): annotate sensitive dirs as blocked in recursive listing

When ListDirTool's recursive traversal encounters a sensitive directory,
annotate it with [sensitive - access blocked] so users understand why
its contents are suppressed.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(tools): integrate is_sensitive_path into shell tool file-access checking

Add defense-in-depth check for shell commands that read sensitive
credential files (cat, head, tail, less, cp, etc.). Extracts file
path arguments from known file-reading commands and checks them
against the shared is_sensitive_path function from ironclaw_safety.

This is best-effort — shell-level bypass via aliases, variable
expansion, or encoding is still possible. Full mitigation requires
filesystem-level sandboxing (seccomp/landlock).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(tools): add output redirection checks, fix segment splitting

Address zmanian's re-review findings:
- Check > and >> redirection targets against is_sensitive_path
  (write-path equivalent of read-path protection)
- Replace single-char & splitting with proper &&/|| aware parser
  to avoid fragmenting double operators
- Document subshell/command-substitution gap (partially covered by
  detect_command_injection upstream)
- Extract helpers: split_shell_segments, check_segment_file_commands,
  check_redirect_target, expand_tilde
- Add tests for output redirection, chained commands, segment splitting

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(safety): address PR nearai#1675 review feedback and absorb nearai#1713 patterns

Absorb nearai#1713's sensitive path patterns into ironclaw_safety crate:
- Add shell history files, SSH key types, /etc/gshadow
- Add sensitive file extensions (.pem, .key, .p12, .pfx, .jks, .keystore)
- Add .dist safe suffix; smart .env matching (excludes .envrc, .environment)
- Directory-level blocking for .aws/, .docker/, .kube/ (not just specific files)
- Trailing-slash matching so bare directory paths trigger detection

Shell tool hardening:
- Strip surrounding quotes from tokens before sensitive path check
- Add grep, awk, sed to FILE_READ_COMMANDS
- Scan ALL redirect operators in a segment, not just the first

Other fixes:
- Add trailing \b to Telegram bot token regex to prevent over-matching
- Update error messages to reference secret_list/secret_create
- Strengthen ListDirTool test with tempfile-based .ssh directory

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(safety): close 4 adversarial bypass vectors in leak detector

- Fix .env suffix check: use exact remainder matching instead of
  ends_with, so .env.production.dist is no longer allowed through
- Detect process substitution <(...) in redirect checks and scan
  inner tokens for sensitive paths
- Add missing /id_rsa to SENSITIVE_PATH_PATTERNS (other SSH key
  types were already present)
- Check --flag=value tokens for sensitive paths instead of skipping
  all tokens starting with -

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* Fix 3 items from zmanian re-review on leak detector

1. Document blocking canonicalize() in async context: added comment
   explaining the trade-off (sub-ms on local FS, could block on NFS)
   with guidance to make async if needed.

2. Fix overly broad standalone key patterns: moved id_rsa, id_ed25519,
   id_ecdsa, id_dsa, authorized_keys, known_hosts from substring-based
   SENSITIVE_PATH_PATTERNS to exact filename matching via SENSITIVE_FILENAMES.
   This prevents false positives on paths like /project/grid_rsa_data while
   still blocking /project/test_fixtures/id_rsa.

3. Remove duplicate is_sensitive_path unit tests from file.rs: these
   belong in sensitive_paths.rs which already has comprehensive coverage.
   Kept integration-level tests that exercise tool execute() methods.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: j-bloggs <j-bloggs@users.noreply.github.com>
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

contributor: core 20+ merged PRs risk: medium Business logic, config, or moderate-risk modules scope: tool/builtin Built-in tools size: L 200-499 changed lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants