Skip to content

fix(desktop): validate openExternal URLs by protocol#30666

Open
ulises-jeremias wants to merge 3 commits into
anomalyco:devfrom
ulises-jeremias:fix/desktop-open-link-protocol-validation
Open

fix(desktop): validate openExternal URLs by protocol#30666
ulises-jeremias wants to merge 3 commits into
anomalyco:devfrom
ulises-jeremias:fix/desktop-open-link-protocol-validation

Conversation

@ulises-jeremias

@ulises-jeremias ulises-jeremias commented Jun 4, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Issue for this PR

Closes #30613

Type of change

  • Bug fix
  • New feature
  • Refactor / code improvement
  • Documentation

What does this PR do?

The desktop open-link IPC handler called shell.openExternal(url) without validating the URL scheme.

This PR adds a protocol allowlist guard and blocks non-web protocols before opening links externally.

Allowed protocols:

  • http:
  • https:

Blocked examples:

  • file:
  • javascript:
  • smb:
  • ms-msdt:
  • malformed strings

How did you verify your code works?

  • Added focused tests in packages/desktop/src/main/external-link.test.ts:
    • allows http / https
    • blocks unsafe protocols
    • blocks malformed/relative inputs
  • Ran:
    • bun test src/main/external-link.test.ts (from packages/desktop)
    • bun typecheck (from packages/desktop)

Screenshots / recordings

Not a UI change.

Checklist

  • I have tested my changes locally
  • I have not included unrelated changes in this PR

@github-actions github-actions Bot added contributor needs:compliance This means the issue will auto-close after 2 hours. needs:issue labels Jun 4, 2026
@github-actions

github-actions Bot commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

Thanks for your contribution!

This PR doesn't have a linked issue. All PRs must reference an existing issue.

Please:

  1. Open an issue describing the bug/feature (if one doesn't exist)
  2. Add Fixes #<number> or Closes #<number> to this PR description

See CONTRIBUTING.md for details.

@github-actions

github-actions Bot commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

The following comment was made by an LLM, it may be inaccurate:

Potential Duplicate Found:

This appears to be addressing the same issue as PR #30666 - validating URLs/protocols for the openExternal functionality in the desktop app. Both PRs target the same security concern and likely fix the same problem.

@github-actions github-actions Bot removed needs:compliance This means the issue will auto-close after 2 hours. needs:issue labels Jun 4, 2026
@github-actions

github-actions Bot commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

Thanks for updating your PR! It now meets our contributing guidelines. 👍

@ulises-jeremias

ulises-jeremias commented Jun 4, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the duplicate signal. I intentionally kept this PR open because it adds two concrete pieces beyond the minimal inline guard:

  1. Dedicated helper module (packages/desktop/src/main/external-link.ts)

    • isolates URL-safety policy from IPC wiring
    • keeps future protocol policy changes centralized

  2. Focused regression tests (packages/desktop/src/main/external-link.test.ts)

    • verifies allowed http/https
    • verifies blocked unsafe protocols (file, javascript, smb, ms-msdt)
    • verifies malformed input handling

So this PR contributes test coverage and a reusable boundary, not only the inline check.

If maintainers prefer, I can also align protocol policy with #30614 (e.g. mailto) while preserving this test-backed structure.

@github-actions github-actions Bot mentioned this pull request Jun 4, 2026
Open
@ulises-jeremias
Merge branch 'dev' into fix/desktop-open-link-protocol-validation
e91c1aa 
* upstream/dev: (454 commits)
  chore: update nix node_modules hashes
  feat(oauth): unify OAuth callback browser pages (anomalyco#34025)
  chore: generate
  refactor(app): replace tab drag handling with dndkit (anomalyco#33880)
  refactor(app): use dropdown for project selector (anomalyco#33984)
  chore: update nix node_modules hashes
  fix(app): bump ghostty-web to prevent terminal resize hangs (anomalyco#34020)
  chore: generate
  feat(sdk): expose active sessions (anomalyco#33991)
  fix(sdk): wake embedded session execution (anomalyco#33992)
  refactor(opencode): stop legacy v2 event emission (anomalyco#33993)
  fix(ui): normalize tooltip trigger layout (anomalyco#33979)
  fix(app): suspend hidden terminal renderer (anomalyco#33990)
  fix(app): enable auto-accept in session settings (anomalyco#33974)
  tweak: hide MCP access token prefix (anomalyco#33711)
  fix(core): refine small model defaults (anomalyco#33926)
  feat(core): split MCP timeout configuration (anomalyco#33977)
  fix(app): simplify question prompt (anomalyco#33968)
  feat(llm): pass strict through tool definitions for Codex parity (anomalyco#33392)
  chore: generate
  ...
@ulises-jeremias

ulises-jeremias commented Jun 26, 2026

Copy link
Copy Markdown
Contributor Author

@adamdotdevin @Hona @Brendonovich

Rebased onto latest dev (merge commit pushed). CI should re-run shortly — would appreciate a review when you have a moment. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@adamdotdevin adamdotdevin Awaiting requested review from adamdotdevin
@Hona Hona Awaiting requested review from Hona Hona is a code owner
@Brendonovich Brendonovich Awaiting requested review from Brendonovich Brendonovich is a code owner

Assignees

No one assigned

Labels

contributor

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

fix(desktop): shell.openExternal called without URL protocol validation

1 participant

@ulises-jeremias