fix(security): audit remediation — Telegram path traversal + approval-grant leak

[jkyberneees]

Summary

Starts work on the security/correctness audit remediation. Adds FIX_PLAN.md as the tracking document for all findings, and implements the two highest-priority, lowest-risk fixes (no behavior/default changes, fully tested).

Fixes in this PR

1. Telegram document path traversal — Critical ✅

internal/telegram/download.go. DownloadDocument wrote the sender-controlled Document.FileName through filepath.Join with no sanitization (the variable was named safeName but nothing made it safe). filepath.Join collapses ../, so a document named ../../../home/<user>/.odek/config.json or ../../.ssh/authorized_keys escaped ~/.odek/media/. Both the path and the file contents are attacker-controlled → a remote arbitrary-write / RCE primitive, reachable by anyone who can message the bot.

Fix: new sanitizeDocName() reduces the name to a single path component (filepath.Base) and rejects degenerate names ("", ., ..), falling back to a deterministic doc_<id> name.

2. Batch approval trustAll grant leak — High ✅

internal/loop/loop.go. An approved tool batch set trustAll then reset it via defer inside the iteration loop — so the reset only fired when runLoop returned. The grant stayed active for the rest of the run, auto-approving every subsequent dangerous tool (including destructive/blocked classes). A prompt-injected model could harvest one click on a benign batch and then run anything.

Fix: the grant is reset at the end of each iteration's tool-execution phase, scoped to that iteration.

Tests

• TestSanitizeDocName, TestDownloadDocument_NoTraversal (traversal confined to media dir).
• TestBatchApprovalTrustAllNotLeakedAcrossIterations — verified to fail on the pre-fix code and pass after.
• go build ./..., go vet, and the internal/loop + internal/telegram suites all pass.

Not in this PR (tracked in FIX_PLAN.md)

The remaining audit findings are documented with locations, fixes, and verification steps. Design-sensitive items are intentionally deferred for maintainer direction:

• fix(approval): always render the full command in approval prompts #3 Fail-open Telegram authorization (fail-closed default is a behavior change).
• fix(redact): close secret-leak gaps on the tool output surface #4 / harden(danger): close classifier evasion vectors + fail closed on unknown commands #5 WS//api auth token + serve-mode approval deadlock.
• feat(docker): scheduled reminders via supercronic #6 ~/.odek/ write carve-out / shell-rc classification.
• Plus medium/low items (SSRF dial-time IP check, sub-agent 64KB scanner hang, /new mutex race, etc.).

⚠️ Operational: rotate the live-looking credentials in the working-tree docker/.env if that tree was ever shared/imaged (the file itself is gitignored and not committed).

🤖 Generated with Claude Code

[cloudflare-workers-and-pages]

Deploying with    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	ab00195	Commit Preview URL Branch Preview URL	Jun 10 2026, 10:46 AM

[jkyberneees]

AI Verification Protocol (v5.2.7) — results

Applied the protocol's substantive verification axes adversarially against this diff. Pipeline-orchestration scaffolding (5-agent provider diversity, certificates) was treated as out-of-scope for a self-review; the analytical axes were applied in full.

Classification: KnownGroundTruth (security fixes with regression tests). Changed LOC well under the 1,500 ceiling → standard pipeline.

Axis	Result
2.1 Semantic correctness	✅ sanitizeDocName traced against absolute paths, internal .., root, empty/degenerate input — all collapse to a safe single component; no panic on short/empty fileID.
2.2 Behavioral contract	✅ DownloadDocument signature/return unchanged; behavior for legitimate names preserved.
2.3 Security surface	✅ Path-traversal sink closed at the source; no new injection/secret surface.
2.4 Structural integrity	✅ Pure helper extracted (directly testable); no new coupling. Full suite (21 pkgs) green, -race clean.
2.5 Behavioral exploration	⚠️→✅ Finding: per-iteration trustAll reset did not cover abnormal exits (early return/panic mid-iteration), and the approver is reused across prompts → potential cross-prompt grant leak. Fixed by restoring a function-exit defer backstop (commit fix(loop): add function-exit backstop…).
2.8 Untrusted-input	✅ No verdict-token/injection content; the change itself hardens an untrusted-input boundary.

Test teeth: the trustAll regression test was confirmed to fail on the pre-fix code and pass after — it is not tautological.

Verdict: findings addressed; remaining audit items tracked in FIX_PLAN.md.

[jkyberneees]

Added: Fix #3 — Fail-closed Telegram authorization (High)

internal/telegram/config.go, handler.go, cmd/odek/telegram.go.

Before: an empty allowlist meant allow all — isAllowed returned true when both AllowedChats and AllowedUsers were empty, and the default config left them empty with no warning. Anyone who found the bot could drive the agent (and its shell/file tools).

Now — fail-closed, with an explicit opt-in escape hatch:

• ValidateConfig refuses to start when no allowlist is configured, unless ODEK_TELEGRAM_ALLOW_ALL=true is set.
• isAllowed denies when both lists are empty and AllowAllUsers is unset (defense in depth beyond the startup gate).
• Startup logs a loud warning when running open via the opt-in.

New AllowAllUsers field + ODEK_TELEGRAM_ALLOW_ALL env var, wired through HandlerConfig. Existing deployments that set an allowlist (incl. the bundled .env) are unaffected.

Verification (vprotocol axes)

• 2.1 Semantic: traced isAllowed for all four allowlist combinations; non-empty-list behavior preserved.
• 2.3 Security: ambiguous/malformed input fails closed — parseBool("yes please") and an empty/,-only allowlist both deny rather than open the bot.
• 2.2 Contract: deliberate breaking change for unconfigured bots; documented in TELEGRAM.md, SECURITY.md, .env.example.
• 2.4 Structural: full suite (21 pkgs) green, gofmt/go vet clean.

Tests: TestValidateConfig_noAllowlistFailsClosed / _allowAllOptIn / _allowlistSatisfiesCheck, TestConfigFromEnv_allowAll, TestIsAllowed_EmptyAllowlist (now asserts deny) + TestIsAllowed_EmptyAllowlistWithAllowAll.

Remaining open items (#4/#5 WS auth + serve-mode approval deadlock, #6 ~/.odek carve-out) tracked in FIX_PLAN.md.

[jkyberneees]

Adversarial self-review ("challenge the changes")

Stress-tested this PR against its strongest objections. Results:

Held up (no change needed):

• Does fail-closed ValidateConfig break outbound delivery (odek run --deliver, scheduled reminders)? No — deliverToTelegram (main.go:982) and the scheduler (schedule.go:380) build the bot via NewBot directly and never call ValidateConfig. Only the interactive bot (which ingests untrusted input) is gated.
• sanitizeDocName edge cases — re-traced absolute paths, internal .., root, empty/degenerate input; all safe.
• trustAll per-iteration reset vs. abnormal exit — already covered by the function-exit defer backstop added earlier.

Broke down → fixed in this PR (new commit):

• Callback-query authorization bypass. HandleUpdate routes inline-button presses to handleCallback, which never called isAllowed — only handleMessage did. A characterization test even documented the gap as "current behavior". So the fix(approval): always render the full command in approval prompts #3 fix was incomplete: callbacks could drive per-chat approval/clarify state and trigger outbound API calls unauthenticated. handleCallback now applies the same allowlist (cq.Message.Chat.ID + cq.From.ID). New/updated tests assert the deny path.

Residual, noted not fixed (pre-existing, lower priority):

• In a group chat with only AllowedChats set, any group member can press another user's approval/clarify buttons. Properly fixing this means binding an approval to the user who triggered the run — out of scope here. Tracked in FIX_PLAN.md.

Honest cost: fail-closed is a breaking change — bots intentionally run open (no allowlist) will stop starting on upgrade until ODEK_TELEGRAM_ALLOW_ALL=true is set. Deliberate trade-off; the startup error is explicit.