fix: add http request manager for CLI apps

[tx3stn]

Description

This is somewhere between a fix and a new feature as it's addressing something broken at the the moment but is adding new functionality and a new dependency.

Currently if you build a CLI app that uses a Valdi HTTP client every request fails with No RequestManager set.
I've added apps/cli_http_example to demonstrate this. If you build and run it now you will see the error, and build and run with these changes to see it working.

My use case is that I've made a music player app with Valdi which works great on Android and iOS, and I now want to create a headless CLI that can be controller via the app, so I want to re-use some of the existing code from the apps around playback & auth etc.

So this add a request manager for cli apps using libcurl.

Note, it's on version 8.12.0 because that bumps nothing, 8.21.0 would pull boringssl forward about a year and needs a rules_cc patch bump, which seems better done on its own.

I've gated the http client behind enable_http, a new attribute on valdi_cli_application() that defaults to off, because:

1. explictly opting in to networking seems like a sensible thing to do for security
2. it keeps the curl deps out of a CLI that doesn't need them

valdiCLIRun takes the request manager as a fourth parameter to carry this, null for a binary built without one. It is deliberately not defaulted, so each cli_main template has to say which it is.

Two promises that never settled

1. A failed request never rejected. HTTPRequestManagerModuleFactory.cpp handed the failure to JavaScript as Value(result.error()). valueToJSValue raises a ValueType::Error into the exception tracker instead of marshalling it as an argument (JavaScriptUtils.cpp:337), so the completion never ran and the promise behind it stayed pending.
   It now passes result.error().toString(), which is what PersistentStoreModuleFactory already does, and HTTPClient wraps that text back into an Error so a rejection is always an Error whichever way the request failed.
2. A cancelled request never rejected either. No request manager reports a cancellation, on any platform, so HTTPClient.cancelableRequest cancelled the native request and left its promise pending. Its cancel function now also rejects, with Request was cancelled.
   Rejecting an already settled promise is a no-op, so a late cancel changes nothing.
   Anything currently relying on a cancelled request never settling would start seeing a rejection.

Both are changes iOS and Android get as well, so they want a look even though they are small.

One pre-existing issue found, not fixed here

Value(Error) still converts asymmetrically. The ObjC conversion turns it into a value (SCValdiError), the JS one raises instead.
The fix above is local to the HTTP factory, so the trap is still there for the next module factory to hit.

SwiftValdiMarshaller.cpp:398 takes its callback from an arbitrary marshaller stack slot, so it could carry a JS-backed function and fail the same way.
I couldn't exercise the Swift bridge to confirm. A real fix would belong in the JS conversion, not in Marshaller — stringifying there would break the ObjC contract, which relies on SCValdiError.

TLS

Certificate verification is left on — nothing here touches CURLOPT_SSL_VERIFYPEER or VERIFYHOST.
The minimum protocol version is pinned to TLS 1.2, because curl's own floor is 1.0 while iOS and Android both refuse anything below 1.2.

Which CAs are trusted is decided in CaStore.cpp, in this order:

1. A bundle passed to makeCurlHTTPRequestManager, so an embedder can be explicit.
2. Otherwise the environment: CURL_CA_BUNDLE or SSL_CERT_FILE for a bundle, CURL_CA_PATH or SSL_CERT_DIR for a hashed directory. libcurl reads none of these itself, which is why they are read here.
3. Only if those are empty and the store curl compiled in is not on this machine, the usual distribution paths (/etc/ssl/certs/ca-certificates.crt and three others, plus /etc/ssl/certs and /etc/pki/tls/certs). Probed last, so a deliberate --@curl//:ca_bundle is never overridden.

BoringSSL is the TLS backend on macOS as well as Linux.
@curl compiles SecureTransport in and links the Security framework, but USE_OPENSSL wins the #elif chain in vtls.c and CURL_WITH_MULTI_SSL is not defined, so the keychain is never consulted.

If none of the three steps finds anything, no CAINFO or CAPATH is set and verification runs against an empty store, so every HTTPS request fails. It fails closed, not open.

User-Agent

libcurl sends none unless told to, and a request without one is refused outright by a fair number of CDNs, so it sends curl/<version>, taken from curl_version_info so a curl bump carries itself.

NeitherSCValdiDefaultHTTPRequestManager nor DefaultHTTPRequestManager sets a User-Agent, so iOS sends
CFNetwork's and Android sends the platform's. Valdi never names itself on either, and does not here.
An app that wants to name itself sets the header, which replaces this.

Redirects

curl follows them, capped at 10 hops, so an endless chain fails rather than spinning. Beyond that:

• Protocols are pinned to http,https, which curl checks on the first request and again on every redirect target, so a Location pointing at file:// is refused.
• Authorization and Cookie are withheld from a redirect to another origin, and kept within the same one.
• Method and body follow RFC 9110 15.4: a 303 becomes a GET and drops the body, a PUT body survives a 301, a POST body survives a 307.

One known difference from iOS and Android: a method curl does not model itself goes out through CURLOPT_CUSTOMREQUEST, which curl keeps for the whole chain, so a custom verb survives a 303 rather
than becoming GET. Its body is still dropped.

Header injection: fixed here, but the validation probably belongs upstream

Found while writing request-header tests, curl writes custom headers and a custom request line out verbatim and does no validation of its own, which it documents as the caller's job.
So a JavaScript header value containing \r\n injected an extra header, and one containing \r\n\r\n wrote a complete second request onto the same connection. CR/LF in the method split the request line. An embedded NUL truncated the value silently, since curl_slist_append takes a const char*.

CurlHTTPRequestManager now rejects the request when a header name, header value or the method contains CR, LF or NUL, rather than stripping — a request should never quietly mean something other than what was asked for, which is what fetch() does on the web too: an invalid header value is a TypeError, while a forbidden header name is dropped silently, and that second rule is the one the compression section below leans on.
The URL needs no check; curl parses that itself and rejects it, and there is a test pinning that.

The same validation arguably belongs in HTTPRequestManagerModuleFactory.cpp so that every platform
gets it rather than just the CLI. I have left that alone to keep this PR out of the shared layer beyond the two fixes above, but worth a look: web fetch() rejects these per spec, so iOS and Android are the open question.

Known limitation: no response compression

@curl is built without zlib — no HAVE_LIBZ in its copts, and its MODULE.bazel declares no zlib dep — so this manager neither requests nor decodes compressed responses.

• Responses arrive uncompressed, so transfers are larger than on iOS and Android.
• CURLOPT_ACCEPT_ENCODING, "" is not a fix. With only identity compiled in it would send
  Accept-Encoding: identity and transfer exactly the same bytes.

Enabling it needs a single_version_override patch against curl's own BUILD file to add
-DHAVE_LIBZ=1 and a @zlib dep, rechecked on every curl bump. That seemed the wrong trade for a
CLI, so it is left out (for now - can be looked at again separately if required).

What is handled:

• A caller-supplied Accept-Encoding is dropped rather than sent, so nothing can ask for an encoding this build has no codec for. Content-Length goes the same way, since curl frames the body itself. fetch() forbids both names, so the same JavaScript already loses them on web.
• A response carrying a Content-Encoding anyway — an origin that compresses unasked — fails with an error naming the encoding, rather than handing JavaScript bytes it has no way to read.

Note

This includes a timeout bump to the linux C++ tests. They were running with a medium timeout of 300s, which before these changes was taking 296s. So the new tests here nudge it over that timeout.

Type of Change

• Bug fix (non-breaking change that fixes an issue)
• Documentation improvement
• Performance optimization
• Test improvement
• Other (please describe)

Testing

• Tests pass locally (bazel test //...)
• Added/updated tests for changes (if applicable)
• Tested on multiple platforms (iOS/Android/Web/macOS as applicable)
• Manual testing performed (describe below)

Testing Details

Checklist

• Code follows project style guidelines
• Documentation updated (if needed)
• No breaking changes (or documented in description)
• Commit messages follow conventional format
• No secrets, API keys, or internal URLs included

Related Issues

Additional Context

[github-actions]

Sensitive Files Detected

📦 Dependency change — Modifies the Bazel module graph — needs runtime team review after import.

🔧 Build rules — Affects build rules for all Valdi consumers.

_{This is an automated notice. A maintainer will review after import.}

[github-actions]

⚠️ Bazel & CI Test Results

Test Suite	Result
macOS: C++ & Platform Tests	❌ failure
valdi_web Integration Test	✅ success
Linux: Standalone Tests	✅ success
Test Coverage Delta	✅ success
API Surface Check	✅ success
Linux: Hotreload Smoke	✅ success
Linux: C++ Tests	✅ success
Linux: Build & Export	✅ success
Valdi Smoke Tests	✅ success
Snapshot Tests	✅ success
Linux: Build Compiler	✅ success
Linux: Module Tests	✅ success
Linux: Registry Validation	✅ success

Some tests failed. Please check the workflow logs for details.

🚀 Bazel remote cache is now enabled - future builds will be faster!

_{Workflow: Valdi CI}

[clholgat]

Thanks for this — it's an exceptionally thorough PR. The write-up did a lot of the review for us, and the test suite is genuinely impressive: the redirect matrix, the header-injection cases, and the cancel/teardown coverage especially.

Before this moves, I want to put our read of the three changes that reach past the CLI into the shared HTTP layer on the record, since those affect iOS, Android and web too. We agree with all three and think they're safe to take — here's the reasoning:

1. Failed requests now reject instead of hanging (HTTPRequestManagerModuleFactory.cpp)
Your root-cause diagnosis checks out: handing the error across as a Value holding an Error makes the marshaller raise it into the exception tracker rather than pass it as the callback's argument, so the completion never ran and the promise stayed pending. The string fixes it on every platform. It's strictly more correct — the only observable change is that a transport-level failure (offline/DNS/TLS/timeout) that used to hang now settles as a rejection. We're comfortable taking it: our own use of this path is limited and we have a small follow-up on our side to handle the newly-real rejection. The engine-parametrized tests you added (Hermes/QuickJS/JSCore) cover it well.

2. Rejection reason normalized to Error (HTTPClient)
Wrapping non-Error reasons so a caught rejection is always an Error is a good consistency win and pairs naturally with (1). Low risk.

3. Cancelling now rejects with Request was cancelled (HTTPClient)
Since no manager reports a cancellation, the promise previously stayed pending on cancel — which, as you note, hangs a CLI holding the runtime open. Settling it is the right call, and rejecting an already-settled promise is a no-op, so a late cancel is harmless. The consumers of this on our side all live in the open-source tree, so the blast radius is contained and the new behavior is the more correct one.

One heads-up on timing: the native curl manager deserves a careful pass on its threading/teardown model and its TLS/trust-store handling, so this one will take us a while to review properly.

Thanks again — this is a great contribution, and we'd rather get it right than fast.

[li-feng-sc]

Change looks good to me.

The only ask I have to the author is if we could move the two curl test files into their own valdi_test target (say test_standalone_http) instead of the integration glob, so test_integration and test can stay curl-free.

The reason for this ask is that currently test_integration and test is part of Snap's CI, and our internal build does not directly consume the open source MODULE.bazel so they will not pick up the curl module. Moving the curl dep out of the CI targets will allow Snap to import this PR without breaking our CI.

[tx3stn]

Change looks good to me.

The only ask I have to the author is if we could move the two curl test files into their own valdi_test target (say test_standalone_http) instead of the integration glob, so test_integration and test can stay curl-free.

The reason for this ask is that currently test_integration and test is part of Snap's CI, and our internal build does not directly consume the open source MODULE.bazel so they will not pick up the curl module. Moving the curl dep out of the CI targets will allow Snap to import this PR without breaking our CI.

Sure thing.
Moved them into their own test target. This means they wouldn't currently run in CI here, so I also added CI targets to run them, but added that as it's own separate commit so it can easily be ignored/removed if that part isn't wanted.