Skip to content

Commit 722bf04

Browse files
author
bgagent
committed
fix(agent): prevent Cedar NoDecision on bash commands with quotes
Cedar entity UIDs use Type::"id" format — when the resource ID contained double quotes (e.g. git commit -m "fix bug"), the parser failed and returned NoDecision, which fail-closed denied the call. This blocked virtually all real git/gh commands in production. Fix: use fixed sentinel resource IDs ("command" for execute_bash, "file" for write_file) instead of embedding raw command/path text in the Cedar entity UID. The deny-list policies only match on context.command and context.file_path, never on the resource ID, so behavior is identical.
1 parent a1e9fcb commit 722bf04

2 files changed

Lines changed: 69 additions & 4 deletions

File tree

agent/src/policy.py

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,12 @@ def _evaluate(
131131
"""Run a single Cedar authorization check.
132132
133133
Returns (allowed, reason). Fails closed on NoDecision.
134+
135+
``resource_id`` must be a simple identifier safe for Cedar entity
136+
UID parsing (no quotes, newlines, or special chars). Callers that
137+
evaluate user-supplied values (bash commands, file paths) should
138+
pass a fixed sentinel and put the real value in ``context`` where
139+
the policies match against it.
134140
"""
135141
cedarpy = self._cedarpy
136142
if cedarpy is None:
@@ -194,31 +200,38 @@ def evaluate_tool_use(self, tool_name: str, tool_input: dict) -> PolicyDecision:
194200
)
195201
return PolicyDecision(allowed=False, reason=reason, duration_ms=elapsed)
196202

197-
# Additional checks for Write/Edit: check file path
203+
# Additional checks for Write/Edit: check file path.
204+
# Use a fixed sentinel resource_id ("file") because file paths
205+
# may contain quotes/special chars that break Cedar UID parsing.
206+
# The actual path is in context.file_path where policies match it.
198207
if tool_name in ("Write", "Edit"):
199208
file_path = tool_input.get("file_path", "")
200209
if file_path:
201210
file_context = {**base_context, "file_path": file_path}
202211
allowed, deny_reason = self._evaluate(
203212
"write_file",
204213
"Agent::File",
205-
file_path,
214+
"file",
206215
file_context,
207216
)
208217
if not allowed:
209218
elapsed = (time.monotonic() - start) * 1000
210219
reason = deny_reason or f"Cedar policy denied write to {file_path}"
211220
return PolicyDecision(allowed=False, reason=reason, duration_ms=elapsed)
212221

213-
# Additional checks for Bash: check command
222+
# Additional checks for Bash: check command.
223+
# Use a fixed sentinel resource_id ("command") because bash
224+
# commands contain quotes/special chars that break Cedar UID
225+
# parsing. The actual command is in context.command where
226+
# policies match it.
214227
if tool_name == "Bash":
215228
command = tool_input.get("command", "")
216229
if command:
217230
bash_context = {**base_context, "command": command}
218231
allowed, deny_reason = self._evaluate(
219232
"execute_bash",
220233
"Agent::BashCommand",
221-
command[:200],
234+
"command",
222235
bash_context,
223236
)
224237
if not allowed:

agent/tests/test_policy.py

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,58 @@ def test_allows_mise_run_build(self):
134134
assert result.allowed is True
135135

136136

137+
class TestBashCommandsWithQuotes:
138+
"""Commands containing double quotes must not cause NoDecision."""
139+
140+
def test_allows_git_commit_with_message(self):
141+
engine = PolicyEngine(task_type="new_task", repo="owner/repo")
142+
result = engine.evaluate_tool_use("Bash", {"command": 'git commit -m "fix: login bug"'})
143+
assert result.allowed is True
144+
145+
def test_allows_git_commit_tree(self):
146+
engine = PolicyEngine(task_type="new_task", repo="owner/repo")
147+
cmd = 'git commit-tree HEAD^{tree} -m "squash"'
148+
result = engine.evaluate_tool_use("Bash", {"command": cmd})
149+
assert result.allowed is True
150+
151+
def test_allows_gh_pr_create(self):
152+
engine = PolicyEngine(task_type="new_task", repo="owner/repo")
153+
cmd = 'gh pr create --title "my PR" --body "desc"'
154+
result = engine.evaluate_tool_use("Bash", {"command": cmd})
155+
assert result.allowed is True
156+
157+
def test_allows_gh_api_post(self):
158+
engine = PolicyEngine(task_type="new_task", repo="owner/repo")
159+
cmd = 'gh api --method POST /repos/o/r/pulls -f title="PR"'
160+
result = engine.evaluate_tool_use("Bash", {"command": cmd})
161+
assert result.allowed is True
162+
163+
def test_allows_heredoc_commit(self):
164+
engine = PolicyEngine(task_type="new_task", repo="owner/repo")
165+
cmd = "git commit -m \"$(cat <<'EOF'\nFix the bug\nEOF\n)\""
166+
result = engine.evaluate_tool_use("Bash", {"command": cmd})
167+
assert result.allowed is True
168+
169+
def test_denies_force_push_with_quotes(self):
170+
engine = PolicyEngine(task_type="new_task", repo="owner/repo")
171+
result = engine.evaluate_tool_use("Bash", {"command": 'git push --force "origin" main'})
172+
assert result.allowed is False
173+
174+
175+
class TestFilePathsWithSpecialChars:
176+
"""File paths with special characters must not cause NoDecision."""
177+
178+
def test_allows_path_with_quotes(self):
179+
engine = PolicyEngine(task_type="new_task", repo="owner/repo")
180+
result = engine.evaluate_tool_use("Write", {"file_path": '/workspace/it"s-a-file.ts'})
181+
assert result.allowed is True
182+
183+
def test_denies_protected_path_with_quotes(self):
184+
engine = PolicyEngine(task_type="new_task", repo="owner/repo")
185+
result = engine.evaluate_tool_use("Write", {"file_path": '.github/workflows/ci"test.yml'})
186+
assert result.allowed is False
187+
188+
137189
class TestExtraPolicies:
138190
def test_extra_forbid_applied(self):
139191
extra = [

0 commit comments

Comments
 (0)