Skip to content

fix: atomic Slack approval guard, safe JSON deserialization fallbacks in state DB#6705

Closed
aaronlab wants to merge 1 commit into
NousResearch:mainfrom
aaronlab:fix/slack-toctou-state-deser
Closed

fix: atomic Slack approval guard, safe JSON deserialization fallbacks in state DB#6705
aaronlab wants to merge 1 commit into
NousResearch:mainfrom
aaronlab:fix/slack-toctou-state-deser

Conversation

@aaronlab

@aaronlab aaronlab commented Apr 9, 2026

Copy link
Copy Markdown
Contributor

Summary

  • gateway/platforms/slack.py (HIGH-security): Replace non-atomic check-then-set on _approval_resolved with atomic dict.pop(). Two concurrent Slack button click handlers (async coroutines) could both pass the if .get() guard before either set it to True. This causes resolve_gateway_approval() to be called twice — and when multiple approvals are queued for the same session, the second call resolves the wrong pending command approval.
  • hermes_state.py (HIGH-reliability): Add logger.warning() and proper fallbacks when json.loads() fails on tool_calls (→ []), reasoning_details (→ None), and codex_reasoning_items (→ None). Previously, 4 except: pass blocks silently swallowed deserialization errors: tool_calls stayed as a raw string (causing downstream code to iterate over characters instead of tool call objects), and reasoning fields were simply missing from the message dict.

Test plan

  • Verify rapid double-click on Slack approval button only resolves once
  • Corrupt a tool_calls JSON value in state.db and verify WARNING appears + [] fallback
  • Verify conversation replay with corrupted reasoning_details doesn't crash

🤖 Generated with Claude Code

1. gateway/platforms/slack.py: Replace check-then-set TOCTOU race on
   _approval_resolved with atomic dict.pop(). Two concurrent button
   clicks could both pass the guard before either set it to True,
   causing double resolve_gateway_approval — which can resolve the
   WRONG queued approval when multiple are pending for the same session.

2. hermes_state.py: Add WARNING log and proper fallbacks when
   json.loads fails on tool_calls (→ []), reasoning_details (→ None),
   and codex_reasoning_items (→ None). Previously, failures were
   silently swallowed: tool_calls stayed as a raw string (iterating
   yields characters, not objects), and reasoning fields were simply
   missing from the dict.

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
@teknium1

teknium1 commented Apr 9, 2026

Copy link
Copy Markdown
Contributor

Merged via PR #6809 as part of a consolidated Slack adapter improvement. Your contribution was cherry-picked directly. Your authorship is preserved in git history. Thank you @aaronagent for your work on this!

@teknium1 teknium1 closed this Apr 9, 2026
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