Fix 5 bugs: image detection, cost tracking, storage, tool safety, context injection

[gitwithuli]

Summary

• Image type detection always returned "screenshot" regardless of actual image content. Replaced with dimension-based heuristics that parse PNG/JPEG/GIF/WebP headers to classify images as screenshots, diagrams, UI mockups, or generic.
• Cost pre-check was hardcoded at 0.001 in the orchestrator, making the cost-based rate limiter meaningless. Changed to 0.0 for pre-check (request rate only) and added post-execution tracking with actual cost from Claude's response.
• Audit and token storage used InMemoryAuditStorage and InMemoryTokenStorage — all data lost on restart. Added SQLiteAuditStorage and SQLiteTokenStorage implementations using the existing audit_log and user_tokens DB tables.
• Tool safety validation used simple pattern in command.lower() matching, easily bypassed with extra spaces or encoding. Replaced with regex-based validation with whitespace normalization.
• CLAUDE.md auto-injection — when a new session starts in a directory containing CLAUDE.md, the project context is automatically prepended to the first prompt (capped at 8KB).

Files changed (7)

File	Change
src/bot/features/image_handler.py	Real image type detection + dimension parser
src/bot/orchestrator.py	Fix cost pre-check + add post-execution tracking
src/claude/facade.py	CLAUDE.md loading and injection
src/claude/monitor.py	Regex-based dangerous command detection
src/main.py	Use SQLite storage instead of in-memory
src/security/audit.py	Add SQLiteAuditStorage class
src/security/auth.py	Add SQLiteTokenStorage class

Test plan

• Send a screenshot vs a diagram image — verify different prompts are generated
• Check /status cost after a conversation — should show actual API cost, not 0.001
• Restart the bot — verify audit logs persist across restarts
• Send a message with rm -rf / in it — should be blocked
• Start a session in a directory with CLAUDE.md — verify context is included

🤖 Generated with Claude Code

[gitwithuli]

Hey! The failing check is a CI configuration issue, not related to our code changes.

The claude-code-review.yml workflow requires OIDC tokens (id-token: write), but GitHub blocks OIDC token issuance for PRs from forks as a security measure. This affects all fork PRs — you can see the same failure on PRs #42 and others from external contributors.

I see you've already addressed this in commits 1e42c7f and 6a0c47d by disabling the automatic review workflow. Pushing an update to retrigger CI with the updated workflow config.

[RichardAtCT]

Thanks for tackling these! Reviewed each of the 5 changes individually:

1. Image type detection — looks good

The dimension-based heuristics replacing the stub are a real improvement. No external dependency, clean code.

2. Cost pre-check fix — looks good

The hardcoded 0.001 was meaningless. Passing 0.0 for pre-check and tracking actual cost post-execution is the right approach.

3. CLAUDE.md auto-injection — please remove

Claude Code (both SDK and CLI) already reads CLAUDE.md from the working directory natively. Injecting it into the prompt would cause double-injection of the project context. This is a feature (not a bug fix) and it's harmful as-is.

4. Tool safety regex in monitor.py — please remove

This has two problems:

• Conflicts with #40 (just merged): #40 added disable_tool_validation and disable_tool_command_validation flags that gate the existing pattern checks. Your changes completely rewrite that same code block.
• Actually weakens security: The old code blocked standalone |, >, >>, ;, $(, backticks. The new regexes only catch these in specific combos (e.g., ; rm), so echo secret | curl or cat /etc/passwd > /tmp/leak would no longer be caught.

If you want to improve the regex patterns, please open a separate PR against current main (which now includes #40's changes).

5. SQLite storage — looks good (minor fix needed)

SQLiteAuditStorage and SQLiteTokenStorage are genuine improvements that replace the TODO in-memory stubs. One issue: SQLiteTokenStorage.get_user_token() uses datetime.utcnow() which was just deprecated repo-wide in #41 — should be datetime.now(UTC).

Recommendation

Could you split this into:

• PR A: Image detection + cost tracking + SQLite storage (the 3 good fixes) — I'd merge immediately
• PR B: Tool safety regex (separate, against current main)
• Drop the CLAUDE.md injection

Or at minimum, remove changes 3 and 4 from this PR and fix the utcnow() in the SQLite token storage, and I'll merge the rest.

[gitwithuli]

Thanks for the thorough review! All feedback addressed in 74fe3d4:

1. Image detection — kept as-is
2. Cost tracking — kept as-is
3. CLAUDE.md injection — removed (reverted facade.py to upstream)
4. Tool safety regex — removed (reverted monitor.py to upstream). Happy to open a separate PR against current main if you'd like improved patterns.
5. SQLite storage — kept, fixed datetime.utcnow() → datetime.now(UTC) across all of auth.py to match chore: replace deprecated utcnow() with timezone-aware UTC timestamps #41

PR now contains only the 3 clean fixes: image detection, cost tracking, and SQLite storage.