fix(contract): reject control characters in the scan target remote

[rohanpoudel2]

Fixes #231

Problem

validateCanonicalContract validated scan.target.remote against a string that was not the string it stored.

The authority is extracted with:

const authority = /^[A-Za-z][A-Za-z0-9+.-]*:\/\/([^/?#]+)/.exec(remote)?.[1];

[^/?#] matches ASCII tab, LF and CR. The follow-up new URL(remote) check does not catch them either, because the WHATWG URL parser strips exactly those three characters from its input before parsing. So a manifest carrying:

"remote": "https://example.com\nhttps://evil.example.net"

loaded successfully — new URL() saw host example.comhttps, while loadContract handed the original two-line string straight back to callers.

Neither surrounding guard closes it: the schema pattern ^(?![^:/?#]+://[^/?#]*@)[^?#]+$ also matches newlines, and validateParsedJson checks only for well-formed Unicode.

Change

Reject C0/C1 control characters and Unicode line separators in remote before the regex and URL checks run, mirroring the class requireModelSafeOutputDir already applies in runtime.ts for the same reason. That constant is not exported, so it is mirrored locally with a comment naming the source of truth rather than widening runtime.ts's public surface for one caller.

Why not also require href === remote

I tested this and rejected it. The WHATWG serializer legitimately rewrites valid URLs — it strips a default :443, lowercases mixed-case hostnames, and punycode-encodes internationalized ones — so exact canonical equality would reject real remotes. The bundled producer (finalize_scan_contract.py) validates remote only via urlsplit scheme and netloc checks and imposes no canonical-equality requirement, so it can legitimately emit any of those forms.

That trade would only be worth it if the URL parser smuggled a wider set of characters. It does not. Enumerating every C0/C1 code point through new URL("https://exa<c>mple.com"):

SURVIVED new URL(): 09->example.com 0a->example.com 0d->example.com
threw count: 62

Only tab, LF and CR slip through; the other 62 already make new URL() throw. The narrow control-character rejection is therefore sufficient, and the broader C0/C1 class is defence in depth consistent with runtime.ts.

Impact, stated plainly

This closes a broken guarantee rather than a live exploit. Nothing under src/ consumes .remote today, and credential smuggling remains correctly blocked — any @ in the authority is still rejected before URL parsing, and that check is untouched. The value is that the contract layer is precisely where an untrusted manifest field is meant to be sanitized, and SDK consumers that render, log or re-emit manifest.scan.target.remote were inheriting a multi-line value that had passed validation.

Verification

Three rejection cases (\n, \r, \t) were added to the existing table-driven "rejects schema-valid but canonically invalid contract data" test, plus a standalone test proving an ordinary https://github.com/example/repo remote still loads and round-trips unchanged — so the fix cannot pass by rejecting everything.

Against the unfixed source the new case fails with Expected promise that rejects / Received promise that resolved; with the fix, contract.test.ts is 30 pass / 0 fail.

Full suite: 718 pass / 5 skip / 0 fail (717 baseline plus the new test). pnpm run types and pnpm run format are clean.