fix(engine): reach model endpoints through fake-IP tunnels - #469
Conversation
Replaces the hand-rolled address blocklist with @microsoft/antissrf, whose policy is enforced inside the agent's DNS lookup and whose range table covers cloud metadata, Azure wireserver, AS112 and SRv6 that the local table missed. RFC-reserved ranges are re-permitted so a fake-IP tunnel's placeholder resolves through, and plaintext HTTP no longer carries a secret.
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
Important
The mechanism works — I extracted @microsoft/antissrf@1.0.0 from npm, ran the probe suite, and confirmed both gates (IP-literal and DNS-lookup) refuse loopback before a socket opens. But the test named for the address boundary proves the protocol boundary instead, so the rewrite's core claim is unverified in CI, and ExternalOnlyLatest quietly turns on a header the PR body doesn't mention.
Reviewed changes — the initial review of f382df6a, covering the swap from the hand-rolled BlockList pair to @microsoft/antissrf and the test rewrites that follow it.
- Enforcement moves into the agent —
resolvePublicEndpoint,requestModelListAtAddress, and the localwithAborthelper are deleted;requestPublicModelList(url, headers, signal?, agent?)is the sole transport entry and issues a normal hostname request, with the antissrf agent checking the address insidecreateConnection(IP literals) and a policy-wrappedlookup(names). - Range table outsourced —
PolicyConfigOptions.ExternalOnlyLatestsupplies 34 denied ranges;benchmarking,documentation, andfc00::/18are re-permitted so a fake-IP tunnel's placeholder addresses resolve through. - Plaintext HTTP now refused —
allowPlainTextHttpdefaults tofalsein antissrf, so a secret can no longer ride anhttp://probe. - Errors collapse to one string —
normalizeErrormaps anyAntiSSRFErrorto'Model detection only reaches public HTTPS endpoints'. - Tests — the address/scheme assertions are rewritten against
requestPublicModelListandPROBE_POLICY; transport tests inject a barenew Agent()to bypass the policy and drive a loopback server.
Things I checked and found sound, so they need no attention: _isNetworkConnectionAllowed evaluates the allow list before the deny list, so the re-permits genuinely override ExternalOnlyLatest; antissrf stores every rule IPv4-mapped in one IPv6 BlockList, so allowedAddresses.check(v4, 'ipv4') really does match, making the new assertion form valid; 1.0.0 was published 2026-05-29, so the release-age install policy needs no minimumReleaseAgeExclude entry; no stale references to the three removed exports remain; and all 8 catalog models.url constants are already https://, so the plaintext tightening cannot break a catalog or gateway submit path.
ℹ️ The plaintext tightening has no counterpart at the schema or form layer
allowPlainTextHttp: false is the right default, but nothing upstream tells a user their endpoint will be refused. AccountEndpointSchema.baseUrl is z.url() with no scheme constraint, and add-flow.tsx's CustomDraftSchema uses a bare z.string(), so an http:// custom endpoint stays savable and the refusal only surfaces when the user clicks fetch. That is a product decision rather than a bug — worth making deliberately rather than inheriting from a library default.
Technical details
# Plaintext HTTP endpoints stay configurable but can no longer be probed
## Affected sites
- `packages/foundation/schema/src/model/account.ts:38` — `baseUrl: z.url()` accepts `http://`.
- `apps/.../add-flow.tsx` `CustomDraftSchema` — `baseUrl` is `z.string()`, weaker still. The
`https://…` placeholder is cosmetic.
- `packages/host/engine/src/agent/model-probe.ts:47` — the refusal string the user eventually sees,
wrapped by `request-handler.ts` into `Model detection failed: …`.
## Required outcome
Decide, and make the codebase say which it is:
- If plaintext endpoints are unsupported, reject the scheme where the URL is entered so the user
learns at type time rather than at fetch time.
- If a self-hosted plaintext endpoint is a case worth supporting, that needs an explicit,
per-account opt-in rather than a global policy flag.
## Open questions for the human
- Are there existing accounts in the wild with `http://` base URLs? For custom accounts the probe is
not in the submit path, so those users keep a working account and lose only model detection —
which may be an acceptable silent degradation, or may warrant a migration notice.ℹ️ Nitpicks
- Two ranges the old hand-rolled table denied are absent from
recommendedLatest:::/96(IPv4-compatible IPv6) and::ffff:0:0:0/96(SIIT). I confirmedPROBE_POLICYnow permits::7f00:1, butconnect()to it isENETUNREACHon Linux, so I could not demonstrate any impact — flagging only because the PR body enumerates what the new table adds without noting what it drops.
Claude Opus ｜ 𝕏
| const port = (server.address() as AddressInfo).port; | ||
| try { | ||
| await expect( | ||
| requestPublicModelList(new URL(`http://127.0.0.1:${port}/models`), {}), |
There was a problem hiding this comment.
This test never reaches the address gate. AntiSSRFHttpAgent.addRequest runs the protocol check first, so an http:// URL is refused with Request headers or protocol disallowed by policy before createConnection is ever called — received === 0 and REFUSAL_PATTERN both hold purely because of allowPlainTextHttp. I proved it: adding 127.0.0.0/8, 169.254.0.0/16, and 192.168.0.0/16 to addAllowedAddresses leaves all 15 tests green. Switching to https:// fixes it — verified failing under that neutering and passing against the real policy.
Technical details
# The address deny-list this PR introduces has no behavioral test
## Affected sites
- `packages/host/engine/src/__tests__/model-probe.test.ts:128-148` — `it('never connects to a
loopback endpoint')`. `addRequest` → `_isHttpRequestAllowed` returns false on `http:` +
`allowPlainTextHttp === false`, emits `AntiSSRFError` on `process.nextTick`, and returns without
calling `super.addRequest`. `createConnection` and the policy `lookup` never run.
- `packages/host/engine/src/__tests__/model-probe.test.ts:150-161` — asserts `PROBE_POLICY`'s
`allowedAddresses` / `deniedAddresses` `BlockList`s. A config predicate, not the agent's decision.
- `packages/host/engine/src/__tests__/model-probe.test.ts:163-165` — asserts the
`allowPlainTextHttp` field, again config not behavior.
- Every remaining transport test passes `unguarded()`, so the guarded agents are exercised by
nothing.
Net effect: the diff swaps out the entire enforcement mechanism and no test drives it.
## Required outcome
A test that fails if the address policy stops refusing a denied address, covering both gates:
the IP-literal gate in `createConnection` and the policy `lookup` (the gate the doc comment's
"a rebind between check and connect has no gap to land in" claim rests on).
## Suggested approach
The one-character version of this — `http://` → `https://` on this line — already covers the
IP-literal gate. I ran it: it fails with `write EPROTO … wrong version number` once loopback is
allow-listed (the request connects), and passes against the real policy with `received === 0`.
Adding a `https://localhost:${port}` case covers the DNS gate, since `_lookupAll` refuses when
*any* returned address is denied.
Worth pinning separately: the fake-IP fix depends on antissrf evaluating the allow list *before*
the deny list. That happens in `_isNetworkConnectionAllowed`, which is marked `@internal`, and the
dependency range is `^1.0.0`. If a minor release flipped to deny-first, `198.18.x` would be refused
again and every current test would stay green. A request to an allow-listed-but-unreachable literal
(e.g. `https://198.18.16.15:1/models`) asserting the rejection does *not* match `/public HTTPS/`
would catch that.| requestPublicModelList(new URL(`http://127.0.0.1:${port}/models`), {}), | |
| requestPublicModelList(new URL(`https://127.0.0.1:${port}/models`), {}), |
| /** Both refusal kinds the policy raises — a denied address, and plaintext HTTP carrying a secret. */ | ||
| const POLICY_REFUSAL = 'Model detection only reaches public HTTPS endpoints'; | ||
|
|
||
| export const PROBE_POLICY = new AntiSSRFPolicy(PolicyConfigOptions.ExternalOnlyLatest); |
There was a problem hiding this comment.
ExternalOnlyLatest also sets addXFFHeader = true, so every probe now sends X-Forwarded-For: true to the user's endpoint — I confirmed it on the wire. That value isn't a valid XFF (the header takes an IP list), and it buys nothing here: antissrf adds it to defeat IMDS, but 169.254.169.254 and 168.63.129.16 are already denied by address and none of the re-permitted ranges host a metadata service. Worth setting PROBE_POLICY.addXFFHeader = false unless it's wanted.
Technical details
# `ExternalOnlyLatest` injects an invalid `X-Forwarded-For` into every probe
## Affected sites
- `packages/host/engine/src/agent/model-probe.ts:49` — the `PolicyConfigOptions.ExternalOnlyLatest`
constructor arm runs `this._addXFFHeader = true` (alongside
`addDeniedAddresses(IPAddressRanges.recommendedLatest)`).
- Enforced in `AntiSSRFHttpAgent.addRequest` → `_isHttpRequestAllowed`, which calls
`req.setHeader("X-Forwarded-For", "true")` when the header is absent.
Observed server-side for a probe request:
`{"accept":"application/json","x-api-key":"…","host":"…","x-forwarded-for":"true","connection":"close"}`
This reaches every configured model endpoint — vendor APIs, third-party gateways, self-hosted
relays. Note the diff deliberately drops the manual `host` header override while silently gaining
this one.
## Required outcome
The outgoing header set is a deliberate choice, not a side effect of the preset. Either suppress it
(`addXFFHeader` is a public setter, so one line after construction) or keep it with a comment saying
why the probe wants to be seen as proxied.
## Open questions for the human
- Any endpoint doing client-IP-based rate limiting, geo-routing, or strict header validation could
read or reject a malformed XFF. `gateway.linkcode.ai` tolerates it per the PR's verification
table, but that's one endpoint out of the open set a user can configure.
Symptom
Adding a LinkCode Gateway account failed with
Model detection failed: Model detection cannot access private or non-routable addresses, while the endpoint itself was perfectly reachable.Cause
resolvePublicEndpointrefused any hostname resolving into198.18.0.0/15. On a machine running a TUN proxy in fake-IP mode that is every hostname: the resolver hands out a placeholder from RFC 2544's benchmarking range (Clash/mihomo/sing-box/Surge all mint from it) and the tunnel routes by that placeholder. The probe's SSRF blocklist and fake-IP addressing collide by construction.Verified locally: system resolver
198.18.0.2,gateway.linkcode.ai→198.18.16.15, real address via1.1.1.1→172.66.158.8. Connecting to the placeholder with SNI set to the real name returns the gateway's own401, TLSauthorized: true.Change
The hand-rolled
BlockListpair is replaced by@microsoft/antissrf(MIT, no runtime deps):resolvePublicEndpoint, the manual pin, and the manualhost/servernamejuggling are deleted; the request is a normal hostname request again.168.63.129.16/32), AS112, 6to4 relay, SRv6,3fff::/20, and a wider Teredo range.fc00::/18). No routable host may live there, so a name resolving into one can only be a local resolver's placeholder. Denying that class buys no protection and strands every user behind a tunnel. Real ULA (fd00::/8— Tailscale, Docker) stays denied.allowPlainTextHttpdefaults tofalse, which closes a P1 raised in feat(desktop,agent-adapter): API-key / relay login on the signed-out agent card #350's review that was never implemented.Verification
Live, through a fake-IP TUN, driving the real
probeEndpointModels:https://gateway.linkcode.ai/v1/models401 invalid_api_key— reaches the endpointhttp://127.0.0.1:19533/v1/modelshttp://169.254.169.254/latest/meta-data/http://192.168.1.1/modelshttp://gateway.linkcode.ai/v1/modelsAlso: 20 probe tests pass,
pnpm check:ciclean, and the tsup-bundled daemon boots fromdisttoDaemon listener bound— the policy is constructed at import time, so only a real boot proves the CJS dependency survives bundling.pnpm testhas 3 pre-existing failures inrelease-artifact.test.ts, present onmasterwithout this change.Not addressed
createKeystill runs before the probe inadd-flow.tsx, so a failed probe leaves an orphan gateway key.