sofos 0.2.3

use crate::error::{Result, SofosError};
use crate::tools::permissions::{CommandPermission, PermissionManager};
use crate::tools::utils::{
    MAX_TOOL_OUTPUT_TOKENS, TruncationKind, is_absolute_path, truncate_for_context,
};
use std::collections::HashSet;
use std::path::PathBuf;
use std::process::Command;
use std::sync::{Arc, Mutex};

const MAX_OUTPUT_SIZE: usize = 10 * 1024 * 1024; // 10MB limit

/// Return true when a command argument looks like a parent-directory
/// reference (`..`, `../foo`, `foo/..`, `foo/../bar`). Substring matches
/// inside opaque tokens — like git revision ranges (`HEAD~5..HEAD`) or
/// regex patterns (`\.\.\.`) — are intentionally NOT flagged: blocking
/// them was what stopped the AI from running legitimate git diagnostics
/// when a file ended up corrupted.
///
/// The command is split on whitespace and also on `=` / `:`, so that
/// flag-embedded traversals (`--include=../secret.h`) and PATH-style
/// assignments (`PATH=/usr/bin:../foo`) surface their `..` fragment as
/// its own token rather than hiding inside an opaque `KEY=VALUE`
/// string. Git range syntax (`HEAD~5..HEAD`, `HEAD^:path`) survives
/// the split because neither `..` nor `^` are delimiters here.
fn has_path_traversal(command: &str) -> bool {
    let split = |c: char| c.is_whitespace() || matches!(c, '=' | ':');
    for raw in command.split(split).filter(|t| !t.is_empty()) {
        // Strip the common shell wrappers the parser would peel off
        // anyway, so `"../foo"`, `` `../foo` ``, and `$(cat ../foo)`
        // all still flag as traversal after the trailing `)`, quote,
        // or backtick is removed.
        let t = raw.trim_matches(|c: char| {
            matches!(
                c,
                '"' | '\'' | '`' | '(' | ')' | '{' | '}' | '[' | ']' | ';' | ','
            )
        });
        if t == ".." || t.starts_with("../") || t.ends_with("/..") || t.contains("/../") {
            return true;
        }
    }
    false
}

/// Return true when `op` appears as a command-name prefix anywhere in
/// `command` — at the start, or immediately after a shell
/// command-boundary sequence. The set of boundaries must cover every
/// place the shell can start executing a new command, because this
/// function gates our forbidden-git detection: if we miss one, the
/// model can wrap `git push` in that construct and bypass the check.
///
/// Covered: plain space, `;`, `&&`, `||`, `|`, backtick substitution
/// (`` `git push` ``), `$(...)` command substitution, `(...)` subshell,
/// `{...; }` group. False positives (e.g. `ls {git,svn}` brace
/// expansion triggering `git` detection) are acceptable — the worst
/// outcome is the user being prompted to confirm a benign command.
fn command_contains_op(command: &str, op: &str) -> bool {
    const BOUNDARIES: &[&str] = &[" ", ";", "&&", "||", "|", "`", "$(", "(", "{"];
    if command.starts_with(op) {
        return true;
    }
    BOUNDARIES
        .iter()
        .any(|sep| command.contains(&format!("{sep}{op}")))
}

/// Convert Unix signal number to human-readable name
#[cfg(unix)]
fn signal_name(sig: i32) -> &'static str {
    match sig {
        1 => "SIGHUP",
        2 => "SIGINT",
        3 => "SIGQUIT",
        4 => "SIGILL",
        6 => "SIGABRT",
        8 => "SIGFPE",
        9 => "SIGKILL",
        11 => "SIGSEGV",
        13 => "SIGPIPE",
        14 => "SIGALRM",
        15 => "SIGTERM",
        _ => "unknown",
    }
}

#[derive(Clone)]
pub struct BashExecutor {
    workspace: PathBuf,
    /// Whether interactive prompts (stdin) are available
    interactive: bool,
    /// Session-scoped temporary permissions (not persisted to config)
    session_allowed: Arc<Mutex<HashSet<String>>>,
    session_denied: Arc<Mutex<HashSet<String>>>,
    /// Session-scoped Bash path grants for external directories
    bash_path_session_allowed: Arc<Mutex<HashSet<String>>>,
    bash_path_session_denied: Arc<Mutex<HashSet<String>>>,
}

impl BashExecutor {
    pub fn new(workspace: PathBuf, interactive: bool) -> Result<Self> {
        Ok(Self {
            workspace,
            interactive,
            session_allowed: Arc::new(Mutex::new(HashSet::new())),
            session_denied: Arc::new(Mutex::new(HashSet::new())),
            bash_path_session_allowed: Arc::new(Mutex::new(HashSet::new())),
            bash_path_session_denied: Arc::new(Mutex::new(HashSet::new())),
        })
    }

    pub fn execute(&self, command: &str) -> Result<String> {
        let normalized = format!("Bash({})", command.trim());

        // Check session-scoped decisions first (for "allow once" / "deny once")
        if let Ok(allowed) = self.session_allowed.lock() {
            if allowed.contains(&normalized) {
                // Previously allowed this session, skip permission check
                return self.execute_after_permission_check(command);
            }
        }
        if let Ok(denied) = self.session_denied.lock() {
            if denied.contains(&normalized) {
                return Err(SofosError::ToolExecution(format!(
                    "User already declined '{}' earlier this session. \
                     Propose a different approach or ask the user to clarify \
                     rather than retrying the same command.",
                    command
                )));
            }
        }

        let mut permission_manager = PermissionManager::new(self.workspace.clone())?;
        let permission = permission_manager.check_command_permission(command)?;

        match permission {
            CommandPermission::Allowed => {
                // Command is in allowed list, execute directly
            }
            CommandPermission::Denied => {
                return Err(SofosError::ToolExecution(
                    self.get_rejection_reason(command),
                ));
            }
            CommandPermission::Ask => {
                let (allowed, remember) = permission_manager.ask_user_permission(command)?;
                if !allowed {
                    if !remember {
                        // Store session-scoped denial
                        if let Ok(mut denied) = self.session_denied.lock() {
                            denied.insert(normalized);
                        }
                    }
                    return Err(SofosError::ToolExecution(format!(
                        "User declined '{}'. Propose a different approach or \
                         ask the user to clarify rather than retrying the same \
                         command.",
                        command
                    )));
                }
                if !remember {
                    // Store session-scoped allowance
                    if let Ok(mut allowed) = self.session_allowed.lock() {
                        allowed.insert(normalized);
                    }
                }
            }
        }

        self.execute_after_permission_check(command)
    }

    fn execute_after_permission_check(&self, command: &str) -> Result<String> {
        let mut permission_manager = PermissionManager::new(self.workspace.clone())?;

        // Enforce read permissions on paths referenced in the command
        self.enforce_read_permissions(&permission_manager, command)?;

        // Non-path structural safety checks (parent traversal, redirection, git restrictions)
        if !self.is_safe_command_structure(command) {
            return Err(SofosError::ToolExecution(
                self.get_rejection_reason(command),
            ));
        }

        // Commands that aren't destructive enough to hard-deny but
        // mutate working-tree state in a way the user should see before
        // it happens — e.g. `git checkout <branch>` switches branches,
        // `git checkout HEAD~N` detaches HEAD, `git checkout -- <path>`
        // overwrites uncommitted changes. Fires AFTER the structural
        // hard-deny above so `git checkout -f` / `git checkout -b`
        // stay hard-blocked instead of being askable.
        self.confirm_askable_command(command)?;

        // Check external paths in command — ask user for paths not covered by Bash path grants
        self.check_bash_external_paths(command, &mut permission_manager)?;

        let output = Command::new("sh")
            .arg("-c")
            .arg(command)
            .current_dir(&self.workspace)
            .output()
            .map_err(|e| SofosError::ToolExecution(format!("Failed to execute command: {}", e)))?;

        if output.stdout.len() > MAX_OUTPUT_SIZE {
            return Err(SofosError::ToolExecution(format!(
                "Command output too large ({} bytes). Maximum size is {} MB",
                output.stdout.len(),
                MAX_OUTPUT_SIZE / (1024 * 1024)
            )));
        }

        if output.stderr.len() > MAX_OUTPUT_SIZE {
            return Err(SofosError::ToolExecution(format!(
                "Command error output too large ({} bytes). Maximum size is {} MB",
                output.stderr.len(),
                MAX_OUTPUT_SIZE / (1024 * 1024)
            )));
        }

        let stdout = String::from_utf8_lossy(&output.stdout);
        let stderr = String::from_utf8_lossy(&output.stderr);

        if !output.status.success() {
            let exit_info = match output.status.code() {
                Some(code) => format!("exit code: {}", code),
                None => {
                    #[cfg(unix)]
                    {
                        use std::os::unix::process::ExitStatusExt;
                        match output.status.signal() {
                            Some(sig) => format!("signal: {} ({})", sig, signal_name(sig)),
                            None => "unknown termination".to_string(),
                        }
                    }
                    #[cfg(not(unix))]
                    {
                        "unknown termination".to_string()
                    }
                }
            };
            let error_output = format!(
                "Command failed with {}\nSTDOUT:\n{}\nSTDERR:\n{}",
                exit_info, stdout, stderr
            );
            return Ok(truncate_for_context(
                &error_output,
                MAX_TOOL_OUTPUT_TOKENS,
                TruncationKind::BashOutput,
            ));
        }

        let mut result = String::new();
        if !stdout.is_empty() {
            result.push_str("STDOUT:\n");
            result.push_str(&stdout);
        }
        if !stderr.is_empty() {
            if !result.is_empty() {
                result.push('\n');
            }
            result.push_str("STDERR:\n");
            result.push_str(&stderr);
        }

        if result.is_empty() {
            result = "Command executed successfully (no output)".to_string();
        }

        Ok(truncate_for_context(
            &result,
            MAX_TOOL_OUTPUT_TOKENS,
            TruncationKind::BashOutput,
        ))
    }

    /// Prompt the user before running commands that mutate working-tree
    /// state in a way that's easy to overlook. Currently just
    /// `git checkout <anything>` — plain branch switches, detached-HEAD
    /// checkouts, and `git checkout -- <path>` file recovery all land
    /// here. Hard-denied variants (`git checkout -f`, `git checkout -b`)
    /// are filtered out earlier by `is_safe_command_structure`.
    ///
    /// Declining the prompt aborts the command. Accepting is scoped to
    /// this one invocation — the user has to confirm each `git
    /// checkout` explicitly, matching `confirm_destructive`'s policy of
    /// "no remember button for working-tree mutations".
    fn confirm_askable_command(&self, command: &str) -> Result<()> {
        const ASKABLE_PREFIXES: &[&str] = &["git checkout"];

        let matches = ASKABLE_PREFIXES
            .iter()
            .any(|prefix| command_contains_op(command, prefix));
        if !matches {
            return Ok(());
        }

        if !self.interactive {
            return Err(SofosError::ToolExecution(format!(
                "Command '{}' requires interactive confirmation\n\
                 Hint: `git checkout` prompts before running because it switches branches \
                 (or overwrites working-tree files). Run sofos interactively to confirm.",
                command
            )));
        }

        let prompt = format!("Run bash command: {}", command);
        if !crate::tools::utils::confirm_destructive(&prompt)? {
            return Err(SofosError::ToolExecution(format!(
                "User declined '{}'. Propose a different approach or ask \
                 the user to clarify rather than retrying the same command.",
                command
            )));
        }
        Ok(())
    }

    /// Check all external paths (absolute or tilde) in a command against Bash path grants.
    /// Asks the user interactively for any paths not yet covered.
    fn check_bash_external_paths(
        &self,
        command: &str,
        permission_manager: &mut PermissionManager,
    ) -> Result<()> {
        for token in command.split_whitespace() {
            let cleaned = token
                .trim_matches('"')
                .trim_matches('\'')
                .trim_matches(';')
                .trim();

            if cleaned.is_empty() {
                continue;
            }

            // `--flag=/path` and `--flag=~/path` tokens would otherwise
            // be swallowed whole by the `starts_with('-')` filter below,
            // so split at the first `=` to expose the path half. Without
            // this, `grep --include=/etc/passwd` bypasses the external-
            // path prompt entirely.
            let path_candidate = if cleaned.starts_with('-') {
                match cleaned.find('=') {
                    Some(i) => cleaned[i + 1..].trim_matches(|c: char| matches!(c, '"' | '\'')),
                    None => continue,
                }
            } else {
                cleaned
            };

            // Check tilde before absolute so `~` / `~/foo` get expanded
            // first. `is_absolute_path` catches Unix (`/foo`) and
            // Windows (`C:\foo`, `\\server\share`) shapes on every
            // platform — `Path::is_absolute` alone would miss Unix
            // paths on Windows, letting a bash command referencing
            // `/etc/passwd` bypass the external-path prompt when the
            // binary runs there.
            if path_candidate.starts_with("~/") || path_candidate == "~" {
                let expanded = PermissionManager::expand_tilde_pub(path_candidate);
                self.check_bash_external_path(&expanded, permission_manager)?;
            } else if is_absolute_path(path_candidate) {
                self.check_bash_external_path(path_candidate, permission_manager)?;
            }
        }

        Ok(())
    }

    /// Check a single external path against Bash path grants; ask user if not covered.
    fn check_bash_external_path(
        &self,
        path: &str,
        permission_manager: &mut PermissionManager,
    ) -> Result<()> {
        // Canonicalize to resolve symlinks (e.g. /var/folders -> /private/var/folders on macOS)
        let resolved = std::fs::canonicalize(path)
            .map(|p| p.to_string_lossy().to_string())
            .unwrap_or_else(|_| path.to_string());
        let check_path = resolved.as_str();

        // Enforce deny rules first (takes priority over allow)
        if permission_manager.is_bash_path_denied(check_path) {
            return Err(SofosError::ToolExecution(format!(
                "Bash access denied for path '{}'\n\
                 Hint: Blocked by deny rule in .sofos/config.local.toml or ~/.sofos/config.toml",
                check_path
            )));
        }

        // Already allowed by config?
        if permission_manager.is_bash_path_allowed(check_path) {
            return Ok(());
        }

        let path_obj = std::path::Path::new(check_path);

        // Session allowed?
        if let Ok(allowed_dirs) = self.bash_path_session_allowed.lock() {
            for dir in allowed_dirs.iter() {
                if path_obj.starts_with(std::path::Path::new(dir)) {
                    return Ok(());
                }
            }
        }

        // Session denied?
        if let Ok(denied_dirs) = self.bash_path_session_denied.lock() {
            for dir in denied_dirs.iter() {
                if path_obj.starts_with(std::path::Path::new(dir)) {
                    return Err(SofosError::ToolExecution(format!(
                        "Bash access denied for path '{}' (denied earlier this session)",
                        check_path
                    )));
                }
            }
        }

        let parent = std::path::Path::new(check_path)
            .parent()
            .and_then(|p| p.to_str())
            .unwrap_or(check_path);

        // Non-interactive mode (tests, piped input): deny with a config hint
        if !self.interactive {
            return Err(SofosError::ToolExecution(format!(
                "Command references path '{}' outside workspace\n\
                 Hint: Add Bash({}/**) to 'allow' list in .sofos/config.local.toml",
                check_path, parent
            )));
        }

        // Ask user interactively
        let (allowed, remember) = permission_manager.ask_user_path_permission("Bash", parent)?;

        if allowed {
            if !remember {
                if let Ok(mut dirs) = self.bash_path_session_allowed.lock() {
                    dirs.insert(parent.to_string());
                }
            }
            Ok(())
        } else {
            if !remember {
                if let Ok(mut dirs) = self.bash_path_session_denied.lock() {
                    dirs.insert(parent.to_string());
                }
            }
            Err(SofosError::ToolExecution(format!(
                "Bash access denied by user for path '{}'",
                check_path
            )))
        }
    }

    fn enforce_read_permissions(
        &self,
        permission_manager: &PermissionManager,
        command: &str,
    ) -> Result<()> {
        // Heuristic-based detection of file paths in commands.
        // Checks paths against Read deny rules (regardless of Bash path grants).
        // External path access is handled separately by check_bash_external_paths.
        for token in command.split_whitespace().skip(1) {
            let cleaned = token
                .trim_matches('"')
                .trim_matches('\'')
                .trim_matches(';')
                .trim();

            if cleaned.is_empty() || cleaned.starts_with('-') {
                continue;
            }

            let is_path = cleaned.contains('/')
                || cleaned.starts_with('.')
                || cleaned.starts_with('~')
                || (!cleaned.contains('$')
                    && !cleaned.contains('`')
                    && !cleaned.contains('*')
                    && !cleaned.contains('?')
                    && !cleaned.contains('['));

            if is_path {
                // For deny rules: check if explicitly denied
                let (perm, matched_rule) =
                    permission_manager.check_read_permission_with_source(cleaned);
                match perm {
                    CommandPermission::Allowed => {}
                    CommandPermission::Denied => {
                        let config_source = if let Some(ref rule) = matched_rule {
                            permission_manager.get_rule_source(rule)
                        } else {
                            ".sofos/config.local.toml or ~/.sofos/config.toml".to_string()
                        };
                        return Err(SofosError::ToolExecution(format!(
                            "Read access denied for path '{}' in command\n\
                             Hint: Blocked by deny rule in {}",
                            cleaned, config_source
                        )));
                    }
                    CommandPermission::Ask => {
                        return Err(SofosError::ToolExecution(format!(
                            "Path '{}' requires confirmation per config file\n\
                             Hint: Move it to 'allow' or 'deny' list.",
                            cleaned
                        )));
                    }
                }
            }
        }

        Ok(())
    }

    fn is_safe_command_structure(&self, command: &str) -> bool {
        // Parent directory traversal — always blocked (use absolute paths for external access)
        if has_path_traversal(command) {
            return false;
        }

        // Note: absolute paths (/...) and tilde paths (~/) are now handled by
        // check_bash_external_paths which asks the user interactively.

        // Allow "2>&1" (stderr to stdout redirection) but block file output redirection
        let command_without_stderr_redirect = command.replace("2>&1", "");

        if command_without_stderr_redirect.contains('>')
            || command_without_stderr_redirect.contains(">>")
        {
            return false;
        }

        if command.contains("<<") {
            return false;
        }

        if !self.is_safe_git_command(&command.to_lowercase()) {
            return false;
        }

        true
    }

    fn is_safe_git_command(&self, command: &str) -> bool {
        if !command.starts_with("git ")
            && !command.contains(" git ")
            && !command.contains(";git ")
            && !command.contains("&&git ")
            && !command.contains("||git ")
            && !command.contains("|git ")
        {
            return true;
        }

        // Allow safe git stash read-only operations
        if command.contains("git stash list") || command.contains("git stash show") {
            return true;
        }

        // Dangerous git operations that are completely blocked.
        //
        // File-recovery commands (`git restore <path>`, `git checkout --`)
        // are intentionally NOT on this list any more: when `morph_edit_file`
        // or `edit_file` corrupts a file, the model needs a way to roll back
        // to HEAD without going through the write tools (which would just
        // write whatever broken content the model already has). These
        // commands only affect specified paths, so the blast radius is
        // bounded by whichever path the model names.
        let dangerous_git_ops = [
            "git push",
            "git pull",
            "git fetch",
            "git clone",
            "git clean",
            "git reset --hard",
            "git reset --mixed",
            "git checkout -f",
            "git checkout -b",
            "git branch -d",
            "git branch -D",
            "git branch -m",
            "git branch -M",
            "git remote add",
            "git remote set-url",
            "git remote remove",
            "git remote rm",
            "git submodule",
            "git filter-branch",
            "git gc",
            "git prune",
            "git update-ref",
            "git send-email",
            "git apply",
            "git am",
            "git cherry-pick",
            "git revert",
            "git commit",
            "git merge",
            "git rebase",
            "git tag -d",
            "git stash",
            "git init",
            "git add",
            "git rm",
            "git mv",
            "git switch",
        ];

        for dangerous_op in &dangerous_git_ops {
            if command_contains_op(command, dangerous_op) {
                return false;
            }
        }

        true
    }

    fn get_rejection_reason(&self, command: &str) -> String {
        let command_lower = command.to_lowercase();

        if has_path_traversal(command) {
            return format!(
                "Command '{}' contains '..' as a path component (parent directory traversal)\n\
                 Hint: Use absolute paths for external directory access instead of '..'. \
                 Git revision ranges like `HEAD~5..HEAD` are allowed.",
                command
            );
        }

        if !self.is_safe_git_command(&command_lower) {
            return self.get_git_rejection_reason(command);
        }

        let command_without_stderr_redirect = command.replace("2>&1", "");
        if command_without_stderr_redirect.contains('>')
            || command_without_stderr_redirect.contains(">>")
        {
            return format!(
                "Command '{}' contains output redirection ('>' or '>>')\n\
                 Hint: Use write_file tool to create or edit_file/morph_edit_file to modify files. Note: '2>&1' is allowed.",
                command
            );
        }

        if command.contains("<<") {
            return format!(
                "Command '{}' contains here-doc ('<<')\n\
                 Hint: Use write_file tool to create files instead.",
                command
            );
        }

        format!(
            "Command '{}' is in the forbidden list (destructive or violates sandbox)\n\
             Hint: Use appropriate file operation tools instead.",
            command
        )
    }

    fn get_git_rejection_reason(&self, command: &str) -> String {
        let command_lower = command.to_lowercase();

        if command_lower.contains("git push") {
            return format!(
                "Command '{}' blocked: 'git push' sends data to remote repositories\n\
                 Hint: Use 'git status', 'git log', 'git diff' to view changes.",
                command
            );
        }

        if command_lower.contains("git pull") || command_lower.contains("git fetch") {
            let op = if command_lower.contains("git pull") {
                "git pull"
            } else {
                "git fetch"
            };
            return format!(
                "Command '{}' blocked: '{}' fetches data from remote repositories\n\
                 Hint: Use 'git status', 'git log', 'git diff' to view local changes.",
                command, op
            );
        }

        if command_lower.contains("git clone") {
            return format!(
                "Command '{}' blocked: 'git clone' downloads repositories\n\
                 Hint: Clone repositories manually outside of Sofos.",
                command
            );
        }

        if command_lower.contains("git commit") || command_lower.contains("git add") {
            let op = if command_lower.contains("git commit") {
                "git commit"
            } else {
                "git add"
            };
            return format!(
                "Command '{}' blocked: '{}' modifies the git repository\n\
                 Hint: Use 'git status', 'git diff' to view changes. Create commits manually.",
                command, op
            );
        }

        if command_lower.contains("git reset") || command_lower.contains("git clean") {
            let op = if command_lower.contains("git reset") {
                "git reset"
            } else {
                "git clean"
            };
            return format!(
                "Command '{}' blocked: '{}' is a destructive operation\n\
                 Hint: Use 'git status', 'git log', 'git diff' to view repository state.",
                command, op
            );
        }

        if command_lower.contains("git checkout") || command_lower.contains("git switch") {
            let op = if command_lower.contains("git checkout") {
                "git checkout"
            } else {
                "git switch"
            };
            return format!(
                "Command '{}' blocked: '{}' changes branches or modifies working directory\n\
                 Hint: Use 'git branch' to list branches, 'git status' to see current branch.",
                command, op
            );
        }

        if command_lower.contains("git merge") || command_lower.contains("git rebase") {
            let op = if command_lower.contains("git merge") {
                "git merge"
            } else {
                "git rebase"
            };
            return format!(
                "Command '{}' blocked: '{}' modifies git history\n\
                 Hint: Perform merges/rebases manually outside of Sofos.",
                command, op
            );
        }

        if command_lower.contains("git stash")
            && !command_lower.contains("git stash list")
            && !command_lower.contains("git stash show")
        {
            return format!(
                "Command '{}' blocked: 'git stash' modifies repository state\n\
                 Hint: Use 'git stash list' or 'git stash show' to view stashed changes.",
                command
            );
        }

        if command_lower.contains("git remote add") || command_lower.contains("git remote set-url")
        {
            return format!(
                "Command '{}' blocked: Modifying git remotes is not allowed\n\
                 Hint: Use 'git remote -v' to view configured remotes.",
                command
            );
        }

        if command_lower.contains("git submodule") {
            return format!(
                "Command '{}' blocked: 'git submodule' can fetch from remote repositories\n\
                 Hint: Manage submodules manually outside of Sofos.",
                command
            );
        }

        format!(
            "Command '{}' blocked: git operation modifies repository or accesses network\n\
             Hint: Allowed git commands: status, log, diff, show, branch, remote -v, grep, blame",
            command
        )
    }
}

#[cfg(test)]
mod tests {
    use super::*;

    /// `command_contains_op` gates our forbidden-git detection. A miss
    /// here is a real security bypass: the model could wrap `git push`
    /// in a subshell or command substitution and slip past.
    #[test]
    fn command_contains_op_catches_shell_boundaries() {
        assert!(command_contains_op("git push", "git push"));
        assert!(command_contains_op("ls; git push", "git push"));
        assert!(command_contains_op("ls && git push", "git push"));
        assert!(command_contains_op("ls || git push", "git push"));
        assert!(command_contains_op("ls | git push", "git push"));

        // Shell-substitution boundaries — the regressions from the audit.
        assert!(command_contains_op("echo hi; `git push`", "git push"));
        assert!(command_contains_op("echo $(git push)", "git push"));
        assert!(command_contains_op("(git push)", "git push"));
        assert!(command_contains_op("{ git push; }", "git push"));

        // Genuinely unrelated commands shouldn't trigger.
        assert!(!command_contains_op("rgit push", "git push")); // non-boundary prefix
        assert!(!command_contains_op("ls", "git push"));
    }

    #[test]
    fn test_safe_commands() {
        let executor = BashExecutor::new(PathBuf::from("."), false).unwrap();

        // Note: These tests check the command structure safety only
        // Actual permission checking is done by PermissionManager
        assert!(executor.is_safe_command_structure("ls -la"));
        assert!(executor.is_safe_command_structure("cat file.txt"));
        assert!(executor.is_safe_command_structure("grep pattern file.txt"));
        assert!(executor.is_safe_command_structure("cargo test"));
        assert!(executor.is_safe_command_structure("cargo build"));
        assert!(executor.is_safe_command_structure("echo hello"));
        assert!(executor.is_safe_command_structure("pwd"));

        // Test that 2>&1 is allowed (combines stderr to stdout)
        assert!(executor.is_safe_command_structure("cargo build 2>&1"));
        assert!(executor.is_safe_command_structure("npm test 2>&1"));
        assert!(executor.is_safe_command_structure("ls 2>&1 | grep error"));
        assert!(executor.is_safe_command_structure("cargo test 2>&1"));
    }

    #[test]
    fn test_unsafe_command_structures() {
        let executor = BashExecutor::new(PathBuf::from("."), false).unwrap();

        // Test structural safety issues (not permission-based)
        assert!(!executor.is_safe_command_structure("echo hello > file.txt"));
        assert!(!executor.is_safe_command_structure("cat file.txt >> output.txt"));

        // These should still be blocked (file redirection even with 2>&1)
        assert!(!executor.is_safe_command_structure("echo hello > file.txt 2>&1"));
        assert!(!executor.is_safe_command_structure("cargo build 2>&1 > output.txt"));
    }

    #[test]
    fn test_path_traversal_blocked() {
        let executor = BashExecutor::new(PathBuf::from("."), false).unwrap();

        assert!(!executor.is_safe_command_structure("cat ../file.txt"));
        assert!(!executor.is_safe_command_structure("ls ../../etc"));
        assert!(!executor.is_safe_command_structure("cat ../../../etc/passwd"));
        assert!(!executor.is_safe_command_structure("cat file.txt && ls .."));
        assert!(!executor.is_safe_command_structure("ls | cat ../secret"));
    }

    #[test]
    fn test_absolute_paths_pass_structural_check() {
        // Absolute paths are no longer blocked by is_safe_command_structure.
        // They are handled by check_bash_external_paths which asks the user.
        let executor = BashExecutor::new(PathBuf::from("."), false).unwrap();

        assert!(executor.is_safe_command_structure("/bin/ls"));
        assert!(executor.is_safe_command_structure("cat /etc/passwd"));
        assert!(executor.is_safe_command_structure("ls /tmp"));
        assert!(executor.is_safe_command_structure("cat /home/user/file"));
    }

    #[test]
    fn test_output_size_limit() {
        use tempfile;

        let temp_dir = tempfile::tempdir().unwrap();
        let executor = BashExecutor::new(temp_dir.path().to_path_buf(), false).unwrap();

        let result = executor.execute("seq 1 2000000");

        assert!(result.is_err());
        if let Err(SofosError::ToolExecution(msg)) = result {
            assert!(msg.contains("too large"));
            assert!(msg.contains("10 MB"));
        } else {
            panic!("Expected ToolExecution error");
        }
    }

    #[test]
    fn test_read_permission_blocks_cat() {
        use std::fs;
        use tempfile::tempdir;

        let temp_dir = tempdir().unwrap();

        // Write deny config for test folder reads
        let config_dir = temp_dir.path().join(".sofos");
        fs::create_dir_all(&config_dir).unwrap();
        fs::write(
            config_dir.join("config.local.toml"),
            r#"[permissions]
allow = []
deny = ["Read(./test/**)"]
ask = []
"#,
        )
        .unwrap();

        let executor = BashExecutor::new(temp_dir.path().to_path_buf(), false).unwrap();

        // Even without creating the file, permission check should block before execution
        let result = executor.execute("cat ./test/secret.txt");

        assert!(result.is_err());
        if let Err(SofosError::ToolExecution(msg)) = result {
            assert!(msg.contains("Read access denied") || msg.contains("denied"));
        } else {
            panic!("Expected ToolExecution error");
        }
    }

    #[test]
    fn test_safe_git_commands() {
        let executor = BashExecutor::new(PathBuf::from("."), false).unwrap();

        // Safe read-only git commands
        assert!(executor.is_safe_command_structure("git status"));
        assert!(executor.is_safe_command_structure("git log"));
        assert!(executor.is_safe_command_structure("git log --oneline"));
        assert!(executor.is_safe_command_structure("git diff"));
        assert!(executor.is_safe_command_structure("git diff HEAD~1"));
        assert!(executor.is_safe_command_structure("git show"));
        assert!(executor.is_safe_command_structure("git show HEAD"));
        assert!(executor.is_safe_command_structure("git branch"));
        assert!(executor.is_safe_command_structure("git branch -v"));
        assert!(executor.is_safe_command_structure("git branch --list"));
        assert!(executor.is_safe_command_structure("git remote -v"));
        assert!(executor.is_safe_command_structure("git config --list"));
        assert!(executor.is_safe_command_structure("git ls-files"));
        assert!(executor.is_safe_command_structure("git ls-tree HEAD"));
        assert!(executor.is_safe_command_structure("git blame file.txt"));
        assert!(executor.is_safe_command_structure("git grep pattern"));
        assert!(executor.is_safe_command_structure("git rev-parse HEAD"));
        assert!(executor.is_safe_command_structure("git describe --tags"));
        assert!(executor.is_safe_command_structure("git stash list"));
        assert!(executor.is_safe_command_structure("git stash show"));
        assert!(executor.is_safe_command_structure("git stash show stash@{0}"));
        // File-recovery commands — allowed so the model can roll back a
        // botched edit without going through the write tools.
        assert!(executor.is_safe_command_structure("git restore file.txt"));
        assert!(executor.is_safe_command_structure("git restore src/foo.rs"));
        assert!(executor.is_safe_command_structure("git checkout -- file.txt"));
        assert!(executor.is_safe_command_structure("git checkout HEAD -- src/foo.rs"));
        // Revision ranges (`HEAD~5..HEAD`) are not path traversal —
        // they're opaque token substrings that used to be blocked by
        // the old substring check on `..`.
        assert!(executor.is_safe_command_structure("git log HEAD~5..HEAD"));
        assert!(executor.is_safe_command_structure("git diff HEAD~1..HEAD"));
        assert!(executor.is_safe_command_structure("git log HEAD~5..HEAD -- src/foo.rs"));
    }

    #[test]
    fn test_path_traversal_token_detection() {
        // Literal path-traversal tokens — all blocked.
        assert!(has_path_traversal("cd .."));
        assert!(has_path_traversal("cat ../file"));
        assert!(has_path_traversal("ls ../../etc"));
        assert!(has_path_traversal("cat /foo/..")); // ends_with /..
        assert!(has_path_traversal("cat foo/../bar")); // contains /../
        // Quoted / shell-wrapped variants — still blocked after the
        // trailing paren, quote, or backtick is stripped.
        assert!(has_path_traversal("cat \"../secret\""));
        assert!(has_path_traversal("cat '../secret'"));
        assert!(has_path_traversal("echo $(cat ../secret)"));
        assert!(has_path_traversal("ls `../bin/tool`"));

        // Flag-embedded / assignment-embedded traversal. These used
        // to slip through the token-only split because the entire
        // `KEY=VALUE` arg was a single opaque token.
        assert!(has_path_traversal("clang --include=../secret.h file.c"));
        assert!(has_path_traversal("PATH=/usr/bin:../foo cmd"));
        assert!(has_path_traversal("FOO=.. cmd"));

        // Opaque tokens that happen to contain `..` — allowed. These
        // are the false positives the old `contains("..")` check
        // produced and broke legitimate git diagnostics.
        assert!(!has_path_traversal("git log HEAD~5..HEAD"));
        assert!(!has_path_traversal("git diff HEAD~1..HEAD -- src/foo.rs"));
        assert!(!has_path_traversal("grep '\\.\\.\\.' file.txt"));
        assert!(!has_path_traversal("ls foo..bar")); // unusual filename, not traversal
        // Git colon path syntax survives the `:` split because
        // neither `HEAD` nor the path contain a traversal fragment.
        assert!(!has_path_traversal("git show HEAD:src/foo.rs"));
        assert!(!has_path_traversal("git show HEAD~5:src/foo.rs"));
    }

    #[test]
    fn test_dangerous_git_commands() {
        let executor = BashExecutor::new(PathBuf::from("."), false).unwrap();

        // Remote operations (data leakage risk)
        assert!(!executor.is_safe_command_structure("git push"));
        assert!(!executor.is_safe_command_structure("git push origin main"));
        assert!(!executor.is_safe_command_structure("git push --force"));
        assert!(!executor.is_safe_command_structure("git pull"));
        assert!(!executor.is_safe_command_structure("git pull origin main"));
        assert!(!executor.is_safe_command_structure("git fetch"));
        assert!(!executor.is_safe_command_structure("git fetch origin"));
        assert!(!executor.is_safe_command_structure("git clone https://example.com/repo.git"));

        // Destructive local operations
        assert!(!executor.is_safe_command_structure("git clean -fd"));
        assert!(!executor.is_safe_command_structure("git reset --hard"));
        assert!(!executor.is_safe_command_structure("git reset --hard HEAD~1"));
        assert!(!executor.is_safe_command_structure("git checkout -f"));
        assert!(!executor.is_safe_command_structure("git checkout -b newbranch"));
        assert!(!executor.is_safe_command_structure("git branch -D branch-name"));
        assert!(!executor.is_safe_command_structure("git branch -d branch-name"));
        assert!(!executor.is_safe_command_structure("git filter-branch"));

        // Modifications
        assert!(!executor.is_safe_command_structure("git add ."));
        assert!(!executor.is_safe_command_structure("git add file.txt"));
        assert!(!executor.is_safe_command_structure("git commit -m 'message'"));
        assert!(!executor.is_safe_command_structure("git commit --amend"));
        assert!(!executor.is_safe_command_structure("git rm file.txt"));
        assert!(!executor.is_safe_command_structure("git mv old.txt new.txt"));
        assert!(!executor.is_safe_command_structure("git merge branch"));
        assert!(!executor.is_safe_command_structure("git rebase main"));
        assert!(!executor.is_safe_command_structure("git cherry-pick abc123"));
        assert!(!executor.is_safe_command_structure("git revert abc123"));
        assert!(!executor.is_safe_command_structure("git switch main"));

        // Remote configuration changes
        assert!(
            !executor.is_safe_command_structure("git remote add origin https://evil.com/repo.git")
        );
        assert!(
            !executor
                .is_safe_command_structure("git remote set-url origin https://evil.com/repo.git")
        );
        assert!(!executor.is_safe_command_structure("git remote remove origin"));

        // Submodules (can fetch from remote)
        assert!(!executor.is_safe_command_structure("git submodule update"));
        assert!(!executor.is_safe_command_structure("git submodule init"));

        // Stash operations (modify state)
        assert!(!executor.is_safe_command_structure("git stash"));
        assert!(!executor.is_safe_command_structure("git stash pop"));
        assert!(!executor.is_safe_command_structure("git stash apply"));
        assert!(!executor.is_safe_command_structure("git stash drop"));
        assert!(!executor.is_safe_command_structure("git stash clear"));

        // Init (creates repository)
        assert!(!executor.is_safe_command_structure("git init"));
        assert!(!executor.is_safe_command_structure("git init new-repo"));
    }

    #[test]
    fn test_git_commands_in_chains() {
        let executor = BashExecutor::new(PathBuf::from("."), false).unwrap();

        // Safe commands in chains
        assert!(executor.is_safe_command_structure("git status && git log"));
        assert!(executor.is_safe_command_structure("git diff | grep pattern"));
        assert!(executor.is_safe_command_structure("echo test; git status"));

        // Dangerous commands in chains
        assert!(!executor.is_safe_command_structure("git status && git push"));
        assert!(!executor.is_safe_command_structure("git log | git commit -m 'test'"));
        assert!(!executor.is_safe_command_structure("echo test; git add ."));
        assert!(!executor.is_safe_command_structure("git status || git pull"));
    }

    #[test]
    fn test_error_messages_are_informative() {
        let executor = BashExecutor::new(PathBuf::from("."), false).unwrap();

        let reason = executor.get_git_rejection_reason("git push origin main");
        assert!(reason.contains("git push origin main"));
        assert!(reason.contains("remote repositories"));
        assert!(reason.contains("git status"));
    }

    #[test]
    fn test_tilde_paths_pass_structural_check() {
        // Tilde paths are no longer blocked by is_safe_command_structure.
        // They are handled by check_bash_external_paths which asks the user.
        let executor = BashExecutor::new(PathBuf::from("."), false).unwrap();

        assert!(executor.is_safe_command_structure("ls ~/tmp"));
        assert!(executor.is_safe_command_structure("cat ~/file.txt"));
        assert!(executor.is_safe_command_structure("grep pattern ~/docs/file.txt"));
    }

    #[test]
    fn test_git_checkout_requires_confirmation_non_interactive() {
        // Plain `git checkout <branch>` isn't destructive enough to hard-
        // deny (git refuses dirty-tree switches), but it mutates
        // working-tree state in a way the user should see before it
        // runs. In non-interactive mode (tests, piped stdin) there's no
        // way to prompt, so the executor returns a clear error pointing
        // at the interactive-mode requirement.
        let temp_dir = tempfile::tempdir().unwrap();
        let executor = BashExecutor::new(temp_dir.path().to_path_buf(), false).unwrap();

        for cmd in &[
            "git checkout main",
            "git checkout HEAD~3",
            "git checkout -- src/lib.rs",
        ] {
            let result = executor.execute(cmd);
            assert!(
                result.is_err(),
                "expected confirmation gate to deny `{}` in non-interactive mode",
                cmd
            );
            if let Err(SofosError::ToolExecution(msg)) = result {
                assert!(
                    msg.contains("confirmation"),
                    "expected 'confirmation' hint for `{}`, got: {}",
                    cmd,
                    msg
                );
            } else {
                panic!(
                    "expected ToolExecution error for `{}`, got: {:?}",
                    cmd, result
                );
            }
        }
    }

    #[test]
    fn test_git_checkout_force_stays_hard_denied() {
        // `git checkout -f` and `git checkout -b` must reject BEFORE the
        // confirmation gate — they're in `dangerous_git_ops` and stay
        // in the hard-deny tier even with the new askable mechanism.
        // The error message mentions the dangerous-op reason, not the
        // interactive-confirmation hint.
        let temp_dir = tempfile::tempdir().unwrap();
        let executor = BashExecutor::new(temp_dir.path().to_path_buf(), false).unwrap();

        for cmd in &["git checkout -f main", "git checkout -b new-branch"] {
            let result = executor.execute(cmd);
            assert!(result.is_err(), "`{}` must stay hard-denied", cmd);
            if let Err(SofosError::ToolExecution(msg)) = result {
                assert!(
                    !msg.contains("requires interactive confirmation"),
                    "`{}` should be hard-denied, not askable — got: {}",
                    cmd,
                    msg
                );
            }
        }
    }

    #[test]
    fn test_flag_embedded_external_path_is_checked() {
        // `--include=/etc/passwd` previously slipped past the external-path
        // prompt because the whole token was filtered by `starts_with('-')`.
        // The path portion is now extracted and routed to
        // `check_bash_external_path`, which deny in non-interactive mode
        // when no grant is configured.
        let temp_dir = tempfile::tempdir().unwrap();
        let executor = BashExecutor::new(temp_dir.path().to_path_buf(), false).unwrap();

        let result = executor.execute("grep --include=/etc/passwd pattern .");

        assert!(result.is_err(), "expected external-path rejection");
        if let Err(SofosError::ToolExecution(msg)) = result {
            assert!(
                msg.contains("outside workspace"),
                "expected 'outside workspace' in error, got: {msg}"
            );
        } else {
            panic!("Expected ToolExecution error, got: {result:?}");
        }
    }

    #[test]
    fn test_session_scoped_permissions_persist() {
        let executor = BashExecutor::new(PathBuf::from("."), false).unwrap();

        // Simulate adding a command to session_allowed
        {
            let mut allowed = executor.session_allowed.lock().unwrap();
            allowed.insert("Bash(my_custom_cmd)".to_string());
        }

        // Verify it's recognized on subsequent check
        {
            let allowed = executor.session_allowed.lock().unwrap();
            assert!(allowed.contains("Bash(my_custom_cmd)"));
        }

        // Simulate adding a command to session_denied
        {
            let mut denied = executor.session_denied.lock().unwrap();
            denied.insert("Bash(blocked_cmd)".to_string());
        }

        // Verify denied is recognized
        {
            let denied = executor.session_denied.lock().unwrap();
            assert!(denied.contains("Bash(blocked_cmd)"));
        }
    }

    #[test]
    fn test_session_permissions_shared_across_clones() {
        let executor1 = BashExecutor::new(PathBuf::from("."), false).unwrap();
        let executor2 = executor1.clone();

        // Add permission via executor1
        {
            let mut allowed = executor1.session_allowed.lock().unwrap();
            allowed.insert("Bash(shared_cmd)".to_string());
        }

        // Verify executor2 sees it (Arc sharing)
        {
            let allowed = executor2.session_allowed.lock().unwrap();
            assert!(allowed.contains("Bash(shared_cmd)"));
        }
    }
}