Skip to content

fix: detect logged-out Claude plugin sessions - #6

Merged
Kshitij Jhunjhunwala (KJ-11) merged 3 commits into
masterfrom
kj/fix-whoami-auth-probe
Jul 13, 2026
Merged

Kshitij Jhunjhunwala (KJ-11) merged 3 commits into
masterfrom
kj/fix-whoami-auth-probe

Conversation

@KJ-11

@KJ-11 Kshitij Jhunjhunwala (KJ-11) commented Jul 10, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • treat CLI 0.2.31 exit-0 empty output as logged out
  • preserve authenticated output from old and new CLI formats
  • recognize explicit authenticated: false and logged-out warnings
  • rely on the existing 8-second Claude hook timeout instead of custom watchdog processes
  • bump the plugin version to 0.2.1

Implementation

The hook defaults to logged out and reports signed in only when composio whoami succeeds, returns non-whitespace output, and does not explicitly report a logged-out state. The toolkit cache still refreshes in parallel, with the hook-level timeout bounding the entire SessionStart operation.

Validation

  • 37 unit tests
  • shell syntax and diff checks
  • Claude marketplace and plugin validation
  • clean-HOME smoke with published CLI 0.2.31

@sparkle-code-security

Copy link
Copy Markdown
Review Summary · Total 1 · 🟤 Critical 0 · 🔴 High 1

plugins/composio/hooks/session-start.sh

🔴 ReDoS & Catastrophic Regex Backtracking Prevention · Line 57 · 1 violation

Issue
The regex pattern contains a catastrophic backtracking vulnerability with .* in api[ _-]?key.*(missing|not[[:space:]]+(set|found)|invalid). While input is bounded to 8192 bytes via head -c 8192, the greedy .* followed by alternation can cause exponential backtracking if the input contains patterns like api key followed by characters that don't match the alternation.

Recommended fix

Replace the vulnerable pattern segment `api[ _-]?key.*(missing|not[[:space:]]+(set|found)|invalid)` with a possessive or
more specific pattern that doesn't use greedy `.*` before alternation. For example, use `api[
_-]?key[^:]*?(missing|not[[:space:]]+(set|found)|invalid)` or split the check into two separate grep operations to avoid
nested backtracking. Alternatively, reduce the input limit to a more reasonable size (e.g., 1024 or 512 bytes) to
further mitigate backtracking impact.

Sparkle AI One-Click Remediation Use this prompt in your AI assistant or CLI to patch all findings in one pass.
Fix all 1 issue flagged by the Sparkle PR review below.
For each issue, update the specified file and line while preserving existing behavior and surrounding code style.

[Line 57] ReDoS & Catastrophic Regex Backtracking Prevention (high)
File: plugins/composio/hooks/session-start.sh
Issue: The regex pattern contains a catastrophic backtracking vulnerability with `.*` in `api[
_-]?key.*(missing|not[[:space:]]+(set|found)|invalid)`. While input is bounded to 8192 bytes via `head -c 8192`, the
greedy `.*` followed by alternation can cause exponential backtracking if the input contains patterns like `api key`
followed by characters that don't match the alternation.
Fix: Replace the vulnerable pattern segment `api[ _-]?key.*(missing|not[[:space:]]+(set|found)|invalid)` with a
possessive or more specific pattern that doesn't use greedy `.*` before alternation. For example, use `api[
_-]?key[^:]*?(missing|not[[:space:]]+(set|found)|invalid)` or split the check into two separate grep operations to avoid
nested backtracking. Alternatively, reduce the input limit to a more reasonable size (e.g., 1024 or 512 bytes) to
further mitigate backtracking impact.

Blast Radius Analysis No connected services identified. Changes appear isolated to the current repository.

@sparkle-code-security

Copy link
Copy Markdown
Review Summary · Total 0 · 🟤 Critical 0 · 🔴 High 0

Blast Radius Analysis No connected services identified. Changes appear isolated to the current repository.

@sparkle-code-security

Copy link
Copy Markdown
Review Summary · Total 1 · 🟤 Critical 0 · 🔴 High 1

plugins/composio/hooks/session-start.sh

🔴 Resource Lifecycle Management — No Leaks · Line 87 · 1 violation

Issue
The composio whoami 2>&1 call lacks an explicit operation-level timeout. This I/O operation could potentially block the hook for the entire 8-second manifest-level timeout if the composio CLI hangs or is slow. The original code enforced a 3-second internal timeout using background processes with SIGTERM. Removing this operation-level timeout violates the defense-in-depth principle for timeout handling.

Recommended fix

Add explicit operation-level timeout handling around the `composio whoami` call. Restore the timeout mechanism from the
original code: wrap the command in a background process with a timeout watcher (e.g., `timeout 3 composio whoami 2>&1`
if available, or spawn with `( sleep 3; kill -TERM "$pid" 2>/dev/null ) & pid=$!` pattern). This ensures the operation
fails fast (after 3 seconds) rather than blocking up to the hook's 8-second timeout.

Sparkle AI One-Click Remediation Use this prompt in your AI assistant or CLI to patch all findings in one pass.
Fix all 1 issue flagged by the Sparkle PR review below.
For each issue, update the specified file and line while preserving existing behavior and surrounding code style.

[Line 87] Resource Lifecycle Management — No Leaks (high)
File: plugins/composio/hooks/session-start.sh
Issue: The `composio whoami 2>&1` call lacks an explicit operation-level timeout. This I/O operation could potentially
block the hook for the entire 8-second manifest-level timeout if the composio CLI hangs or is slow. The original code
enforced a 3-second internal timeout using background processes with SIGTERM. Removing this operation-level timeout
violates the defense-in-depth principle for timeout handling.
Fix: Add explicit operation-level timeout handling around the `composio whoami` call. Restore the timeout mechanism from
the original code: wrap the command in a background process with a timeout watcher (e.g., `timeout 3 composio whoami
2>&1` if available, or spawn with `( sleep 3; kill -TERM "$pid" 2>/dev/null ) & pid=$!` pattern). This ensures the
operation fails fast (after 3 seconds) rather than blocking up to the hook's 8-second timeout.

Blast Radius Analysis No connected services identified. Changes appear isolated to the current repository.

@KJ-11
Kshitij Jhunjhunwala (KJ-11) marked this pull request as ready for review July 13, 2026 17:59
@KJ-11

Copy link
Copy Markdown
Contributor Author

Thanks for flagging this. This is intentional: the SessionStart command already has a host-enforced timeout: 8 in hooks.json, and the test suite now guards that boundary. We removed the separate three-second shell watchdog because it duplicated the host timeout and introduced its own child-process and pipe lifecycle complexity. The tradeoff is that a hung whoami may consume the full eight-second hook budget, but the operation remains bounded and Claude can continue after the hook timeout. We are accepting that tradeoff and do not plan an additional operation-level watchdog in this PR.

@KJ-11
Kshitij Jhunjhunwala (KJ-11) merged commit 5858b99 into master Jul 13, 2026
6 checks passed
@KJ-11
Kshitij Jhunjhunwala (KJ-11) deleted the kj/fix-whoami-auth-probe branch July 16, 2026 22:54
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