Send attestation tokens on login-server requests - #736
Conversation
|
Current version of PR was reviewed by /review-bugbot with additional instructions on Aug 13, 21:44 PDT. It flagged 0 findings. Bugbot on commit |
|
|
||
| if (url.protocol === 'http:' || url.protocol === 'ws:') { | ||
| if (url.hostname === 'localhost') return | ||
| if (isPrivateHost(url.hostname)) return |
There was a problem hiding this comment.
Side effect on fake-world routing: makeFakeWorld with allowNetworkAccess: true routes fetches by "passes validateServer -> fakeFetch, throws -> real network" (src/core/fake/fake-world.ts). Private-IP URIs now pass, so a fake-world test pointed at a real LAN dev server (the exact use case this PR enables) silently gets answered by the in-memory fake server instead of the network. If that is not intended, the fake-world path may want to keep the old localhost-only check.
There was a problem hiding this comment.
Fixed in 552ba21, and in a better place than I suggested: fake-world no longer consults validateServer at all, so test routing stops tracking a production policy function. The explicit isFakeAccountInfrastructure allowlist is the right decoupling.
I checked the regex against the hosts this repo actually ships: login1/login2, info1/info2, sync-us1 through sync-us6, sync-eu (root.ts) and the fake server's own info-fake1, sync-fake1..3 all match.
Two intentional behavior changes worth naming, both consistent with the updated doc comment: under allowNetworkAccess, localhost and non-account *.edge.app hosts (the cors1..4.edge.app proxies, for instance) now reach the real network instead of the fake server.
| /** | ||
| * We only accept *.edge.app or localhost as valid domain names. | ||
| * We only accept *.edge.app, localhost, or (for http/ws only) private LAN IPv4. | ||
| * https/wss still require localhost or *.edge(test)?.app — private IPs are not |
There was a problem hiding this comment.
Nit: em-dash in the comment; repo convention is zero em-dashes in committed code (comma or semicolon instead). Full ruleset: https://github.com/EdgeApp/edge-dev-agents/blob/main/.cursor/skills/no-slop/SKILL.md
Add setAttestationToken and attach x-attestation-token so attested clients can skip CAPTCHA challenges from the login server. Validate private LAN hosts via real IPv4 octets (not DNS prefix matches), update the rejection message, and treat empty-string attestation tokens as cleared.
095bbe2 to
552ba21
Compare
2 participants
Summary
EdgeContext.setAttestationTokenand attachx-attestation-tokenon login-server/api/requests so attested clients can receive CAPTCHA rate overrides.http/wsserver overrides (still reject them forhttps/wss).Test plan
attestation-headertests: set / clear / empty-string token behaviorvalidateServertests: LANhttpaccepted; public IPs andhttpsLAN rejectedx-attestation-tokenonly when a token is setNote
Medium Risk
Touches login-server request headers and server URL validation used at context init; mistakes could weaken override rules or break local dev server configs.
Overview
Adds
EdgeContext.setAttestationTokenso apps can supply a device attestation JWT that is sent asx-attestation-tokenon subsequent login-server/api/requests (for CAPTCHA rate overrides). Empty or cleared tokens omit the header.validateServernow allows RFC1918 and loopback IPv4 hostnames forhttp/wsoverrides only;https/wssstill require localhost or*.edge(test)?.app, with tests for spoofed DNS names like10.evil.com.Fake-world
allowNetworkAccessno longer routes all non-Edge URLs through realio.fetchviavalidateServer; only login, info, and sync*.edge.apphosts stay on the in-memory fake server—change servers and private LAN use real fetch.Reviewed by Cursor Bugbot for commit 552ba21. Bugbot is set up for automated code reviews on this repo. Configure here.