Skip to content

fix(siwe): isUri rejects authority-only URIs with an empty path - #434

Merged
jxom merged 2 commits into
wevm:mainfrom
gomesalexandre:fix_siwe_isuri_pathless
Sep 1, 2026
Merged

fix(siwe): isUri rejects authority-only URIs with an empty path#434
jxom merged 2 commits into
wevm:mainfrom
gomesalexandre:fix_siwe_isuri_pathless

Conversation

@gomesalexandre

Copy link
Copy Markdown
Contributor

Siwe.isUri rejects every authority-only URI - https://login.xyz, the reference example from EIP-4361 itself, cannot be used as a SIWE uri.

The bug

src/core/Siwe.ts:333:

// scheme and path are required, though the path can be empty
if (!(scheme?.length && path && path.length >= 0)) return false

path is a plain string; for an authority-only URI (no path segment) splitUri yields '', which is falsy, so path && short-circuits before path.length >= 0 - the clause whose entire purpose is to permit an empty path - ever runs. The comment on the line above states the opposite of what the code does.

Second tell: two lines down, if (!(path.length === 0 || path.startsWith('/'))) return false was already provably dead code - path.length === 0 could never be reached past line 333's bug.

Third: viem's identical sibling function (utils/siwe/utils.ts) never had this guard:

if (!(scheme?.length && path.length >= 0)) return false

RFC 3986

An authority-only URI legitimately has an empty path component (URI = scheme ":" hier-part ..., hier-part = "//" authority path-abempty, where path-abempty explicitly allows zero segments - RFC 3986 §3). https://example.com is a valid, common URI.

Repro (real run against current main)

=== Siwe.isUri (BEFORE fix) ===
https://example.com          false
https://login.xyz            false
https://example.com:8080     false
https://example.com?a=1      false
https://example.com/foo      https://example.com/foo
ipfs://bafybeiabc            false

=== Siwe.createMessage with pathless uri ===
THREW: InvalidMessageFieldError - Invalid Sign-In with Ethereum message field "uri".

=== Siwe.createMessage with pathless resources entry ===
THREW: InvalidMessageFieldError - Invalid Sign-In with Ethereum message field "resources".

Also confirmed live in the currently published ox@1.7.4 npm package, both src/core/Siwe.ts and the compiled dist/core/Siwe.js - this isn't a working-tree-only bug.

Fix

Drop the redundant path &&, matching viem exactly:

if (!(scheme?.length && path.length >= 0)) return false

Safe by construction: splitUri's path capture group ([^?#]*) is a mandatory (non-optional) regex group, so path is always at least '', never undefined - confirmed both by reading the regex and by Codex's independent review (see below).

Bonus catch: an existing test was unknowingly pinning the bug

behavior: invalid resources passed resources: ['https://example.com', 'foo'] expecting 'foo' to be flagged, but the old buggy code rejected 'https://example.com' first (a legitimate bare-origin URI) and the pinned snapshot had silently encoded that wrong behavior as "expected". Fixed code correctly reports foo as the invalid entry. Updated the snapshot accordingly - flagging explicitly so it doesn't read as unexplained test churn.

Testing

Real command output, 82/82 passing:

$ vp test run src/core/_test/Siwe.test.ts
 Test Files  1 passed (1)
      Tests  82 passed (82)

Confirmed genuine red-before/green-after: reverted only Siwe.ts (kept the new tests), re-ran - 8 failed | 74 passed (82), exactly the 7 new tests plus the corrected pinned snapshot. Restored the fix - back to 82/82.

Full core project suite: 189/193 files pass; the 4 failing files (AbiConstructor, AbiError, Log, Provider, RpcResponse, RpcTransport, TransactionEnvelope*, TransactionRequest - all named behavior: network) are live-RPC-dependent tests unrelated to Siwe, hitting oxlib.sh/live infra 400s - none touch Siwe.ts or Siwe.test.ts.

tsc --noEmit -p .: clean. vp check: 0 errors (2 pre-existing unrelated warnings in site/src/components/Landing.tsx).

New test cases added to the isUri and createMessage describe blocks: bare origin, with port, with query, with fragment, non-http scheme (all authority + empty path), plus createMessage with a pathless uri and a pathless resources entry.

Codex adversarial review

Ran synchronously, verdict SHIP, no blocking findings. Independently confirmed: RFC 3986 correctness, that path cannot be undefined/null given the mandatory regex capture, and that the corrected pinned snapshot is right. One non-blocking style note (the path.length >= 0 check is tautological given the invariant) - kept as-is to match viem's exact wording for cross-repo consistency, which Codex agreed was a reasonable call.

Scope

This fails closed (rejects/throws) - a functional/availability bug, not an authentication bypass or security issue.

Siwe.isUri required scheme?.length && path && path.length >= 0 - the
`path &&` guard short-circuited before the tautological length check
ever ran, so any URI with an empty path (e.g. a bare origin like
https://example.com) was rejected. The comment directly above states
the opposite intent: "scheme and path are required, though the path
can be empty".

Per RFC 3986 an authority-only URI legitimately has an empty path
component, and this is also the single most common SIWE uri value -
Siwe.createMessage threw for https://login.xyz, the EIP-4361
reference example, and for any pathless entry in resources.

viem's identical sibling function (utils/siwe/utils.ts) never had
this guard and has always accepted these URIs correctly.

Fix: drop the redundant `path &&`, matching viem exactly. path can
never be undefined here - splitUri's third capture group is a
mandatory (non-optional) regex group, always at least an empty
string.

Also corrects an existing pinned test ('behavior: invalid resources')
that was unknowingly asserting the bug: with resources set to
['https://example.com', 'foo'], the old code rejected the FIRST entry
(a legitimate bare-origin URI) instead of the actually-invalid second
one. Fixed code correctly flags 'foo'.
@vercel

vercel Bot commented Sep 1, 2026

Copy link
Copy Markdown

@gomesalexandre is attempting to deploy a commit to the Wevm Team on Vercel.

A member of the Team first needs to authorize it.

@gomesalexandre
gomesalexandre marked this pull request as ready for review September 1, 2026 14:44
@pkg-pr-new

pkg-pr-new Bot commented Sep 1, 2026

Copy link
Copy Markdown

Open in StackBlitz

npm i https://pkg.pr.new/ox@434

commit: 4f14061

@jxom
jxom merged commit 633388e into wevm:main Sep 1, 2026
11 of 13 checks passed
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