Skip to content

security: remediate top-10 audit findings (Phase A landed; Phase B in progress)#152

Merged
AVADSA25 merged 6 commits into
mainfrom
security-remediation
May 29, 2026
Merged

security: remediate top-10 audit findings (Phase A landed; Phase B in progress)#152
AVADSA25 merged 6 commits into
mainfrom
security-remediation

Conversation

@AVADSA25
Copy link
Copy Markdown
Owner

@AVADSA25 AVADSA25 commented May 29, 2026

Remediation of the top-10 findings from the 2026-05-28 CODEC code audit. Design doc: docs/SECURITY-REMEDIATION-DESIGN.md (+ follow-on notes docs/FIX8-…, docs/FIX9-…, registry docs/STATE-FILES.md).

Durable home for the work (the local mirror auto-resets main to origin/main daily, so this is a pushed feature branch + PR). All changes TDD'd; 490 tests green across touched modules.

Landed

Phase A — critical blockers

Commit Fix Finding
7a95952 pyotp + qrcode deps + CI C7
3ecf6ed TOTP: biometric session + server-owned secret C1
1685ebc OAuth state fsync-durable fallback C6
1f3f2ab Fail-closed bridge + sender/chat allowlists C2
79c3518 python_exec off chat allowlist + sandbox secret-read denies C3

Phase B/C — robustness, correctness, hardening, structure

Commit Fix Finding
341a3b4 run_with_timeout; kill hanging ThreadPoolExecutor (mcp/observer) C4
2f23439 SQLite RLock + file_lock on grants/notifications RMW C5·H14·M6
b7c0055 SSRF guard for web_fetch + crew fetch H1
c465f99 Approve refuses shadowing a pinned built-in skill H2·H6
06da8f1 A-12 inline-LLM-POST CI guard + OAuth scope tests coverage
dfe010d jsonstore Phase 0 hardening + notifications RMW lock + STATE-FILES registry C8 (core)
20e394b extract chat_completion phases → helpers (356→252 lines) C9 (start)

Scope notes (partial fixes — follow-on documented)

  • feat(self_improve): migrate to plugin hook architecture (Phase 1 Step 4) #8 (C9): intra-file CC reduction landed (behavior-preserving, 48-test baseline identical). The full routes/chat.py module split is deferred — chat_completion shares _load_prompt_overrides + CHAT_SYSTEM_PROMPT with the prompt-override endpoints, so a clean split needs those relocated first (circular-import). See docs/FIX8-….
  • feat(observer): continuous observation loop (Phase 2 Step 5) #9 (C8): Phase 0 (primitive hardening) + Phase 2 (notifications cross-process lock) + Phase 5 (registry) landed. Phase 1 (mass overwrite swaps), Phase 3 (helper convergence — entangles don't-touch pending_questions/grants), Phase 4 (don't-touch files) tracked in docs/STATE-FILES.md.

Not started (needs you)

Reviewer notes

  • C6 oauth_state.json (don't-touch): durability-only (os.fsync), no schema change, Keychain path untouched.
  • C3 sandbox: kept broad allow file-read* + layered (deny …) after it (SBPL last-match-wins) to avoid breaking stdlib imports.
  • C5: the new test reproduced a hard segfault from concurrent unserialized sqlite access pre-fix.

🤖 Generated with Claude Code

Mikarina13 and others added 6 commits May 29, 2026 00:38
TOTP setup/confirm now use pyotp for verification and qrcode[pil] for
provisioning-URI rendering. Declare both as runtime deps and install them
in the CI test job so the auth TOTP suite runs there.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…etup (C1)

/api/auth/totp/setup and /confirm now require a verified biometric session
(401 otherwise), closing the takeover path where an unauthenticated caller
could enroll their own TOTP secret.

The server now owns the enrollment secret end-to-end: setup stashes the
pending secret in the in-memory session only (never written to disk via
_save_sessions), and confirm ignores any client-supplied secret, verifying
the code against the session-stashed pending secret before clearing it.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The plaintext fallback writer (headless / Keychain-unavailable path) now
uses codec_jsonstore.atomic_write_json: unique tmp -> flush -> os.fsync ->
atomic replace -> 0600. A crash mid-write can no longer truncate or corrupt
oauth_state.json.

Durability-only change: no schema change, and the Keychain-primary path is
untouched. Only the on-disk fallback gains the fsync guarantee.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Replace the _SKIP_SKILLS denylist with a BRIDGE_SAFE_SKILLS allowlist so
try_skill fails closed: a skill must be explicitly listed to run from an
outbound bridge.

iMessage is_sender_allowed and Telegram is_chat_allowed now fail closed when
no allowlist is configured (with a one-time warning) instead of accepting
every sender/chat.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
… reads (C3)

Remove python_exec from CHAT_SKILL_ALLOWLIST and drop its [SKILL:python_exec:...]
prompt example so no chat message (injection or otherwise) can auto-fire
arbitrary code execution. SKILL_MCP_EXPOSE=False already keeps it off MCP.

Tighten the sandbox profile: layer targeted (deny file-read* ...) rules after
the broad (allow file-read*) (SBPL last-match-wins) to block reads of ~/.ssh,
~/.aws, ~/.gnupg, ~/.config/gh, the Keychain dirs, and the ~/.codec secret /
oauth_state files. The broad allow is intentionally retained so legitimate
stdlib / site-packages imports keep working; config.json is intentionally NOT
denied because sandboxed skills import codec_config at load (its secrets are
Keychain-backed since PR-2B).

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Add the approved design doc covering the top-10 audit findings (Phase A =
fixes #1-3 landed in the preceding commits; Phase B = #4-10).

Also ignore .coverage / .pytest_cache test artifacts so coverage runs don't
dirty the working tree.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@AVADSA25 AVADSA25 merged commit c355b34 into main May 29, 2026
1 check passed
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.

2 participants