From 3e8f83bb396a939805343ab295548bf940588f07 Mon Sep 17 00:00:00 2001 From: Widthdom Date: Wed, 23 Sep 2026 06:34:13 +0900 Subject: [PATCH 1/3] Fix quoted DLL path detection in command guards (#5417) --- .agent_harness/command_guard_core.py | 54 ++++++++- .agent_harness/guard_policy_contract.json | 140 ++++++++++++++++++++++ AGENT_GUIDE.md | 1 + TESTING_GUIDE.md | 12 ++ changelog.d/unreleased/5417.fixed.md | 18 +++ 5 files changed, 222 insertions(+), 3 deletions(-) create mode 100644 changelog.d/unreleased/5417.fixed.md diff --git a/.agent_harness/command_guard_core.py b/.agent_harness/command_guard_core.py index c3b4acf489..42a5ca8431 100644 --- a/.agent_harness/command_guard_core.py +++ b/.agent_harness/command_guard_core.py @@ -301,9 +301,11 @@ def _deny(reason: str) -> GuardDecision: return GuardDecision(False, reason) -def _split_command(command: str) -> list[str]: +def _split_command(command: str, *, preserve_newlines: bool = False) -> list[str]: try: - lexer = shlex.shlex(command, posix=True, punctuation_chars=True) + lexer = shlex.shlex(command, posix=True, punctuation_chars="();<>|&\n" if preserve_newlines else True) + if preserve_newlines: + lexer.whitespace = lexer.whitespace.replace("\n", "") lexer.whitespace_split = True lexer.commenters = "" return list(lexer) @@ -366,6 +368,52 @@ def _command_is_safe_local_cdidx(command: str, cwd: Path, project_root: Path) -> return dll == expected +def _has_active_command_substitution(command: str) -> bool: + quote: str | None = None + index = 0 + while index < len(command): + char = command[index] + if char == "\\" and quote != "'": + index += 2 + continue + if char in {"'", '"'}: + if quote is None: + quote = char + elif quote == char: + quote = None + elif quote != "'" and (char == "`" or command.startswith("$(", index)): + return True + elif quote is None and command[index : index + 2] in {"<(", ">("}: + return True + index += 1 + return False + + +def _command_invokes_cdidx_dll(command: str) -> bool: + tokens = _split_command(command, preserve_newlines=True) + # Fail closed for malformed commands and active substitutions; single-quoted + # Markdown/backticks are data, but double-quoted substitutions execute code. + if not tokens or _has_active_command_substitution(command): + return bool(LOCAL_CDIDX_DLL_RE.search(command)) + + tokens = [";" if token and set(token) <= set("();<>|&\n") else token for token in tokens] + for segment in _token_segments(tokens): + segment = _strip_transparent_script_wrappers(_strip_leading_env_assignments(segment)) + while segment and segment[0] in _SHELL_COMMAND_PREFIX_KEYWORDS: + segment = _strip_transparent_script_wrappers(_strip_leading_env_assignments(segment[1:])) + if not segment: + continue + if LOCAL_CDIDX_DLL_RE.search(segment[0]): + return True + # Keep unsupported dotnet host forms (including `dotnet exec`) denied. + # Arguments to unrelated programs, such as review prompts, are data. + if _token_command_name(segment[0]) == "dotnet" and any( + LOCAL_CDIDX_DLL_RE.search(arg) for arg in segment[1:] + ): + return True + return False + + def _token_is_expanded_installed_cdidx(token: str, cwd: Path) -> bool: if not token.startswith("/"): return False @@ -1267,7 +1315,7 @@ def evaluate_bash_command(command: str, cwd: Path, project_root: Path) -> GuardD if _command_mentions_local_cdidx(command, project_root): return _deny("local cdidx commands must not use shell control operators or command substitutions") - if LOCAL_CDIDX_DLL_RE.search(command): + if _command_invokes_cdidx_dll(command): return _deny("use dotnet ./src/CodeIndex/bin/Debug/net8.0/cdidx.dll instead") if _command_is_safe_cdidx_resolver(command): diff --git a/.agent_harness/guard_policy_contract.json b/.agent_harness/guard_policy_contract.json index fd18fb4edd..b62ddd4126 100644 --- a/.agent_harness/guard_policy_contract.json +++ b/.agent_harness/guard_policy_contract.json @@ -14,6 +14,146 @@ "command": "dotnet ./src/CodeIndex/bin/Debug/net8.0/cdidx.dll search SymbolExtractor", "expected": "allow" }, + { + "name": "review_prompt_local_dll_single_quoted", + "command": "codex exec --sandbox read-only --ephemeral --output-last-message /tmp/issue5417-review.txt 'For source discovery/search use only the repository-built binary dotnet ./src/CodeIndex/bin/Debug/net8.0/cdidx.dll with --db .cdidx/codeindex.db'", + "expected": "allow" + }, + { + "name": "review_prompt_local_dll_double_quoted", + "command": "codex exec --sandbox read-only \"Use dotnet ./src/CodeIndex/bin/Debug/net8.0/cdidx.dll with --db .cdidx/codeindex.db\"", + "expected": "allow" + }, + { + "name": "review_prompt_local_dll_multiline_markdown", + "command": "codex exec --sandbox read-only 'Review the change.\nUse `dotnet ./src/CodeIndex/bin/Debug/net8.0/cdidx.dll` for discovery.'", + "expected": "allow" + }, + { + "name": "document_body_local_dll_literal_argument", + "command": "gh pr create --title Update --body 'dotnet ./src/CodeIndex/bin/Debug/net8.0/cdidx.dll'", + "expected": "allow" + }, + { + "name": "local_dll_read_operand", + "command": "ls -l './src/CodeIndex/bin/Debug/net8.0/cdidx.dll'", + "expected": "allow" + }, + { + "name": "local_cdidx_quoted_operand", + "command": "dotnet './src/CodeIndex/bin/Debug/net8.0/cdidx.dll' status", + "expected": "allow" + }, + { + "name": "wrapped_review_prompt_local_dll", + "command": "env REVIEW_MODE=readonly timeout 30 codex exec 'Use dotnet ./src/CodeIndex/bin/Debug/net8.0/cdidx.dll'", + "expected": "allow" + }, + { + "name": "unsupported_cdidx_dll", + "command": "dotnet ./src/CodeIndex/bin/Release/net8.0/cdidx.dll status", + "expected": "deny" + }, + { + "name": "unsupported_cdidx_dll_fragmented_quotes", + "command": "dotnet ./src/CodeIndex/bin/Release/net8.0/cdi''dx.dll status", + "expected": "deny" + }, + { + "name": "absolute_dotnet_host_cdidx_dll", + "command": "/usr/local/bin/dotnet ./src/CodeIndex/bin/Debug/net8.0/cdidx.dll status", + "expected": "deny" + }, + { + "name": "local_cdidx_trailing_command", + "command": "dotnet ./src/CodeIndex/bin/Debug/net8.0/cdidx.dll status; true", + "expected": "deny" + }, + { + "name": "direct_cdidx_dll", + "command": "'./src/CodeIndex/bin/Debug/net8.0/cdidx.dll' status", + "expected": "deny" + }, + { + "name": "dotnet_exec_cdidx_dll", + "command": "dotnet exec './src/CodeIndex/bin/Debug/net8.0/cdidx.dll' status", + "expected": "deny" + }, + { + "name": "wrapped_local_cdidx_dll", + "command": "env REVIEW_MODE=readonly timeout 30 dotnet ./src/CodeIndex/bin/Debug/net8.0/cdidx.dll status", + "expected": "deny" + }, + { + "name": "split_string_local_cdidx_dll", + "command": "env -S 'dotnet ./src/CodeIndex/bin/Debug/net8.0/cdidx.dll status'", + "expected": "deny" + }, + { + "name": "chained_local_cdidx_dll", + "command": "true && dotnet ./src/CodeIndex/bin/Debug/net8.0/cdidx.dll status", + "expected": "deny" + }, + { + "name": "newline_local_cdidx_dll", + "command": "true\ndotnet ./src/CodeIndex/bin/Debug/net8.0/cdidx.dll status", + "expected": "deny" + }, + { + "name": "subshell_local_cdidx_dll", + "command": "true;(dotnet ./src/CodeIndex/bin/Debug/net8.0/cdidx.dll status)", + "expected": "deny" + }, + { + "name": "process_substitution_local_cdidx_dll", + "command": "cat <(dotnet ./src/CodeIndex/bin/Debug/net8.0/cdidx.dll status)", + "expected": "deny" + }, + { + "name": "conditional_local_cdidx_dll", + "command": "if dotnet ./src/CodeIndex/bin/Debug/net8.0/cdidx.dll status; then true; fi", + "expected": "deny" + }, + { + "name": "inline_shell_local_cdidx_dll", + "command": "bash -lc 'dotnet ./src/CodeIndex/bin/Debug/net8.0/cdidx.dll status'", + "expected": "deny" + }, + { + "name": "eval_local_cdidx_dll", + "command": "eval 'dotnet ./src/CodeIndex/bin/Debug/net8.0/cdidx.dll status'", + "expected": "deny" + }, + { + "name": "review_prompt_dll_command_substitution", + "command": "codex exec \"Review $(dotnet ./src/CodeIndex/bin/Debug/net8.0/cdidx.dll status)\"", + "expected": "deny" + }, + { + "name": "review_prompt_dll_backtick_substitution", + "command": "codex exec \"Review `dotnet ./src/CodeIndex/bin/Debug/net8.0/cdidx.dll status`\"", + "expected": "deny" + }, + { + "name": "review_prompt_dll_literal_substitution", + "command": "codex exec 'Review $(dotnet ./src/CodeIndex/bin/Debug/net8.0/cdidx.dll status)'", + "expected": "allow" + }, + { + "name": "review_prompt_dll_escaped_substitution", + "command": "codex exec \"Review \\$(dotnet ./src/CodeIndex/bin/Debug/net8.0/cdidx.dll status)\"", + "expected": "allow" + }, + { + "name": "review_prompt_dll_unterminated_quote", + "command": "codex exec 'Use dotnet ./src/CodeIndex/bin/Debug/net8.0/cdidx.dll", + "expected": "deny" + }, + { + "name": "global_cdidx_execution", + "command": "cdidx status", + "expected": "deny" + }, { "name": "repo_local_installer_doctor", "command": "bash ./install.sh --doctor v1.2.3", diff --git a/AGENT_GUIDE.md b/AGENT_GUIDE.md index ed291c47fe..06cfaba651 100644 --- a/AGENT_GUIDE.md +++ b/AGENT_GUIDE.md @@ -69,6 +69,7 @@ Command-search enforcement is tool-specific and adapter-driven: - Codex uses `.codex/hooks.json`, which invokes `.codex/hooks/bash_guard.py` and `.codex/hooks/permission_request_guard.py`. - Claude Code uses `.claude/settings.json`, which invokes `.claude/hooks/bash-guard.py`. - Both Bash guard adapters delegate shared command policy to `.agent_harness/command_guard_core.py`; update the shared core for common command policy and review both adapters only when tool-specific behavior changes. +- Quoted review prompts and document arguments may mention the repository-built `cdidx.dll` path. The DLL check classifies command positions rather than rejecting every textual mention; unsupported DLL invocations, execution wrappers, and active shell substitutions remain blocked. Other guard checks still apply to the entire command. - Codex uses the `codeindex_workspace` permission profile for workspace writes plus limited GitHub CLI network access to `github.com` and `api.github.com`. - Codex may use normal development GitHub CLI commands including `gh issue list/view/create/edit/comment`, `gh pr list/view/create/edit/comment/ready/close`, `gh repo view`, and `gh status`. - Keep `gh auth`, `gh api`, `gh secret`, `gh release`, `gh repo create`, `gh repo fork`, `gh repo delete`, and `gh pr merge` blocked. `gh api` is blocked because arbitrary REST/GraphQL calls can bypass subcommand-level policy intent; `gh pr merge` is blocked because it mutates remote PR state in a high-risk way. diff --git a/TESTING_GUIDE.md b/TESTING_GUIDE.md index dc6de04232..885bea61e1 100644 --- a/TESTING_GUIDE.md +++ b/TESTING_GUIDE.md @@ -1,5 +1,11 @@ # Testing Guide +Run `python3 -m unittest discover -s .agent_harness/tests` for shared command-guard +changes. The existing policy-contract matrix checks the core, Codex PreToolUse and +PermissionRequest, and Claude PreToolUse without executing the tested commands. +Keep #5417's quoted review/document data, multiline Markdown and literal substitution +controls alongside real DLL invocations, wrappers, shell operators and active substitutions. + `JsonEnvelopeWrapperIssue5412Tests` shares an isolated indexed fixture across literal, line-regex and multiline find cursor errors. Keep JSON/envelope/fields/compact selectors, malformed/mismatched/stale cursors, human and count controls, and exact @@ -1826,6 +1832,12 @@ Issue #5300 のテストは隣接・入れ子の C# callable、対象行の除 # テストガイド +共有コマンドガードの変更では `python3 -m unittest discover -s .agent_harness/tests` を +実行してください。既存の契約テストは、対象コマンド自体を実行せずに共有コア、Codex の +PreToolUse・PermissionRequest、Claude の PreToolUse を検証します。#5417 の引用された +レビュー・文書データ、複数行 Markdown、置換構文の文字列としての使用とともに、実際の DLL +実行、ラッパー、シェル演算子、有効なコマンド置換の対照例を維持してください。 + `JsonEnvelopeWrapperIssue5412Tests` は分離した索引 fixture を共有し、リテラル・行単位正規表現・ 複数行 find のカーソルエラーを検証します。JSON・envelope・fields・compact の各指定、 不正・不一致・世代変更済みカーソル、人向け出力と件数の対照、pretty 出力と末尾改行を含む diff --git a/changelog.d/unreleased/5417.fixed.md b/changelog.d/unreleased/5417.fixed.md new file mode 100644 index 0000000000..ebee0da5c0 --- /dev/null +++ b/changelog.d/unreleased/5417.fixed.md @@ -0,0 +1,18 @@ +--- +category: fixed +issues: + - 5417 +affected: + - .agent_harness/command_guard_core.py + - .agent_harness/guard_policy_contract.json + - AGENT_GUIDE.md + - TESTING_GUIDE.md +--- + +## English + +- **Allow local DLL paths in quoted review prompts (#5417)** — the shared Codex/Claude command guard now distinguishes prompt and document arguments from executable positions. Required reviews can mention the repository-built `cdidx.dll` while unsupported DLL execution, execution wrappers and active shell substitutions remain blocked. + +## 日本語 + +- **引用されたレビュー文中のローカル DLL パスを許可 (#5417)** — Codex／Claude の共有コマンドガードが、プロンプト・文書の引数と実行位置を区別するようになりました。必須レビューでリポジトリ内ビルドの `cdidx.dll` に言及でき、未対応の DLL 実行、実行ラッパー、有効なシェル置換の拒否は維持されます。 From a1c52d81221390107679cd7a9e584f2c3cea392e Mon Sep 17 00:00:00 2001 From: Widthdom Date: Wed, 23 Sep 2026 06:42:27 +0900 Subject: [PATCH 2/3] Preserve conservative DLL guard denials after review (#5417) --- .agent_harness/command_guard_core.py | 72 +++++++++++-------- .agent_harness/guard_policy_contract.json | 87 ++++++++++++++++++++++- AGENT_GUIDE.md | 2 +- TESTING_GUIDE.md | 6 ++ changelog.d/unreleased/5417.fixed.md | 4 +- 5 files changed, 137 insertions(+), 34 deletions(-) diff --git a/.agent_harness/command_guard_core.py b/.agent_harness/command_guard_core.py index 42a5ca8431..394925b672 100644 --- a/.agent_harness/command_guard_core.py +++ b/.agent_harness/command_guard_core.py @@ -301,11 +301,9 @@ def _deny(reason: str) -> GuardDecision: return GuardDecision(False, reason) -def _split_command(command: str, *, preserve_newlines: bool = False) -> list[str]: +def _split_command(command: str) -> list[str]: try: - lexer = shlex.shlex(command, posix=True, punctuation_chars="();<>|&\n" if preserve_newlines else True) - if preserve_newlines: - lexer.whitespace = lexer.whitespace.replace("\n", "") + lexer = shlex.shlex(command, posix=True, punctuation_chars=True) lexer.whitespace_split = True lexer.commenters = "" return list(lexer) @@ -368,7 +366,7 @@ def _command_is_safe_local_cdidx(command: str, cwd: Path, project_root: Path) -> return dll == expected -def _has_active_command_substitution(command: str) -> bool: +def _has_active_shell_syntax(command: str) -> bool: quote: str | None = None index = 0 while index < len(command): @@ -383,35 +381,49 @@ def _has_active_command_substitution(command: str) -> bool: quote = None elif quote != "'" and (char == "`" or command.startswith("$(", index)): return True - elif quote is None and command[index : index + 2] in {"<(", ">("}: + elif quote is None and char in "();<>|&\n#": return True index += 1 return False -def _command_invokes_cdidx_dll(command: str) -> bool: - tokens = _split_command(command, preserve_newlines=True) - # Fail closed for malformed commands and active substitutions; single-quoted - # Markdown/backticks are data, but double-quoted substitutions execute code. - if not tokens or _has_active_command_substitution(command): - return bool(LOCAL_CDIDX_DLL_RE.search(command)) - - tokens = [";" if token and set(token) <= set("();<>|&\n") else token for token in tokens] - for segment in _token_segments(tokens): - segment = _strip_transparent_script_wrappers(_strip_leading_env_assignments(segment)) - while segment and segment[0] in _SHELL_COMMAND_PREFIX_KEYWORDS: - segment = _strip_transparent_script_wrappers(_strip_leading_env_assignments(segment[1:])) - if not segment: - continue - if LOCAL_CDIDX_DLL_RE.search(segment[0]): - return True - # Keep unsupported dotnet host forms (including `dotnet exec`) denied. - # Arguments to unrelated programs, such as review prompts, are data. - if _token_command_name(segment[0]) == "dotnet" and any( - LOCAL_CDIDX_DLL_RE.search(arg) for arg in segment[1:] - ): - return True - return False +def _command_has_unsupported_cdidx_dll(command: str) -> bool: + tokens = _split_command(command) + if not LOCAL_CDIDX_DLL_RE.search(command) and not any(LOCAL_CDIDX_DLL_RE.search(token) for token in tokens): + return False + # This is a bounded data-argument exception, not a general shell parser. + # Preserve denial for unknown executors, wrappers, comments, redirections + # and compound commands. Quoted/escaped punctuation stays literal data. + if not tokens or _has_active_shell_syntax(command): + return True + if tokens[0] in {"echo", "printf", "ls"}: + return False + if tokens[0] == "codex" and len(tokens) >= 3 and tokens[1] in {"exec", "review"}: + prompt, rest = _subcommand_args( + tokens[2:], + {"--sandbox", "-s", "--output-last-message", "-o", "--output-schema", "--model", "-m", + "--profile", "-p", "--config", "-c", "--cd", "-C", "--image", "-i", "--color", + "--enable", "--disable", "--base", "--commit", "--title"}, + valueless_options={"--ephemeral", "--json", "--skip-git-repo-check", "--uncommitted"}, + fail_on_unknown_option=True, + ) + return not ( + prompt == tokens[-1] and not rest + and not any(LOCAL_CDIDX_DLL_RE.search(token) for token in tokens[:-1]) + ) + if tokens[0] == "gh": + subcommand, _ = _subcommand_args( + tokens[1:], {"-R", "--repo", "--hostname", "--config"}, fail_on_unknown_option=True, + ) + if subcommand in {"pr", "issue"}: + data_options = {"--body", "-b", "--title", "-t", "--search", "-S"} + return any( + LOCAL_CDIDX_DLL_RE.search(token) + and not (index > 0 and tokens[index - 1] in data_options) + and not any(token.startswith(option + "=") for option in data_options if option.startswith("--")) + for index, token in enumerate(tokens) + ) + return True def _token_is_expanded_installed_cdidx(token: str, cwd: Path) -> bool: @@ -1315,7 +1327,7 @@ def evaluate_bash_command(command: str, cwd: Path, project_root: Path) -> GuardD if _command_mentions_local_cdidx(command, project_root): return _deny("local cdidx commands must not use shell control operators or command substitutions") - if _command_invokes_cdidx_dll(command): + if _command_has_unsupported_cdidx_dll(command): return _deny("use dotnet ./src/CodeIndex/bin/Debug/net8.0/cdidx.dll instead") if _command_is_safe_cdidx_resolver(command): diff --git a/.agent_harness/guard_policy_contract.json b/.agent_harness/guard_policy_contract.json index b62ddd4126..4a2e45d8e9 100644 --- a/.agent_harness/guard_policy_contract.json +++ b/.agent_harness/guard_policy_contract.json @@ -47,7 +47,7 @@ { "name": "wrapped_review_prompt_local_dll", "command": "env REVIEW_MODE=readonly timeout 30 codex exec 'Use dotnet ./src/CodeIndex/bin/Debug/net8.0/cdidx.dll'", - "expected": "allow" + "expected": "deny" }, { "name": "unsupported_cdidx_dll", @@ -154,6 +154,91 @@ "command": "cdidx status", "expected": "deny" }, + { + "name": "comment_quotes_cannot_hide_dll_substitution", + "command": "true # '\nprintf '%s' \"$(dotnet ./src/CodeIndex/bin/Release/net8.0/cdidx.dll status)\"\n#'", + "expected": "deny" + }, + { + "name": "comment_quotes_cannot_hide_dll_execution", + "command": "echo ignore # 'fake\ndotnet ./src/CodeIndex/bin/Release/net8.0/cdidx.dll status\n#'", + "expected": "deny" + }, + { + "name": "interspersed_redirection_dll_execution", + "command": "dotnet 2>&1 ./src/CodeIndex/bin/Release/net8.0/cdidx.dll status", + "expected": "deny" + }, + { + "name": "leading_redirection_dll_execution", + "command": "2>&1 dotnet ./src/CodeIndex/bin/Release/net8.0/cdidx.dll status", + "expected": "deny" + }, + { + "name": "negated_dll_execution", + "command": "! dotnet ./src/CodeIndex/bin/Release/net8.0/cdidx.dll status", + "expected": "deny" + }, + { + "name": "brace_group_dll_execution", + "command": "{ dotnet ./src/CodeIndex/bin/Release/net8.0/cdidx.dll status; }", + "expected": "deny" + }, + { + "name": "xargs_dll_execution", + "command": "printf 'status\\n' | xargs dotnet ./src/CodeIndex/bin/Release/net8.0/cdidx.dll", + "expected": "deny" + }, + { + "name": "legacy_nice_dll_execution", + "command": "nice -5 dotnet ./src/CodeIndex/bin/Release/net8.0/cdidx.dll status", + "expected": "deny" + }, + { + "name": "builtin_command_dll_execution", + "command": "builtin command dotnet ./src/CodeIndex/bin/Release/net8.0/cdidx.dll status", + "expected": "deny" + }, + { + "name": "variable_host_dll_execution", + "command": "RUNNER=dotnet; \"$RUNNER\" ./src/CodeIndex/bin/Release/net8.0/cdidx.dll status", + "expected": "deny" + }, + { + "name": "line_continuation_dll_host", + "command": "dot\\\nnet ./src/CodeIndex/bin/Release/net8.0/cdidx.dll status", + "expected": "deny" + }, + { + "name": "printed_dll_document_with_literal_separator", + "command": "printf '%s\\n' ';' 'dotnet ./src/CodeIndex/bin/Debug/net8.0/cdidx.dll status'", + "expected": "allow" + }, + { + "name": "printed_dll_document_with_escaped_separator", + "command": "printf '%s\\n' \\; 'dotnet ./src/CodeIndex/bin/Debug/net8.0/cdidx.dll status'", + "expected": "allow" + }, + { + "name": "review_dll_config_is_not_prompt_data", + "command": "codex exec --config 'mcp_servers.example.args=[\"/tmp/cdidx.dll\"]' Review", + "expected": "deny" + }, + { + "name": "review_dll_config_without_prompt", + "command": "codex exec --config 'mcp_servers.example.args=[\"/tmp/cdidx.dll\"]'", + "expected": "deny" + }, + { + "name": "review_prompt_supported_review_subcommand", + "command": "codex review --base origin/main 'Use dotnet ./src/CodeIndex/bin/Debug/net8.0/cdidx.dll'", + "expected": "allow" + }, + { + "name": "document_body_dll_equals_option", + "command": "gh --repo Widthdom/CodeIndex issue create --title Update --body='Use dotnet ./src/CodeIndex/bin/Debug/net8.0/cdidx.dll'", + "expected": "allow" + }, { "name": "repo_local_installer_doctor", "command": "bash ./install.sh --doctor v1.2.3", diff --git a/AGENT_GUIDE.md b/AGENT_GUIDE.md index 06cfaba651..384ed01ac0 100644 --- a/AGENT_GUIDE.md +++ b/AGENT_GUIDE.md @@ -69,7 +69,7 @@ Command-search enforcement is tool-specific and adapter-driven: - Codex uses `.codex/hooks.json`, which invokes `.codex/hooks/bash_guard.py` and `.codex/hooks/permission_request_guard.py`. - Claude Code uses `.claude/settings.json`, which invokes `.claude/hooks/bash-guard.py`. - Both Bash guard adapters delegate shared command policy to `.agent_harness/command_guard_core.py`; update the shared core for common command policy and review both adapters only when tool-specific behavior changes. -- Quoted review prompts and document arguments may mention the repository-built `cdidx.dll` path. The DLL check classifies command positions rather than rejecting every textual mention; unsupported DLL invocations, execution wrappers, and active shell substitutions remain blocked. Other guard checks still apply to the entire command. +- The DLL guard permits `cdidx.dll` text in a final prompt to direct `codex exec` / `codex review` commands with recognized options, in `gh pr` / `gh issue` body/title/search arguments, and in direct `echo` / `printf` / `ls` arguments. Unknown command forms, wrappers, comments, redirections, compound commands and active substitutions remain conservatively blocked when they mention the DLL. Quoted or escaped punctuation stays data; other guard checks still apply to the entire command. Use a direct review command and a body file for more complex document content. - Codex uses the `codeindex_workspace` permission profile for workspace writes plus limited GitHub CLI network access to `github.com` and `api.github.com`. - Codex may use normal development GitHub CLI commands including `gh issue list/view/create/edit/comment`, `gh pr list/view/create/edit/comment/ready/close`, `gh repo view`, and `gh status`. - Keep `gh auth`, `gh api`, `gh secret`, `gh release`, `gh repo create`, `gh repo fork`, `gh repo delete`, and `gh pr merge` blocked. `gh api` is blocked because arbitrary REST/GraphQL calls can bypass subcommand-level policy intent; `gh pr merge` is blocked because it mutates remote PR state in a high-risk way. diff --git a/TESTING_GUIDE.md b/TESTING_GUIDE.md index 885bea61e1..ec502c420a 100644 --- a/TESTING_GUIDE.md +++ b/TESTING_GUIDE.md @@ -5,6 +5,9 @@ changes. The existing policy-contract matrix checks the core, Codex PreToolUse a PermissionRequest, and Claude PreToolUse without executing the tested commands. Keep #5417's quoted review/document data, multiline Markdown and literal substitution controls alongside real DLL invocations, wrappers, shell operators and active substitutions. +Cover comment quotes, leading/interspersed redirections, negation, brace groups, +indirect hosts, line continuations and literal separators. Unknown DLL-bearing command +forms stay denied; a DLL in a Codex configuration option is not prompt data. `JsonEnvelopeWrapperIssue5412Tests` shares an isolated indexed fixture across literal, line-regex and multiline find cursor errors. Keep JSON/envelope/fields/compact @@ -1837,6 +1840,9 @@ Issue #5300 のテストは隣接・入れ子の C# callable、対象行の除 PreToolUse・PermissionRequest、Claude の PreToolUse を検証します。#5417 の引用された レビュー・文書データ、複数行 Markdown、置換構文の文字列としての使用とともに、実際の DLL 実行、ラッパー、シェル演算子、有効なコマンド置換の対照例を維持してください。 +コメント内の引用符、先頭・途中のリダイレクト、否定、波括弧のグループ、間接的な実行元、 +行継続、文字列としての区切り記号も検証します。DLL に言及する未対応のコマンド形式は拒否し、 +Codex の設定オプション内の DLL をプロンプトデータとして扱わないことを確認してください。 `JsonEnvelopeWrapperIssue5412Tests` は分離した索引 fixture を共有し、リテラル・行単位正規表現・ 複数行 find のカーソルエラーを検証します。JSON・envelope・fields・compact の各指定、 diff --git a/changelog.d/unreleased/5417.fixed.md b/changelog.d/unreleased/5417.fixed.md index ebee0da5c0..68036e3f36 100644 --- a/changelog.d/unreleased/5417.fixed.md +++ b/changelog.d/unreleased/5417.fixed.md @@ -11,8 +11,8 @@ affected: ## English -- **Allow local DLL paths in quoted review prompts (#5417)** — the shared Codex/Claude command guard now distinguishes prompt and document arguments from executable positions. Required reviews can mention the repository-built `cdidx.dll` while unsupported DLL execution, execution wrappers and active shell substitutions remain blocked. +- **Allow local DLL paths in quoted review prompts (#5417)** — the shared Codex/Claude command guard now recognizes data arguments in direct review, GitHub document and simple display commands. Required reviews can mention the repository-built `cdidx.dll`; unknown DLL-bearing command forms, execution wrappers and active shell syntax remain conservatively blocked. ## 日本語 -- **引用されたレビュー文中のローカル DLL パスを許可 (#5417)** — Codex／Claude の共有コマンドガードが、プロンプト・文書の引数と実行位置を区別するようになりました。必須レビューでリポジトリ内ビルドの `cdidx.dll` に言及でき、未対応の DLL 実行、実行ラッパー、有効なシェル置換の拒否は維持されます。 +- **引用されたレビュー文中のローカル DLL パスを許可 (#5417)** — Codex／Claude の共有コマンドガードが、直接のレビュー、GitHub 文書、単純な表示コマンドのデータ引数を識別するようになりました。必須レビューでリポジトリ内ビルドの `cdidx.dll` に言及でき、DLL を含む未対応のコマンド形式、実行ラッパー、有効なシェル構文は引き続き保守的に拒否します。 From 76bb8a290a2adc9b744f1422a6281c7f81246d81 Mon Sep 17 00:00:00 2001 From: Widthdom Date: Wed, 23 Sep 2026 06:49:09 +0900 Subject: [PATCH 3/3] Close shell expansion and printf guard gaps (#5417) --- .agent_harness/command_guard_core.py | 14 ++++++-- .agent_harness/guard_policy_contract.json | 40 +++++++++++++++++++++++ AGENT_GUIDE.md | 2 +- TESTING_GUIDE.md | 4 +++ 4 files changed, 57 insertions(+), 3 deletions(-) diff --git a/.agent_harness/command_guard_core.py b/.agent_harness/command_guard_core.py index 394925b672..3d3824b6a9 100644 --- a/.agent_harness/command_guard_core.py +++ b/.agent_harness/command_guard_core.py @@ -379,7 +379,7 @@ def _has_active_shell_syntax(command: str) -> bool: quote = char elif quote == char: quote = None - elif quote != "'" and (char == "`" or command.startswith("$(", index)): + elif quote != "'" and char in "`$": return True elif quote is None and char in "();<>|&\n#": return True @@ -396,8 +396,18 @@ def _command_has_unsupported_cdidx_dll(command: str) -> bool: # and compound commands. Quoted/escaped punctuation stays literal data. if not tokens or _has_active_shell_syntax(command): return True - if tokens[0] in {"echo", "printf", "ls"}: + if tokens[0] in {"echo", "ls"}: return False + if tokens[0] == "printf": + args = tokens[1:] + if args and args[0] == "--": + args = args[1:] + # Shell printf can evaluate variable targets (-v / %n) and numeric + # arguments. Only literal text, %% and %s are known display forms. + return not ( + args and not args[0].startswith("-") + and re.fullmatch(r"(?:[^%]|%%|%s)*", args[0]) is not None + ) if tokens[0] == "codex" and len(tokens) >= 3 and tokens[1] in {"exec", "review"}: prompt, rest = _subcommand_args( tokens[2:], diff --git a/.agent_harness/guard_policy_contract.json b/.agent_harness/guard_policy_contract.json index 4a2e45d8e9..8a82a3aaef 100644 --- a/.agent_harness/guard_policy_contract.json +++ b/.agent_harness/guard_policy_contract.json @@ -239,6 +239,46 @@ "command": "gh --repo Widthdom/CodeIndex issue create --title Update --body='Use dotnet ./src/CodeIndex/bin/Debug/net8.0/cdidx.dll'", "expected": "allow" }, + { + "name": "review_nested_parameter_dll_substitution", + "command": "codex exec \"${review_text:-\"'$(dotnet ./src/CodeIndex/bin/Release/net8.0/cdidx.dll status)'\"}\"", + "expected": "deny" + }, + { + "name": "echo_nested_parameter_dll_substitution", + "command": "echo \"${review_text:-\"'$(dotnet ./src/CodeIndex/bin/Release/net8.0/cdidx.dll status)'\"}\"", + "expected": "deny" + }, + { + "name": "review_literal_parameter_dll_text", + "command": "codex exec '${review_text:-$(dotnet ./src/CodeIndex/bin/Release/net8.0/cdidx.dll status)}'", + "expected": "allow" + }, + { + "name": "printf_variable_target_dll_substitution", + "command": "printf -v 'a[$(dotnet ./src/CodeIndex/bin/Release/net8.0/cdidx.dll status)]' '%s' x", + "expected": "deny" + }, + { + "name": "printf_count_target_dll_substitution", + "command": "printf '%n' 'a[$(dotnet ./src/CodeIndex/bin/Release/net8.0/cdidx.dll status)]'", + "expected": "deny" + }, + { + "name": "printf_numeric_dll_argument", + "command": "printf '%d' 'a[$(dotnet ./src/CodeIndex/bin/Release/net8.0/cdidx.dll status)]'", + "expected": "deny" + }, + { + "name": "printf_string_dll_argument", + "command": "printf -- '%s%%\\n' 'a[$(dotnet ./src/CodeIndex/bin/Release/net8.0/cdidx.dll status)]'", + "expected": "allow" + }, + { + "name": "printf_literal_dll_format", + "command": "printf 'Use dotnet ./src/CodeIndex/bin/Debug/net8.0/cdidx.dll'", + "expected": "allow" + }, { "name": "repo_local_installer_doctor", "command": "bash ./install.sh --doctor v1.2.3", diff --git a/AGENT_GUIDE.md b/AGENT_GUIDE.md index 384ed01ac0..1fc20d290d 100644 --- a/AGENT_GUIDE.md +++ b/AGENT_GUIDE.md @@ -69,7 +69,7 @@ Command-search enforcement is tool-specific and adapter-driven: - Codex uses `.codex/hooks.json`, which invokes `.codex/hooks/bash_guard.py` and `.codex/hooks/permission_request_guard.py`. - Claude Code uses `.claude/settings.json`, which invokes `.claude/hooks/bash-guard.py`. - Both Bash guard adapters delegate shared command policy to `.agent_harness/command_guard_core.py`; update the shared core for common command policy and review both adapters only when tool-specific behavior changes. -- The DLL guard permits `cdidx.dll` text in a final prompt to direct `codex exec` / `codex review` commands with recognized options, in `gh pr` / `gh issue` body/title/search arguments, and in direct `echo` / `printf` / `ls` arguments. Unknown command forms, wrappers, comments, redirections, compound commands and active substitutions remain conservatively blocked when they mention the DLL. Quoted or escaped punctuation stays data; other guard checks still apply to the entire command. Use a direct review command and a body file for more complex document content. +- The DLL guard permits `cdidx.dll` text in a final prompt to direct `codex exec` / `codex review` commands with recognized options, in `gh pr` / `gh issue` body/title/search arguments, and in direct `echo` / `ls` arguments. Direct `printf` is limited to literal text, `%%` and `%s` formats, with optional `--`; variable assignment and other conversions remain blocked. Unknown command forms, wrappers, comments, redirections, compound commands and active substitutions/parameter expansions remain conservatively blocked when they mention the DLL. Quoted or escaped punctuation stays data; other guard checks still apply to the entire command. Use a direct review command and a body file for more complex document content. - Codex uses the `codeindex_workspace` permission profile for workspace writes plus limited GitHub CLI network access to `github.com` and `api.github.com`. - Codex may use normal development GitHub CLI commands including `gh issue list/view/create/edit/comment`, `gh pr list/view/create/edit/comment/ready/close`, `gh repo view`, and `gh status`. - Keep `gh auth`, `gh api`, `gh secret`, `gh release`, `gh repo create`, `gh repo fork`, `gh repo delete`, and `gh pr merge` blocked. `gh api` is blocked because arbitrary REST/GraphQL calls can bypass subcommand-level policy intent; `gh pr merge` is blocked because it mutates remote PR state in a high-risk way. diff --git a/TESTING_GUIDE.md b/TESTING_GUIDE.md index ec502c420a..22e444260f 100644 --- a/TESTING_GUIDE.md +++ b/TESTING_GUIDE.md @@ -8,6 +8,8 @@ controls alongside real DLL invocations, wrappers, shell operators and active su Cover comment quotes, leading/interspersed redirections, negation, brace groups, indirect hosts, line continuations and literal separators. Unknown DLL-bearing command forms stay denied; a DLL in a Codex configuration option is not prompt data. +Keep nested parameter expansions and `printf` variable/count/numeric targets denied, +with single-quoted expansion text and `%s`/`%%` display arguments as allow controls. `JsonEnvelopeWrapperIssue5412Tests` shares an isolated indexed fixture across literal, line-regex and multiline find cursor errors. Keep JSON/envelope/fields/compact @@ -1843,6 +1845,8 @@ PreToolUse・PermissionRequest、Claude の PreToolUse を検証します。#541 コメント内の引用符、先頭・途中のリダイレクト、否定、波括弧のグループ、間接的な実行元、 行継続、文字列としての区切り記号も検証します。DLL に言及する未対応のコマンド形式は拒否し、 Codex の設定オプション内の DLL をプロンプトデータとして扱わないことを確認してください。 +入れ子のパラメーター展開と `printf` の変数代入・文字数代入・数値変換は拒否し、単一引用符内の +展開構文と `%s`・`%%` による文字列表示を許可する対照例も維持してください。 `JsonEnvelopeWrapperIssue5412Tests` は分離した索引 fixture を共有し、リテラル・行単位正規表現・ 複数行 find のカーソルエラーを検証します。JSON・envelope・fields・compact の各指定、