Skip to content

fix(security): block private/reserved IPs for hosted 1Password Connect SSRF#4818

Merged
waleedlatif1 merged 2 commits into
stagingfrom
waleedlatif1/verify-code-changes-v1
May 31, 2026
Merged

fix(security): block private/reserved IPs for hosted 1Password Connect SSRF#4818
waleedlatif1 merged 2 commits into
stagingfrom
waleedlatif1/verify-code-changes-v1

Conversation

@waleedlatif1
Copy link
Copy Markdown
Collaborator

Summary

  • 1Password Connect-mode routes accept a user-supplied serverUrl. The prior fix blocked link-local/metadata but still allowed localhost + RFC1918, leaving a workflow-authenticated read-SSRF on the hosted service (e.g. serverUrl=http://127.0.0.1: reflected through list-vaults/get-item).
  • Gate the IP policy on deployment type in validateConnectServerUrl: on hosted (isHosted), block all private/reserved IPs via isPrivateOrReservedIP; on self-hosted, keep allowing private destinations (operator controls the network; Connect lives on RFC1918) and continue blocking link-local.
  • Policy is enforced against the DNS-resolved IP, so hostnames resolving to private addresses are also blocked. DNS pinning preserved.
  • Added unit tests covering hosted vs self-hosted across loopback, RFC1918, link-local, and public IPs (literal + DNS-resolved).

Type of Change

  • Bug fix

Testing

  • bunx vitest run app/api/tools/onepassword/utils.test.ts — 15 passed
  • bun run check:api-validation — passed
  • Biome lint clean

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

@vercel
Copy link
Copy Markdown

vercel Bot commented May 31, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
docs Skipped Skipped May 31, 2026 1:09am

Request Review

@cursor
Copy link
Copy Markdown

cursor Bot commented May 31, 2026

PR Summary

High Risk
Closes a workflow-authenticated SSRF path on the hosted service where Connect serverUrl could reach localhost/internal networks; behavior change is intentional and scoped by deployment mode.

Overview
Tightens SSRF protection for user-supplied 1Password Connect serverUrl values by splitting policy on isHosted.

On hosted, validateConnectServerUrl now rejects all private and reserved IPs (loopback, RFC1918, metadata, etc.) via isPrivateOrReservedIP, including after DNS resolution, while still returning the resolved IP for DNS pinning. On self-hosted, private/loopback Connect URLs remain allowed; only link-local (e.g. cloud metadata) stays blocked.

Logic is centralized in assertConnectIpAllowed; the validator is exported and covered by new Vitest cases for hosted vs self-hosted, literals, DNS, and resolution failures.

Reviewed by Cursor Bugbot for commit 243a06e. Configure here.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 31, 2026

Greptile Summary

This PR strengthens the SSRF protection for 1Password Connect-mode routes by gating the IP validation policy on deployment type. The refactoring extracts a new assertConnectIpAllowed helper and correctly exports validateConnectServerUrl for direct testing without networking mocks.

  • Hosted deployments now block all private/reserved IPs (loopback, RFC1918, link-local) via the existing isPrivateOrReservedIP utility, preventing tenants from reaching the platform's internal network.
  • Self-hosted deployments retain the original behaviour — only link-local (cloud-metadata) addresses are blocked, since Connect servers legitimately reside on private RFC1918 ranges controlled by the operator.
  • Policy enforcement covers both IP literals and DNS-resolved hostnames (preserving TOCTOU/rebinding protection), with 15 new unit tests covering all key combinations.

Confidence Score: 5/5

Safe to merge. The change is narrowly scoped to the 1Password Connect URL validator, the logic correctly branches on isHosted, and all IP edge cases are handled by the well-tested isPrivateOrReservedIP utility.

The refactoring is clean and well-tested. All IP edge cases (loopback, RFC1918, link-local, IPv4-mapped IPv6) are covered, and the old DNS catch-block re-throw hack has been correctly eliminated.

No files require special attention.

Important Files Changed

Filename Overview
apps/sim/app/api/tools/onepassword/utils.ts Extracts assertConnectIpAllowed from validateConnectServerUrl, gates hosted deployments on isPrivateOrReservedIP, and exports the validator for testability. Logic is correct for all IP forms.
apps/sim/app/api/tools/onepassword/utils.test.ts New test suite exercising hosted vs. self-hosted paths, IP literals, DNS-resolved addresses, IPv4-mapped IPv6, and IPv6 link-local. Uses real isPrivateOrReservedIP and a proper Vitest vi.hoisted flag for the isHosted toggle.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[validateConnectServerUrl] --> B{hostname is IP literal?}
    B -- Yes --> C[assertConnectIpAllowed\nip = literal]
    B -- No --> D[dns.lookup hostname]
    D -- error --> E[throw: hostname could not be resolved]
    D -- success --> F[assertConnectIpAllowed\nip = resolved address]
    C --> G{isHosted?}
    F --> G
    G -- true --> H{isPrivateOrReservedIP?}
    H -- true --> I[throw: private or reserved IP address]
    H -- false --> J[return IP for DNS pinning]
    G -- false --> K{range === linkLocal?}
    K -- true --> L[throw: link-local address]
    K -- false --> J
Loading

Reviews (2): Last reviewed commit: "test(security): use real isPrivateOrRese..." | Re-trigger Greptile

Comment thread apps/sim/app/api/tools/onepassword/utils.test.ts Outdated
Comment thread apps/sim/app/api/tools/onepassword/utils.test.ts
@waleedlatif1
Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1
Copy link
Copy Markdown
Collaborator Author

@cursor review

Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

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

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 243a06e. Configure here.

@waleedlatif1 waleedlatif1 merged commit 911586a into staging May 31, 2026
14 checks passed
@waleedlatif1 waleedlatif1 deleted the waleedlatif1/verify-code-changes-v1 branch May 31, 2026 01:13
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