Skip to content

Fix security audit findings: fail-safe exit codes, interpreter allowlist, and hardening#21

Merged
sheeki03 merged 1 commit intomainfrom
security-audit-fixes
Feb 6, 2026
Merged

Fix security audit findings: fail-safe exit codes, interpreter allowlist, and hardening#21
sheeki03 merged 1 commit intomainfrom
security-audit-fixes

Conversation

@sheeki03
Copy link
Owner

@sheeki03 sheeki03 commented Feb 6, 2026

Summary

Six security fixes from external audit, plus Issue #20 bash enter mode infrastructure:

  • SEC-CRIT-01: Handle unexpected tirith exit codes in all 4 shell hooks (bash/zsh/fish/PowerShell). Bash enter mode degrades to preexec via persistent safe mode; zsh/fish/PS warn + execute; all paste paths fail-closed (discard).
  • SEC-HIGH-01: Interpreter allowlist in runner.rs — two-tier matching (exact names like sh/bash + versioned families like python3.11). Check runs before user confirmation prompt.
  • SEC-HIGH-03: Bounded stdin read in paste.rs — 1 MiB cap via Read::take().
  • SEC-HIGH-04: File permissions 0600 on audit log, receipts, cache, and last-trigger files (new files via OpenOptionsExt::mode() + legacy hardening via set_permissions).
  • SEC-MED-03: SHA256 hex validation in receipt load()/save()/verify() — prevents path traversal via crafted hash values.
  • Short-hash panic guard: short_hash() via truncate_bytes() replaces all &sha256[..12] slice operations.

Also includes bash enter mode infrastructure (bind-x Enter override, startup health gate, persistent safe mode, PROMPT_COMMAND delivery, degrade-to-preexec) and CR normalization improvements.

230 tests passing (148 unit + 44 integration + 18 golden + 17 policy + 3 CLI).

Test plan

  • cargo fmt --all clean
  • cargo clippy --workspace -- -D warnings clean
  • cargo test --workspace — 230/230 pass
  • Shell hook asset copies match source (embedded_shell_hooks_match_repo_hooks)
  • PTY-backed bash degrade test (bash_hook_unexpected_rc_degrades_in_pty)
  • Oversized paste rejection (paste_oversized_input_rejected)
  • Invalid SHA256 receipt rejection (receipt_verify_invalid_sha256_rejected)
  • Branch logic tests for zsh/fish unexpected exit codes
  • File permission tests (audit log, receipt, cache — all 0600)
  • Manual: echo "test" | tirith paste --shell posix → exit 0
  • Manual: yes | head -c 2000000 | tirith paste --shell posix → "exceeds 1 MiB"
  • Manual: tirith receipt verify "../../etc/passwd" → "invalid sha256"

@macroscopeapp
Copy link

macroscopeapp bot commented Feb 6, 2026

Enforce fail-safe exit codes, interpreter allowlist, and 0600 file permissions while adding bash safe-mode reset and a 1 MiB paste limit across shell hooks and CLI

Introduce explicit rc handling in shell hooks for command and paste, add interpreter allowlisting in tirith-core::runner::is_allowed_interpreter, set 0600 permissions for audit, cache, receipts, and temp files, cap paste input at 1 MiB, and add persistent bash safe-mode with cli::doctor::run reset support. Update tests and docs accordingly.

📍Where to Start

Start with interpreter gating in tirith-core at runner::run and helpers in crates/tirith-core/src/runner.rs, then review bash enter-mode degradation and safe-mode in crates/tirith/assets/shell/lib/bash-hook.bash.


Macroscope summarized 9f2984c.

@sheeki03 sheeki03 force-pushed the security-audit-fixes branch from e7550a1 to 383e66c Compare February 6, 2026 18:29
let mut open_opts = OpenOptions::new();
open_opts.create(true).append(true);
use std::os::unix::fs::OpenOptionsExt;
open_opts.mode(0o600);
Copy link

Choose a reason for hiding this comment

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

🟢 Low

src/audit.rs:175 The test assumes mode(0o600) produces exactly 0o600 permissions, but the process umask is ANDed against the mode. Consider either saving/restoring umask around the test or documenting why this is acceptable for your CI environment.

🚀 Want me to fix this? Reply ex: "fix it for me".

@sheeki03 sheeki03 force-pushed the security-audit-fixes branch from 383e66c to 418603c Compare February 6, 2026 18:38
const ALLOWED_FAMILIES: &[&str] = &["python", "ruby", "perl", "node"];

fn is_allowed_interpreter(interpreter: &str) -> bool {
let base = interpreter.rsplit('/').next().unwrap_or(interpreter);
Copy link

Choose a reason for hiding this comment

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

🟢 Low

src/runner.rs:34 On Windows, paths use \ as separators, so rsplit('/') won't extract the basename correctly. Consider using std::path::Path::file_name() or splitting on both separators.

-    let base = interpreter.rsplit('/').next().unwrap_or(interpreter);
+    let base = std::path::Path::new(interpreter)
+        .file_name()
+        .and_then(|s| s.to_str())
+        .unwrap_or(interpreter);

🚀 Want me to fix this? Reply ex: "fix it for me".


#[cfg(unix)]
#[test]
fn test_receipt_save_permissions_0600() {
Copy link

Choose a reason for hiding this comment

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

🟢 Low

src/receipt.rs:190 This test manually recreates file creation logic rather than calling save(). If save() has a bug setting permissions, this test would still pass. Consider calling Receipt::save() directly and checking the resulting file's permissions.

🚀 Want me to fix this? Reply ex: "fix it for me".

@sheeki03 sheeki03 force-pushed the security-audit-fixes branch 5 times, most recently from dbf4dd5 to ac6de5f Compare February 6, 2026 19:38
@sheeki03 sheeki03 force-pushed the security-audit-fixes branch 2 times, most recently from 68c2479 to 3768713 Compare February 6, 2026 20:04
…ist, and hardening

Six security fixes from external audit:

- SEC-CRIT-01: Handle unexpected tirith exit codes in all shell hooks.
  Bash enter mode degrades to preexec; zsh/fish/PS warn+execute;
  all paste paths fail-closed (discard).
- SEC-HIGH-01: Interpreter allowlist in runner.rs — two-tier matching
  (exact names + versioned families like python3.11). Check runs before
  user confirmation prompt.
- SEC-HIGH-03: Bounded stdin read in paste.rs — 1 MiB cap via Read::take().
- SEC-HIGH-04: File permissions 0600 on audit log, receipts, cache, and
  last-trigger files (new files + legacy hardening via set_permissions).
- SEC-MED-03: SHA256 hex validation in receipt load/save/verify —
  prevents path traversal via crafted hash values.
- Short-hash panic guard: safe truncation via truncate_bytes() replaces
  all &sha256[..12] slice operations.

Also includes Issue #20 bash enter mode infrastructure (bind-x override,
startup health gate, persistent safe mode, degrade-to-preexec) and
CR normalization improvements from prior work on this branch.

Additional fixes:
- Fix audit test env var race (test OpenOptions pattern directly)
- Trim whitespace from XDG_STATE_HOME in state_dir()
- Bump version to 0.1.9

230 tests passing (148 unit + 44 integration + 18 golden + 17 policy + 3 CLI).
@sheeki03 sheeki03 force-pushed the security-audit-fixes branch from 3768713 to 9f2984c Compare February 6, 2026 20:30
@sheeki03 sheeki03 merged commit 34c7778 into main Feb 6, 2026
10 checks passed
This was referenced Feb 6, 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.

1 participant