Skip to content

fix: 69 validated findings from the second 2026-10-12 bug hunt - #318

Merged
jkyberneees merged 111 commits into
mainfrom
fix/bughunt-2026-10-12b
Oct 12, 2026
Merged

jkyberneees merged 111 commits into
mainfrom
fix/bughunt-2026-10-12b

Conversation

@jkyberneees

Copy link
Copy Markdown
Contributor

Summary

Fixes all 69 findings from the second 2026-10-12 functional + security bug hunt over v2.34.3 (3df53de1) — 11 high, 37 medium, 21 low — plus 8 extra issues the fixers confirmed while verifying unproven hunter concerns (Telegram /plan_view rune cut, /plan_resume lock, budget fail-open; config project-limit clamps and skill promote symlink; episode index ordering; MCP LoadProjectFile O_NOFOLLOW and HTTP error-body sanitising) and every issue from the per-area reviews, one full-branch Opus review (2 high, 6 medium, 11 low) and the Sonnet re-reviews of those fixes.

Same workflow as #317: Opus hunters per area proved each finding with a failing TestRED_* test; Sonnet fixers re-validated on base, then RED → GREEN → docs (one commit per finding where hunks didn't interleave); Sonnet adversarial review per area with fix rounds; branches cherry-picked together; full-branch Opus review; Sonnet re-review of the final fixes.

High-severity highlights

  • danger — $(…) inside single quotes in $(( )), $[ ], ${s:off} and subscripts auto-allowed (bash evaluates it); glob spellings of secrets (~/.odek/secrets.en?, /proc/self/env*, ~/.bash*); HOME=. bash -i running unread startup files; composed xargs/parallel expansion now budgeted and failing closed (was unbounded: 13 s / 7.6 GB on a 30 KB command).
  • tools — http_request sent credential headers in cleartext after a two-hop https→http redirect (now compared against the original request).
  • config — a repo-planted .odek-artifacts symlink made sub-agent staging cleanup RemoveAll sibling projects and relocate ~/.ssh keys (staging now via os.Root); --tool/--no-tool, sandbox knobs and project-sandbox approval now propagate to sub-agents through the task file; untrusted AGENTS.md could leave the security pillar inside an open code fence (CommonMark fence tracker incl. blockquotes and list items).
  • memory — credential-named tool arguments/headers persisted unredacted (key-aware JSON redaction).
  • ops — bare-key redaction leaked values with password characters; redaction redesigned around one credential-value check (code expressions stay visible, so read_file/patch output isn't corrupted; linear on hostile input).

Medium/low fixes span untrusted-tag look-alike neutralising (padding, modifier separators, enclosed/Armenian/Cherokee letters, look-alike brackets, spaced forms — linear), argv secret leaks in events, batch-trust for no-risk tools, MCP schema pattern/format/$ref/enum prose (structural rule), MCP tool names > 64 chars (shortened, __ kept), tools/list pagination, HTTP MCP Close, ffmpeg protocol/format whitelists, speak symlink writes, first-turn audit on serve, delete-vs-audit race, late approval/clarify answers, WS lookup rate limits, Telegram MarkdownV2 chunking/fallback/**bold**, media credential files, upload cap, scheduler delivery, budget counter atomicity, compaction digest redaction, ~/.odek reads tainting episodes, pkill/kill broadcast forms, gpg verbs, sqlite readfile(), recursive readers over $HOME//proc, denylist over extracted code bodies, history-file tables shared by ClassifyPath and the @-reference gate.

Per-finding detail: .claude/bughunt-2026-10-12b/ (gitignored) — fix-input-*.json, fix-reports/, review-reports/.

Coverage (before → after)

package v2.34.3 after
cmd/odek 76.6% 77.1%
internal/danger 92.7% 92.8%
internal/redact 96.8% 97.1%
internal/agent 82.3% 83.5%
internal/config 86.9% 87.4%
internal/session 90.4% 90.5%
internal/memory 85.1% 85.2%
internal/loop 93.6% 93.7%
internal/telegram 91.1% 91.1%
internal/mcpclient 96.2% 95.7% (new defensive branches in pagination/Close paths)

Test plan

  • go build ./..., go vet ./..., gofmt -l, go mod tidy -diff clean
  • go test ./...
  • CI mode: go test ./... -short -race with a temp HOME (only pre-existing macOS-temp-HOME failures in internal/telegram media-path tests, identical on main)
  • HOME=/home/user go test -race ./internal/danger/
  • Fuzz 30 s each: FuzzSeparatorThenWipe, FuzzPipeIntoShell, FuzzHarmlessPrefixKeepsRank, FuzzAnalyzeBounded (-parallel 1)
  • Redaction corpus check: 11.2 MB of this repo's Go/Markdown — no non-test source line gains redaction; ~87 former over-redactions removed
  • CI: golangci-lint, govulncheck

Known residuals (documented, not fixed)

  • Redaction: unquoted passwords starting with {, &, * or mixed-case letters-only passphrases before ,/) read as code; values judged on their first 256 bytes; hostile 1 MiB inputs up to ~1–2 s (linear).
  • Classifier: v='a[$(cmd)]'; let "$v"; git $x dynamic verb; sqlite .output; dotglob; rg -f file / -g GLOB parsing (fail-safe); hostile 64 KiB inputs ~2.5× slower than main (bounded); a catch-all pgrep and kill anywhere on one line is flagged.
  • MCP property/required/$defs names can still spell a few joined words.
  • A once ("y") Dockerfile.odek approval persists for a long-lived serve/Telegram process (forwarded to children only for the same dir + content hash).
  • Telegram daily budget counter is keyed by UTC date: one-time reset for non-UTC users on upgrade.
  • Memory is a global operator store (not per Telegram chat) by design.

🤖 Generated with Claude Code

jkyberneees and others added 30 commits October 12, 2026 15:35
…et forged tags

Skip budget failed open past 64 invisible runes, modifier letters (U+02CD) counted as
letters, and look-alike openers/slashes (U+02C2, U+FE64, U+2215, U+2044...) and small
capitals were not recognised. Padding is now unbounded (still linear), non-alphanumeric
runes are separators, and the openers/slashes/small capitals/dashes are folded.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
NAME=(a b) leaked its second element and NAME+=v was reported as the program. Array values
are consumed whole and += counts as an assignment.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…on rune boundary

plan/clarify/bg_* are safe by contract but counted as unclassifiable, forcing a second
prompt per listed command. The dropped-message sketch also cut mid-rune.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
NAME[expr]=v and NAME[expr]+=v were reported as the program name, leaking the value; an
unclosed subscript now yields no argv0.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…look-alikes; keep spaced prose

Squared/regional letters, long s, Armenian/Cherokee confusables and more bracket look-alikes
now match. A space between the opener and the tag name means prose, not a tag.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The downgrade strip compared against the previous hop, but net/http re-copies original headers each hop. Compare every hop with the original request.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Pass a cleaned absolute path and -protocol_whitelist file,pipe so URL-shaped repo file names and nested demuxer URLs are not fetched.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…not followed

os.WriteFile followed a symlink in the artifacts root; use fsatomic temp+rename.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Body-read errors can carry server-controlled text; wrap like the fetch error.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
credentialArgFlag now also matches whole-component auth, pass, pat, cookie, creds, dsn, bearer, session, apikey flags.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…tainers

Drop pipe from the protocol whitelist and add a format whitelist so a disguised HLS/concat playlist cannot read other local files. Also adds probe tests for the two follow-up fixes.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…edential word

--no-auth, --pass-through, --auth-mode and similar take no secret; a credential flag followed by another flag masks nothing.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Lstat the artifacts directory before writing so a symlinked directory is not followed.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…hema

Prose joined with _ . - bypassed the pattern word heuristics, and format/$ref
accepted any 128-rune token. pattern now needs to be short with bounded literal
runs, format is a standard-name allowlist and $ref must be #/$defs|definitions/<name>;
everything else is lifted into the wrapped docs shown at approval.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A denial reason can be the full denied command line; events carried it raw,
unlike tool arguments. Emit reason_sha256 and reason_bytes instead.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The prompt cut descriptions at 2 KiB while the model gets up to 8192 runes, so
appended text was invisible at approval. Both now share one cap helper.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…TP Close aborts calls

tools/list now follows nextCursor (64 pages / 2048 tools); registered names over
the provider's 64-char limit get a deterministic truncated+hash form shown at
approval; closing an HTTP client cancels in-flight calls and rejects later ones;
non-200 error bodies are flattened and capped.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The Lstat symlink check left a swap window before the open; refuse symlinks at open time.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…n repeated cursor

Shortened names cut the server part past the separator, so the loop's MCP
classification and episode taint (keyed on __) missed them. Each part is now
truncated separately. tools/list also stops when a nextCursor repeats.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Patterns with any whitespace or two long letter runs, and enum/const/default
strings with more than three words, are lifted to the wrapped docs. Updates the
earlier test that pinned an alternation of two long words as structural.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
checkMediaPathSensitivity consulted only a short home-centric list, so private keys, PEM/key files, tfstate and similar were uploadable from the CWD or temp dir. It now also applies danger.IsSensitiveReadPath on the symlink-resolved path.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
splitPlainSegment cut long prose beside a code fence without closing and reopening bold/italic entities, so Telegram rejected the chunks. Both split paths now share one entity-balanced cut helper.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
StripMarkdown dropped '_', '*' and '~' inside code spans and pre blocks, corrupting identifiers and globs. Code content is now kept verbatim; only delimiters and code escapes are removed.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A paired '**' was passed through, which MarkdownV2 parses as two empty bold entities, so bold never rendered. It is now emitted as a single '*' pair (and adds no markers inside an already-bold header). Existing tests that pinned the '**' passthrough are updated.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The fallback transport client used a 30 s timeout, equal to the default poll_timeout, so idle getUpdates polls timed out and the poller backed off. Both the primary and fallback clients now use one 75 s timeout (above poll_timeout max 60 s plus margin).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
doUpload buffered the whole file with no bound. Files over 50 MB are now refused up front, and the copy is length-limited.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…llback

sendTelegramResult re-sent the escaped chunk as plain text after any error. It now goes through a context-aware SendWithPlainFallback: retry only on entity-parse errors, with markup stripped.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A corrupt counter file read as zero usage, resetting the budget; it now counts as spent. The counter file was keyed by the local date while users are told it resets at midnight UTC; it is now keyed by the UTC date.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…rs the chat lock

/plan_view truncated at a raw byte offset and could emit invalid UTF-8. /plan_resume mutated and saved the session without the per-chat lock, racing an in-flight run; it now declines while a run holds the lock.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
jkyberneees and others added 25 commits October 12, 2026 16:38
…ings in classifier and denylist

echo 'rm -rf /' | parallel classified safe and the denylist missed echo 'git push' | parallel,
parallel {} ::: cmd, xargs sh -c <<< cmd and xargs -I% %. One composition (argvCompositions)
now expands stdin lines, ::: sources, here-strings and -I/{} placeholders for both; a parallel
with no command and unknowable input is unknown.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…osition and shared history table

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ol list

A parent without a filter wrote its built tool list as the child's allow-list,
so Telegram/schedule children lost skill tools and continue children lost bg_*.
Now only a configured allow/deny filter is forwarded and the child applies it.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Spacing after the opener hid joined tag look-alikes. Spaced forms are now
rewritten when they have the full tag shape (separator-joined words and a
nonce suffix, close or end); plain prose stays untouched.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Only a following --name flag now means the credential flag took no value;
a value such as -AbC123 or - is masked in config_view.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…g dir

Documents in a secrets/ folder outside the bot's working directory were
refused; the directory rules now apply relative to the working dir only.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…pped references consumed

A rejected unquoted value was scanned to the end of the line for every later name (quadratic); rejected values are now judged inside a bounded window and only an accepted value is scanned to its real end. A } or ] ends an unquoted value only when it closes a bracket opened before the name on the same line, so secrets containing brackets are redacted whole again. A skipped variable or template reference is consumed whole so later passes cannot redact inside it.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…acted

Values that start with & or * or {, identifier or selector expressions ending a struct field or call, @env: references and home-relative paths are code or references, not literal secrets.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…arry secrets

GithubToken=<credential> was left visible; such names are now treated as loose credential names, redacted when the value has a generated-credential shape so code stays readable.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…cument credential value rules

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…udget

Crafted xargs/parallel commands multiplied lines, placeholders and words into
gigabytes of composed tokens. Composition now stops at the shared token and
byte budget; the classifier fails closed as unknown and the denylist scan
denies.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…iles are secret reads

A single-letter environ glob (e*, [e]*) and wildcards over ~/.bash*, ~/.zsh*,
~/.*history, ~/.*rc classified safe while the literal paths are not. Globs
are now matched against the environ name and the shared rc/history tables.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
git loads $HOME/.gitconfig and $XDG_CONFIG_HOME/git/config, which can name
code to run (core.fsmonitor, pager, aliases). A non-real value, leading or
tracked from an earlier segment, now escalates like GIT_CONFIG_GLOBAL.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…card roots and find|xargs

With cwd=$HOME, name-only and specific-pattern content searches no longer
escalate (credential-aimed, universal or unrecognised-flag searches, archives
and copies still do; /, /proc and /etc stay strict). Wildcard roots
(/ho?e/user) and find <root> | xargs <reader> are now caught.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…lassifications

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A spaced opener followed by space-joined words (< untrusted content_ab12>) was
left alone. It is now rewritten when a nonce suffix or close follows the name
immediately; prose such as 'a < untrusted content > 5' is untouched.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…as source identifiers

An identifier-shaped value ending in , ) or ; is code only when it is short, contains an underscore or has a camelCase transition; a single run of 12+ letters is a passphrase and is redacted.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Every rejected value was rescanned over 4 KiB per name; the first 256 bytes decide, and only an accepted value is scanned to its real end. 1 MiB of aToken= drops from 3.0s to about 1.0s.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…rch patterns are broad

Regex classes, optional groups and repeated stars passed as specific patterns.
Only a metacharacter-free (or -F) literal of 3+ chars that is not credential-
aimed is narrow; an empty quoted operand of grep/rg-family is rewritten to a
match-everything pattern like pkill's. Updates the markEmptyKillPatterns unit
expectation for grep ''.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Lines beyond maxComposedCommands were dropped silently, so a harmful line after
300 benign ones went unexamined. Hitting the cap now reports overflow
(unknown / denylist deny).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
cat /home/other/.bash_history, /root/.bash_history and /home/*/.bash_history
classified safe while the same account's .ssh files did not.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Comment thread cmd/odek/skill_promote.go Fixed
Comment thread cmd/odek/skill_promote.go Fixed
Comment thread cmd/odek/skill_promote.go Fixed
Comment thread cmd/odek/skill_promote.go Fixed
Comment thread cmd/odek/skill_promote.go Fixed
@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Oct 12, 2026 •

Copy link
Copy Markdown

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Preview URL Updated (UTC)
✅ Deployment successful!
View logs
odek 05b2d0a Commit Preview URL

Branch Preview URL
Oct 12 2026, 04:38 PM

jkyberneees and others added 2 commits October 12, 2026 18:25
Skill promote now performs its existence, symlink and write checks through
an os.Root anchored at the skills directory, so a name can never resolve
outside it. Remaining hunks are staticcheck simplifications and removal of
an unused wrapper.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…cleanup

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@jkyberneees
jkyberneees merged commit 3d3b2e1 into main Oct 12, 2026
10 checks 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