Skip to content

fix(permissions): prevent a corrupt rules file from running permanently blocked commands - #72

Open
EddieMurphy89 wants to merge 1 commit into
ApodexAI:mainfrom
EddieMurphy89:fix/permissions-fail-closed
Open

EddieMurphy89 wants to merge 1 commit into
ApodexAI:mainfrom
EddieMurphy89:fix/permissions-fail-closed

Conversation

@EddieMurphy89

@EddieMurphy89 EddieMurphy89 commented Nov 9, 2026 •

Copy link
Copy Markdown

Summary

In a --yes or auto_for_me session, apodex relies on the persisted deny list in ~/.config/apodex/permissions.json as the last control that stops a command the user permanently blocked. If that file exists but cannot be read or parsed, PermissionStore.load() currently catches the error and returns an empty store, so a saved deny rule can silently disappear and the previously blocked command can run unattended.

This PR makes the read fail closed and the write atomic:

  • FileNotFoundError (first run) still yields an empty policy.
  • A file that exists but is unreadable, unparseable, or has the wrong shape records load_error; denies() returns True and the tool gate returns RISK_DENY with a clear reason instead of falling back to an empty policy.
  • save() writes to a temporary file, fsyncs, and os.replaces it, so an interrupted write leaves the previous file intact instead of truncating it.

Impact

The lost control is the user's entire persisted permission policy, not one command. The deny list accepts command prefixes and bare tool names (bash, write_file, file_editor, run_python_code, web_fetch/download_file), so a single unreadable or truncated permissions.json can remove every saved "never" at once. In a --yes or stock auto_for_me session — the CLI's auto-approve mode for Docker/trusted environments — assess_with_rules() then returns RISK_SAFE for any call no remaining rule rejects, and the tool runs with the user's own privileges and no prompt.

A deployment that implements a read-only or offline posture by denying those tools loses that posture in one step: files become writable, network tools usable, and code execution available. Because the rules are re-read at every session start, the widened surface persists for every later session until someone rewrites the file by hand; nothing in the UI or logs reports the loss, and the rules do not come back on their own. Restored curl/ssh/git push-class commands let an agent steered by repository content or a fetched page send what it can read (source, .env, tokens) to an external host, and an unbanned write_file reaches build scripts and CI configuration.

The same except Exception loader shape also covers settings.json's plan_mode, so a corrupted settings file can silently drop the "no edits until I approve a plan" gate and let a mutating tool such as write_file execute. This PR keeps the settings change as a follow-up, but the impact is the same class of lost security state.

Affected versions

Tested on main at cc32d880ff (2026-11-03), pyproject.toml version 0.1.0. The repository has no release tag; the vulnerable code is still present on the current main as of this writing.

Reproduction

The decision layer can be checked without a model or a full session:

import json, tempfile
from pathlib import Path
from apodex.permissions import PermissionStore
from apodex.agent_tools import assess_with_rules

d = Path(tempfile.mkdtemp())
p = d / "permissions.json"

# Control: a valid file with a permanent deny.
p.write_text(json.dumps({"allow": [], "deny": ["Bash(git push)"]}), encoding="utf-8")
print(PermissionStore.load(str(p)).denies("bash", {"command": "git push"}))  # True

# Fault: the file exists but is truncated / invalid.
p.write_text('{"allow": [], "deny": ["Bash(git push)"]', encoding="utf-8")
store = PermissionStore.load(str(p))
print(store.denies("bash", {"command": "git push"}))
print(assess_with_rules("write_file", {"path": str(d / "x")}, str(d), store).level)

Before this PR: the fault prints False and confirm; under --yes the confirm is auto-approved. After this PR: it prints True and deny, with the reason "saved permission rules could not be read; refusing to run with an unknown policy".

A full CLI-level reproduction is the same sequence with the real binary: write a deny rule, truncate ~/.config/apodex/permissions.json, start apodex --yes, and request the denied command. The control run blocks; the fault run executes before the patch and blocks after it.

Fix

  • apodex/permissions.py:304-319 — load() distinguishes missing from unreadable/invalid; load_error is recorded for the latter; denies() fails closed and allows() returns False in that state. save() now uses a temporary file + fsync + os.replace.
  • apodex/agent_tools.py:440 — assess_with_rules() checks load_error before the normal rule lookup and returns RISK_DENY.
  • apodex/tests/test_permissions.py — new regression tests for missing/corrupt/wrong-shape/unreadable files, the gate decision, and the atomic-save failure path.

The same loader shape also exists in UserSettings.load() for settings.json (plan_mode); this PR deliberately keeps the change focused on the permission store, and I can split the settings change into a follow-up if you prefer.

Testing

  • uv run pytest apodex/tests/test_permissions.py -q — 9 passed
  • The new tests fail on unpatched main and pass with the patch.
  • I could not run the full repository suite in my environment; CI will cover it. I am happy to adjust if any existing test expects the old empty-store behaviour.

This branch has not been deployed

No deployments
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