Skip to content

external links: an allowed_urls wildcard that passes native check can deny every URL at runtime, and nothing reports it #431

Description

@sepehr-safari

An allowed_urls entry that looks like a wildcard, passes native check, and is accepted as well formed by the matcher itself can still match nothing, so every external link in the app silently stops opening. Nothing at runtime says a word.

The table

I ran both halves rather than reading them. Runtime column is upstream/main's own src/security/root.zig compiled verbatim and asked about three ordinary URLs (https://example.com, https://example.com/docs, https://github.com/damus-io/notedeck). CLI column is native check on a scratch copy of examples/capabilities with only allowed_urls varied.

allowed_urls entry native check URLs allowed
"https://*.example.com/*" valid, exit 0 0 of 3
"https://*/*" valid, exit 0 0 of 3
"https://*/" valid, exit 0 0 of 3
"https://*" rejected, exit 1 0 of 3
"https://example.com/*" valid 1 of 3 (denies the bare origin https://example.com)
"*" valid 3 of 3

The first row is the one that matters. "https://*.example.com/*" is the shape every other tool spells a subdomain wildcard with. externalWildcardPrefixValid (src/security/root.zig:68-80) accepts it as a well formed wildcard, and then allowsExternalUrl (:55-66) compares URLs against the literal prefix https://*.example.com/, which nothing can start with. So the pattern is valid, the policy is valid, and every link is denied.

Only two shapes actually work: "*", and a literal scheme://host/ prefix. There is no way to express "any host" or "any subdomain", and nothing anywhere says so.

Why it is hard to find

The refusal has no output. Runtime.openExternalUrl returns error.NavigationDenied (src/runtime/system_services.zig:579), and performNativeHostSend drops it on the floor at src/runtime/effects.zig:9533:

binding.open_external_url_fn(binding.context, payload) catch {};

hostSend returns void (:8736), so there is no channel to report on even if it wanted one. From the app's side a denied link and a working one are identical, and from the user's side the link just does nothing.

Worth noting the sibling that gets this right: src/runtime/builtin_bridge.zig:201 turns the same error.NavigationDenied into a real error response, so window.zero.os.openUrl reports while Cmd.openExternalUrl and fx.hostSend("native-sdk.os.openUrl", ...) do not.

I found this in my own app only by instrumenting a Linux build and printing at each step, because there was nothing else to go on. Links had simply stopped opening, everywhere, on every platform.

Reproduce

cp -r examples/capabilities /tmp/x && cd /tmp/x
# app.zon: set .allowed_urls = .{ "https://*.example.com/*" }
native check          # info[manifest.valid]: app.zon is valid

Then set the same value in the runtime options in src/main.zig and every Cmd.openExternalUrl does nothing, with no diagnostic. examples/capabilities is the only example in the repo that turns open_system_browser on, and it carries the policy in both app.zon and src/main.zig, so it is also the repo's own demonstration that this value is written twice and has to agree.

What I would suggest

Warn when a pattern cannot match, at startup. Non breaking, catches every row above with one rule, and fires whether or not anyone ever clicks a link. One predicate next to the matcher it describes, in src/security/root.zig:

/// True when `pattern` can match some URL. A pattern the matcher can never
/// hit is a typo far more often than it is a deliberate deny.
pub fn externalUrlPatternCanMatch(pattern: []const u8) bool {
    if (std.mem.eql(u8, pattern, "*")) return true;
    if (externalWildcardPrefixValid(pattern)) return std.mem.indexOfScalar(u8, pattern[0 .. pattern.len - 1], '*') == null;
    return std.mem.indexOfScalar(u8, pattern, '*') == null;
}

A * anywhere but the final byte means the only remaining branch is byte equality with the URL, so the pattern is dead.

The place to call it already exists. src/runtime/flow.zig:102 is try app_manifest.validateCommands(self.options.commands);, one line above :103 where the security policy is installed. The runtime already runs a manifest validator over its own options at that exact seam, and src/primitives/app_manifest/root.zig:115 already exports pub const validateSecurity, so this is a missing line rather than new architecture. A log there, alongside the existing one at :101, would have ended my hunt before it started.

I would add the same warning at effects.zig:9533, so a denial from a well formed policy (a host somebody forgot to list) is visible too.

What I am deliberately not asking for. Relaxing the pattern language to accept https://* or a subdomain wildcard. The scheme plus host plus / rule is implemented four times, in three languages: src/security/root.zig:68, src/primitives/app_manifest/validation.zig:685, src/platform/macos/appkit_host.m (NativeSdkWildcardPrefixHasPath, mirrored in cef_host.mm), and src/platform/linux/gtk_host.c (native_sdk_policy_wildcard_prefix_has_path, mirrored in webview2_host.cpp). Three of those gate WebView main frame navigation handoff, so loosening the rule widens a live security boundary. A warning costs nothing and changes no contract.

Two follow ons, entirely your call: tightening validateExternalUrlPattern to reject a * that is not the final byte, so native check catches these too; and stating the grammar in the security docs. The page says "list URL prefixes" and never uses the word wildcard, docs/public/schemas/app.schema.json types allowed_urls as a bare string array with no pattern, and native init scaffolds only action = "deny", so somebody turning the feature on has no correct example anywhere except that one example app. Searching the tree for https://* returns nothing, so the docs did not mislead me by example. They just never say which shapes work.

Versions

Runtime behaviour verified at v0.10.1 and at upstream/main; src/security/root.zig and src/runtime/effects.zig are byte identical at both. native check was run with CLI 0.9.2; validation.zig has moved since, so I read validateExternalWildcardPrefix at upstream/main and its rule is unchanged, which is what the CLI column rests on.

Related: #371 is open and already names the catch {} swallow and states the grammar in its body, so the grammar is known, just not written down anywhere a user reads. #425 was the same shape in markup (passes check and build, fails silently at runtime) and was fixed the day it was filed.

Activity

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

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions