Skip to content

Block state-changing debug deep links#15

Open
kobemartin wants to merge 1 commit into
b-nnett:mainfrom
kobemartin:codex/block-state-change-debug-deep-links
Open

Block state-changing debug deep links#15
kobemartin wants to merge 1 commit into
b-nnett:mainfrom
kobemartin:codex/block-state-change-debug-deep-links

Conversation

@kobemartin

@kobemartin kobemartin commented Jun 3, 2026

Copy link
Copy Markdown

Summary

  • allow external debug-command deep links to invoke read-only research commands only
  • block state-changing and unknown-risk command categories before any Bluetooth write
  • hide remote URL examples for commands that cannot be invoked remotely

Why

The custom URL scheme previously allowed an external app or webpage to immediately invoke a state-changing WHOOP research command while Goose was connected.

Validation

  • git diff --check
  • swiftc -parse GooseSwift/*.swift GooseWorkoutLiveActivityExtension/*.swift

tigercraft4 pushed a commit to tigercraft4/goose that referenced this pull request Jun 3, 2026
- allowsRemoteInvocation computed property on GooseDebugCommandDefinition:
  only 'read' and 'keyed read' risk commands can be invoked via URL scheme
- handleDebugCommandDeepLink validates command exists and allowsRemoteInvocation
  before executing; unknown or state-changing commands are blocked + logged
- remoteURLExample returns 'Remote invocation disabled' for blocked commands

Ported from b-nnett#15 (kobemartin)

var allowsRemoteInvocation: Bool {
risk == "read" || risk == "keyed read"
}

@tigercraft4 tigercraft4 Jun 5, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Stringly-typed allowlist fails open on misclassification.

risk == "read" || risk == "keyed read" means a typo in a future command's risk field — or a new command added with a mislabeled string — silently becomes remotely invokable. There is no compiler check that the two literal strings are the only safe values.

Suggested fix: convert risk to a CommandRisk enum:

enum CommandRisk: String {
    case read = "read"
    case keyedRead = "keyed read"
    case write
    case unknown
}

var allowsRemoteInvocation: Bool {
    risk == .read || risk == .keyedRead
}

The compiler then enforces the allowlist exhaustively — a new risk variant that isn't listed here will not be remotely invokable by default.

return true
}
guard command.allowsRemoteInvocation else {
ble.setDebugCommandStatus("\(command.title) blocked from external deep link")

@tigercraft4 tigercraft4 Jun 5, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Confirm this is the only deep-link enforcement site.

The gate correctly consults allowsRemoteInvocation here, but verify no other code path (e.g. payload-hex form, Shortcuts integration, or a second handleDeepLink branch) bypasses this guard. A single enforcement point is the right design; the risk is that a second entry point added later will not automatically inherit it.

Consider emitting an explicit audit log when a deep link is rejected so internal tooling can detect and reclassify affected commands:

ble.record(level: .warn, source: "ble.debug_command",
    title: "deep_link.blocked",
    body: "\(command.id) risk=\(command.risk) — allowsRemoteInvocation=false")

This also helps diagnose broken Shortcuts automations without requiring a debugger.

tigercraft4 pushed a commit to tigercraft4/goose that referenced this pull request Jun 5, 2026
- allowsRemoteInvocation computed property on GooseDebugCommandDefinition:
  only 'read' and 'keyed read' risk commands can be invoked via URL scheme
- handleDebugCommandDeepLink validates command exists and allowsRemoteInvocation
  before executing; unknown or state-changing commands are blocked + logged
- remoteURLExample returns 'Remote invocation disabled' for blocked commands

Ported from b-nnett#15 (kobemartin)
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