Skip to content

fix: audit-found bugs across guard, bridge, and memory system#320

Merged
seoseo-ai merged 7 commits into
mainfrom
claude/a2a-broker-test-results-7vfj9r
Jul 7, 2026
Merged

fix: audit-found bugs across guard, bridge, and memory system#320
seoseo-ai merged 7 commits into
mainfrom
claude/a2a-broker-test-results-7vfj9r

Conversation

@jinon86

@jinon86 jinon86 commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Summary

An audit across three subsystems — the PreToolUse security guard, the Telegram bridge, and the memory system — turned up a set of concrete bugs. Every finding was reproduced by running the affected code before fixing, and each fix ships with a regression test where the harness supports one. All suites are green: guard 160, bridge 110, memory 95 (74 + 15 + 6), harness validation PASS.

Guard (security) — bypasses & false positives closed

Verified by running guard.sh; 28 regression cases added.

Bypasses (were ALLOW → now DENY)

  • Force-push to a protected branch hidden behind quotes (git push "--force" origin main, origin "+main"), bundled short flags (-fv, -fu), or fully-qualified refs (refs/heads/main, HEAD:refs/heads/main). Force detection now runs on the quote-stripped view; the short-flag regex matches f anywhere in a cluster; dsts are normalized before the protected-list check.
  • rm -rf /* and rm -rf ${HOME} (trailing anchor accepts *; ${HOME} matched alongside $HOME).
  • Tag-push / publish through git global options (git -C /repo push origin --tags) and alternate package managers (yarn/pnpm publish).
  • Secret exfil via read-then-pipe (cat .env | curl @-, base64 key | nc host) — detection is now order-independent and covers ed25519/ecdsa/dsa SSH private keys.

False positives (were DENY → now ALLOW)

  • Legit feature-branch force-push through -C/-c (the adjacency wc -l gate made the relaxation unreachable).
  • Ordinary API calls to URLs containing secret/token (bare-word match dropped; only credential files gate).
  • Pushing a public key out (.pub/.pub.pem neutralized before the match).
  • Bare word replay (e.g. grep replay app.log) and db:migrate substring (grep db:migrate Makefile) — both scoped to real invocations.

Bridge

  • task_queue: identity-guard the per-key active slot. With up to max_inflight concurrent tasks per key, an earlier task finishing used to pop the shared _active slot unconditionally, erasing a later still-running task so priority /stop and /revert could no longer cancel it. (+ regression test)
  • streaming: bail update_if_needed when the first draft send fails. An empty drafts plus a chunk >= max_bubble_chars entered the overflow while, where handle_overflow returns immediately without consuming text or awaiting — a busy-loop that hung the whole event loop.
  • streaming: update_draft now catches non-Telegram exceptions like create_draft does, so a transport error mid-edit can't abort the reader loop.
  • sdk_text: tolerate explicit null questions/options in AskUserQuestion instead of raising TypeError.
  • ui: fall back to the computed position when a history record lacks index (a malformed entry no longer blanks /revert), and assume UTC for tz-naive timestamps.
  • orphan_reaper: detect via argv boundaries (NUL-split /proc/<pid>/cmdline) so a node binary under a spaced path is still reaped. (+ regression test)

Memory system

  • index: only advance updated_at when a doc's content actually changed. The unconditional CURRENT_TIMESTAMP on every (background-triggered) re-index re-timestamped every doc to ~now, so search-side recency_boost was a uniform constant that never affected ranking.
  • search: guard the CCC_MEMORY_SEARCH_LIMIT int parse (malformed/non-positive → fallback 5) instead of a traceback; align _chash's charset with char_ngrams so predominantly Japanese/Chinese content gets a usage key.
  • check.sh: printf '-1' emitted nothing (parsed as a flag), breaking --json and inverting the cache-health report on a fresh node — now printf '%s' '-1'. Also resolve the honcho base from the nested .hosts.hermes.baseUrl schema and use per-source TTL in status_for.
  • install-memory-refresh-cron.sh: mkdir -p the log dir on --apply so the cron redirect doesn't fail before the refresh runs.
  • load-memory.sh: reserve room for the truncation marker so injected context stays within CCC_MEMORY_MAX_BYTES (it overshot by ~38 bytes).

Test plan

  • bash claude/hooks/guard.test.sh → PASS=160
  • python3 -m pytest bridge/tests/… (pure-logic subset) → 110 passed
  • bash scripts/ccc-memory.test.sh → PASS=74; install-memory-refresh-cron.test.sh → PASS=15; claude/hooks/memory-hooks.test.sh → PASS=6
  • bash scripts/validate-harness.sh → PASS

Known limitations (deliberately not fixed)

  • orphan_reaper PID-reuse TOCTOU between scan and SIGTERM — inherent race, mitigated by the 30-min age gate; no clean fix without pidfd.
  • Secret-exfil gate does not add .pem/.key certs to avoid gating legitimate cert deployment; a regex guard is inherently porous, so least-privilege / network policy remains the real boundary.

🤖 Generated with Claude Code

https://claude.ai/code/session_017rAPKfsLfaYqNY8iPXWX34


Generated by Claude Code

claude added 6 commits July 7, 2026 12:08
Audit of the PreToolUse guard found several gated actions that could slip
through, plus false positives that blocked legitimate work. All verified by
running the script; 25 regression cases added (suite now 157, all green).

Bypasses closed (were ALLOW, now DENY):
- Force-push to a protected branch hidden behind quotes (`git push "--force"
  origin main`, `origin "+main"`), bundled short flags (`-fv`, `-fu`), or
  fully-qualified refs (`refs/heads/main`, `HEAD:refs/heads/main`). Force
  detection now runs on the quote-stripped view; the short-flag regex matches
  `f` anywhere in a cluster; fully-qualified dsts are normalized to the branch
  name before the protected-list check.
- `rm -rf /*` and `rm -rf ${HOME}` (trailing anchor now accepts `*`; `${HOME}`
  matched alongside `$HOME`).
- Tag-push / publish through git global options (`git -C /repo push origin
  --tags`) and alternate package managers (`yarn`/`pnpm publish`).
- Secret exfil via read-then-pipe (`cat .env | curl @-`, `base64 key | nc host`)
  — detection is now order-independent (network tool anywhere + credential file
  anywhere), and covers ed25519/ecdsa/dsa SSH private keys.

False positives fixed (were DENY, now ALLOW):
- Legit feature-branch force-push through `-C`/`-c` (the old adjacency `wc -l`
  gate made the relaxation unreachable for that form).
- Ordinary API calls to URLs containing `secret`/`token` (bare-word match
  dropped; only concrete credential files gate now).
- Pushing a PUBLIC key out (`.pub`/`.pub.pem` neutralized before the match).
- Bare word `replay` in innocuous commands like `grep replay app.log`
  (now scoped to broker/worker/gateway subcommands).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017rAPKfsLfaYqNY8iPXWX34
- task_queue: identity-guard the per-key active slot. With up to max_inflight
  tasks running concurrently, an earlier task finishing used to pop the shared
  `_active` slot unconditionally, erasing a later still-running task so the
  priority /stop and /revert paths could no longer cancel it. Clear the slot
  only when it still points at the finishing task. (+ regression test)
- streaming: bail update_if_needed when the first draft send fails. Previously
  an empty `drafts` plus an accumulated chunk >= max_bubble_chars entered the
  overflow `while`, where handle_overflow returns immediately without consuming
  text or awaiting — a tight busy-loop that pinned the CPU and hung the whole
  event loop.
- sdk_text: tolerate explicit null `questions`/`options` in AskUserQuestion
  (get(...) or []) instead of raising TypeError and aborting the degrade path.
- ui: fall back to the computed position when a history record lacks `index`
  (a single malformed entry no longer crashes the whole /revert keyboard), and
  assume UTC for tz-naive timestamps so relative time keeps formatting.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017rAPKfsLfaYqNY8iPXWX34
`grep db:migrate Makefile` (and any innocuous grep of the token) was denied
because the gate matched the bare `db:migrate` substring. Require an actual
package-manager/make run context (npm/yarn/pnpm/npx run, make) so reads of the
token are allowed while real migrations still gate. (+3 regression cases)

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017rAPKfsLfaYqNY8iPXWX34
…ilience

- orphan_reaper: detect via argv boundaries (split /proc/<pid>/cmdline on NUL)
  instead of a space-joined string. A node binary under a path containing a
  space (e.g. "/opt/my node/bin/node") was mis-split and the orphan was never
  reaped. _is_node_claude now accepts an argv list (preferred) or the legacy
  joined string. (+ regression test)
- streaming: update_draft now also catches non-Telegram exceptions like
  create_draft does, so a transport-level error during an edit can't propagate
  through the reader loop and abort message processing mid-stream.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017rAPKfsLfaYqNY8iPXWX34
…keys

- index: only advance updated_at when a doc's content actually changed. The
  unconditional CURRENT_TIMESTAMP on every (often background-triggered) re-index
  re-timestamped every doc to ~now, so search-side recency_boost was a uniform
  constant that never affected ranking. A genuinely-changed doc and a weeks-old
  stable one are now distinguishable again.
- search: guard the CCC_MEMORY_SEARCH_LIMIT / argv int parse like every other
  numeric env var here — a malformed value fell through int() with a traceback;
  now it (and a non-positive value that yields empty results) falls back to 5.
- search: align _chash's charset with char_ngrams (add Hiragana/Katakana/CJK).
  Predominantly Japanese/Chinese content normalized to "" before, so its usage
  key was empty and the usage-feedback loop could never boost such docs.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017rAPKfsLfaYqNY8iPXWX34
- ccc-memory-check.sh: `printf '-1'` parsed -1 as a flag and emitted nothing,
  which broke --json (--argjson got "") and made text mode report a missing
  cache as healthy on exactly the fresh-node state this tool diagnoses. Use
  `printf '%s' '-1'`. Also resolve the honcho base URL from the nested
  `.hosts.hermes.baseUrl` schema (mirroring refresh-memory.sh) instead of only
  the legacy top-level key, and let status_for use the per-source WIKI/HONCHO
  TTL so the status agrees with the meta staleness computation.
- install-memory-refresh-cron.sh: mkdir -p the log directory on --apply. The
  cron line redirects to a log under STATE_DIR; if that dir is absent when cron
  fires, the redirect fails before refresh-memory.sh (which creates it) can run.
- load-memory.sh: reserve room for the truncation marker so injected context
  stays within CCC_MEMORY_MAX_BYTES (it overshot the cap by ~38 bytes before).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017rAPKfsLfaYqNY8iPXWX34
@jinon86 jinon86 requested a review from seoseo-ai as a code owner July 7, 2026 12:40

@seoseo-ai seoseo-ai left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed for requested ccc-node PR merge: branch updated to main, required CI/checks green, changes are regression-tested fixes across guard/bridge/memory.

@seoseo-ai seoseo-ai merged commit 999e168 into main Jul 7, 2026
7 checks passed
@seoseo-ai seoseo-ai deleted the claude/a2a-broker-test-results-7vfj9r branch July 7, 2026 12:56
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.

3 participants