Skip to content

fix(cli): allow runtime permission mode escalation to bypassPermissions#130

Open
lucharo wants to merge 1 commit intohappier-dev:devfrom
lucharo:fix/allow-runtime-permission-escalation
Open

fix(cli): allow runtime permission mode escalation to bypassPermissions#130
lucharo wants to merge 1 commit intohappier-dev:devfrom
lucharo:fix/allow-runtime-permission-escalation

Conversation

@lucharo
Copy link

@lucharo lucharo commented Mar 14, 2026

Summary

  • Always set allowDangerouslySkipPermissions: true at Agent SDK session creation, so that setPermissionMode('bypassPermissions') works on follow-up messages

Problem

When a session is started with a non-yolo permission mode (e.g. default or acceptEdits) and the user later switches to yolo mode from the phone, the runtime setPermissionMode('bypassPermissions') call throws:

Cannot set permission mode to bypassPermissions because the session was not launched with --dangerously-skip-permissions

This surfaces as:

Failed to update runtime settings (non-fatal); continuing.

The user sees the error on their phone, and the session continues running with the wrong permission mode — it stays on whatever mode it was launched with instead of switching to yolo.

Root cause

At session creation (claudeRemoteAgentSdk.ts), allowDangerouslySkipPermissions is only set to true when the initial mode is bypassPermissions:

allowDangerouslySkipPermissions: mappedPermissionMode === 'bypassPermissions',

But on follow-up messages, setPermissionMode() is called to update the mode — and it cannot escalate to bypassPermissions because the Claude Agent SDK requires allowDangerouslySkipPermissions to have been set at launch time.

Fix

Always set allowDangerouslySkipPermissions: true at session creation. This does not change the initial permission mode (still controlled by permissionMode: mappedPermissionMode). It only removes the gate that prevents upgrading permissions at runtime via setPermissionMode().

Test plan

  • Start a session from terminal with default permissions
  • From phone, switch to yolo mode mid-session
  • Verify no "Failed to update runtime settings" error
  • Verify the session actually runs in bypassPermissions mode after switching

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Refactor
    • Updated Claude agent permission handling logic to simplify the permission skip configuration.

…rmissions

When a session is started without bypassPermissions but the user later
switches to yolo mode from the phone, setPermissionMode('bypassPermissions')
throws because allowDangerouslySkipPermissions was not set at launch time.

Fix by always setting allowDangerouslySkipPermissions: true at session
creation. This does not change the initial permission mode — it only
removes the gate that prevents upgrading permissions at runtime via
setPermissionMode.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@coderabbitai
Copy link

coderabbitai bot commented Mar 14, 2026

Walkthrough

A single-line modification in the Claude remote agent SDK replaces conditional permission-skipping logic with a hard-coded true value, causing explicit permission checks to be unconditionally bypassed regardless of the mapped permission mode configuration.

Changes

Cohort / File(s) Summary
Claude Remote Agent SDK
apps/cli/src/backends/claude/remote/claudeRemoteAgentSdk.ts
Modified allowDangerouslySkipPermissions field from conditional logic (mappedPermissionMode === 'bypassPermissions') to always true, removing permission mode gating.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: enabling runtime permission mode escalation to bypassPermissions by always allowing the dangerous skip at session creation.
Description check ✅ Passed The description covers Summary, Problem with root cause analysis, and Fix with test plan. All main template sections are present with concrete detail, though the test plan is marked as incomplete (unchecked boxes).

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
📝 Coding Plan
  • Generate coding plan for human review comments

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Tip

You can make CodeRabbit's review stricter and more nitpicky using the `assertive` profile, if that's what you prefer.

Change the reviews.profile setting to assertive to make CodeRabbit's nitpick more issues in your PRs.

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Mar 14, 2026

Greptile Summary

This PR fixes a bug where switching to bypassPermissions mode at runtime (e.g., from the phone) would silently fail because the Agent SDK session was not launched with allowDangerouslySkipPermissions: true. The fix always sets that flag at session creation, which only removes the gate for runtime escalation — the initial permissionMode still controls the actual starting behavior.

  • Always passes allowDangerouslySkipPermissions: true at session creation to unblock runtime setPermissionMode('bypassPermissions') calls
  • The existing test in claudeRemoteAgentSdk.optionsAndHooks.test.ts (line 103) asserts the old conditional behavior and will now fail — it needs to be updated to expect true for all permission modes

Confidence Score: 3/5

  • The logic fix is sound but the PR ships with a breaking test that needs updating before merge.
  • The one-line change correctly solves the reported runtime escalation bug and does not alter the initial permission mode. However, the existing unit test at claudeRemoteAgentSdk.optionsAndHooks.test.ts:103 will fail because it asserts the old conditional behavior (false for non-bypass modes). This should be fixed before merging.
  • apps/cli/src/backends/claude/remote/claudeRemoteAgentSdk.optionsAndHooks.test.ts — test assertion on line 103 contradicts the new behavior

Important Files Changed

Filename Overview
apps/cli/src/backends/claude/remote/claudeRemoteAgentSdk.ts Always sets allowDangerouslySkipPermissions: true to enable runtime permission escalation to bypassPermissions. The logic change is correct but breaks an existing test assertion.

Sequence Diagram

sequenceDiagram
    participant User as User (Phone)
    participant Server as Happier Server
    participant CLI as CLI Session
    participant SDK as Agent SDK

    User->>Server: Switch to yolo mode
    Server->>CLI: nextMessage (mode: bypassPermissions)
    CLI->>SDK: setPermissionMode('bypassPermissions')
    
    alt Before this PR (allowDangerouslySkipPermissions=false at launch)
        SDK-->>CLI: Error: session not launched with --dangerously-skip-permissions
        CLI-->>Server: Failed to update runtime settings (non-fatal)
        Note over CLI: Session stays on original mode
    end
    
    alt After this PR (allowDangerouslySkipPermissions=true at launch)
        SDK-->>CLI: Success
        CLI-->>Server: Mode updated
        Note over CLI: Session now runs in bypassPermissions
    end
Loading

Last reviewed commit: 0444a46

settingSources,
permissionMode: mappedPermissionMode,
allowDangerouslySkipPermissions: mappedPermissionMode === 'bypassPermissions',
allowDangerouslySkipPermissions: true,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Existing test will now fail

The test in claudeRemoteAgentSdk.optionsAndHooks.test.ts:103 asserts:

expect((await runOnce('default'))?.allowDangerouslySkipPermissions).toBe(false);

With this change, allowDangerouslySkipPermissions is now always true, so this assertion will fail. The test (line 55) and its description — "sets allowDangerouslySkipPermissions only when permissionMode is bypassPermissions" — need to be updated to reflect the new behavior (e.g., assert true for all modes and rename the test case).

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
apps/cli/src/backends/claude/remote/claudeRemoteAgentSdk.ts (1)

553-553: Security consideration: Document intent in a comment.

Hardcoding allowDangerouslySkipPermissions: true is intentional to enable runtime escalation to bypassPermissions mode. While the actual permission enforcement still flows through permissionMode (line 552) and the canUseTool/PermissionRequest hooks, a brief inline comment would clarify this is deliberate and not a security oversight.

📝 Suggested clarifying comment
         permissionMode: mappedPermissionMode,
-        allowDangerouslySkipPermissions: true,
+        // Always allow so runtime escalation to 'bypassPermissions' via setPermissionMode() succeeds.
+        // Initial enforcement is controlled by permissionMode above; hooks still validate tool calls.
+        allowDangerouslySkipPermissions: true,
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/cli/src/backends/claude/remote/claudeRemoteAgentSdk.ts` at line 553, Add
a brief inline comment next to the allowDangerouslySkipPermissions: true setting
to explain this is intentional to allow runtime escalation to bypassPermissions
mode; reference that actual enforcement is handled by permissionMode, the
canUseTool hook, and PermissionRequest flow so reviewers know this is not an
accidental insecure default and that permission checks remain centralized.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@apps/cli/src/backends/claude/remote/claudeRemoteAgentSdk.ts`:
- Line 553: Add a brief inline comment next to the
allowDangerouslySkipPermissions: true setting to explain this is intentional to
allow runtime escalation to bypassPermissions mode; reference that actual
enforcement is handled by permissionMode, the canUseTool hook, and
PermissionRequest flow so reviewers know this is not an accidental insecure
default and that permission checks remain centralized.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: f62a8427-f677-4f81-a498-7389e538f7b3

📥 Commits

Reviewing files that changed from the base of the PR and between cd992ae and 0444a46.

📒 Files selected for processing (1)
  • apps/cli/src/backends/claude/remote/claudeRemoteAgentSdk.ts

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.

1 participant