Skip to content

feat(http-transport)!: make Warp bind host and port configurable - #909

Draft
stepandra wants to merge 5 commits into
neohaskell:mainfrom
stepandra:misc/server-bind-host-port
Draft

stepandra wants to merge 5 commits into
neohaskell:mainfrom
stepandra:misc/server-bind-host-port

Conversation

@stepandra

@stepandra stepandra commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

Apps can choose their web server's address and port with Application.withHost and Application.withPort. A loopback bind such as 127.0.0.1 is suitable for a reverse proxy that can reach the application's network namespace. A proxy in another namespace generally cannot reach that listener through its own loopback.

Without overrides, the default stays *4 (all IPv4 interfaces) on port 8080. A transport's existing custom host or port is preserved when the corresponding application override is absent. IPv6 remains opt-in.

Review follow-up

  • Leading/trailing spaces, tabs and newlines now fail host validation with a whitespace error instead of reaching Warp. New tests distinguish valid bare hosts from padded ones.
  • Earlier review coverage remains: builder ordering; absent, host-only and port-only preservation; input/port validation; visible startup failures; an instance-specific health path; loopback exclusion and a wildcard negative control.
  • C7 lists the exact new whitespace selectors. C8's pre-socket validation tests are unit / none; C9's occupied-port test is integration / http:real. C5/C9 attestations use the actual printed BindSpec fixture group.
  • Default/custom-port descriptions and network-namespace guidance agree across the API docs, specification, ADR and release note.

Verification

  • ./dev test "Service.Transport.Web.Bind" nhcore-test-service: 29 examples, 0 failures.
  • Exact runtime probes and report-mode validation: 3 integration locators pass.
  • Lint, spec/criterion/signature checks, codemap, ADR index, release fragments, expectation guard and whitespace checks pass.

The full PostgreSQL/Hurl suite was not rerun. Still a draft; no final maintainer approval is claimed. The supported builder API needs no migration. The release note records the added fields for code constructing the internal records directly.

Spec 019 · ADR-0079.

Add WebTransport.host (default "*", unchanged behaviour), start Warp via runSettings with setHost/setPort, and add Application.withHost/withPort. Adds ADR-0079, spec 019, release fragment and BindSpec.
…rp.run behaviour

A "*" default would silently start listening on IPv6 on dual-stack hosts whose
firewall rules only cover IPv4 — a new reachable surface that does not belong in
a make-it-configurable change. IPv6 ("*") and loopback ("127.0.0.1") are both
opt-in through Application.withHost. ADR-0079, change 019, the release fragment
and BindSpec updated accordingly.

Gates: ./dev test 'Service.Transport.Web.Bind' nhcore-test-service 12/0; ./dev spec-check 019 OK; ./dev lint No hints; ./dev check Up to date; git diff --check 0.
@coderabbitai

coderabbitai Bot commented Oct 6, 2026

Copy link
Copy Markdown
Contributor

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

  • Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@stepandra stepandra changed the title feat(http-transport)make Warp bind host and port configurable feat(http-transport)!: make Warp bind host and port configurable Oct 6, 2026
@NickSeagull

Copy link
Copy Markdown
Member

Preliminary automated checks — maintainer review pending

Reviewed 0ca76f299e5ff309577d196c5fc56045f749d503 against cab923c0b098fdb82ece539380719c0757ec9057. These are bounded, non-exhaustive checks, not approval or a final maintainer verdict.

  1. Align the remaining default/custom-port descriptions with the declared contract. ADR-0079's decision keeps "*4", but its last consequence says the default now accepts IPv6. The withHost Haddock still calls the absent-override default "*". Also, User impact includes server { port = ... } among apps that keep port 8080, whereas the implementation preserves that custom port when no application override is supplied. Please reconcile these descriptions; this does not ask for a different default.

  2. Repair C1's exact test locator. C1 names server binds all interfaces by default, but the actual example is server binds all IPv4 interfaces by default (unchanged from Warp.run). Running ./dev spec-check --criteria-tests cab923c0b098fdb82ece539380719c0757ec9057 fails with “C1 hspec locator selects zero source examples”. Please update the locator to the exact example name and align C1's wording.

  3. Classify the existing HTTP probe at its actual boundary. C5 says unit | none, but its test starts an application and makes a real HTTP request. Please use integration | http:real and add the matching fixture attestation in docs/changes/test-surfaces.json, following the existing exact-attestation contract. This is a classification repair for the test already present, not a request for another test framework.

  4. Cover the override/preservation behavior already promised. The ADR promises order-independent application overrides. The existing builder tests check stored fields, while the running example uses only withTransport before the builders. Please add focused coverage showing both orders give the configured bind settings, and that omitting the overrides preserves a transport's custom settings. Those cases would catch accidentally discarding settings during composition; they do not require exhaustive combinations.

Local receipts: structural spec validation, signature drift, ADR index, codemap validation, release-fragment validation, expectation guard and whitespace check passed. Exact criterion resolution failed as above. Compilation, the Haskell test run and local HLint could not execute here because the toolchain is absent; the PR author's runtime results have not been independently reproduced.

…concile default-host wording, exact C1 locator, C5 as http:real with attestation, order-independent override coverage

- ADR-0079 consequence and withHost Haddock now say the default is "*4" (unchanged from Warp.run); change 019 user impact says server { port = ... } keeps its port.
- C1 locator is the exact example name; ./dev spec-check --criteria-tests cab923c resolves.
- C5 reclassified integration | http:real with an exact attestation in docs/changes/test-surfaces.json.
- New pure Web.applyBindOverrides (used by runWebTransport) + C6: overrides apply before or after withTransport; absent overrides preserve a transport's custom host/port.
- BindSpec 12 -> 14 examples, 0 failures; lint no hints; codemap regenerated.
@NickSeagull

Copy link
Copy Markdown
Member

Maintainer-reviewed follow-up — specification corrections

Nick reviewed and authorized these additional requests for 0ca76f299e5ff309577d196c5fc56045f749d503. They supplement the initial feedback; this is not final PR approval.

1. Input and failure behavior is underspecified

  • Problem / principle: Predictable configuration needs representative edge/LLM-misuse coverage and early, actionable failures; accepting Int/Text is fine, but the observable contract needs definition.
  • Suggestion: Specify compatible host forms and port range/zero behavior; cover representative mistakes such as a URL placed in the host field. Define earliest-practical invalid-setting checks, visible bind failures and cleanup of resources owned by this operation. Preserve supported behavior; this is not a request for wrapper types or a broad lifecycle rewrite.
  • Evidence: The builder stores the setting; that alone does not establish downstream validation:
withPort port app =
  app {bindPort = Just port}

2. Health reachability does not establish loopback isolation or instance identity

  • Problem / principle: Evidence must distinguish the promised restriction and observe the intended application. The current probe only establishes a successful response:
status <- getHealthStatus freePort 50
status |> shouldBe 200
  • Suggestion: Specify the loopback exclusion guarantee and provide bounded evidence that rejects wildcard binding and a response from a different process when the intended application fails. Choose a proportionate fixture technique; no particular networking framework is required. These are source-reasoned false-pass cases, not claims of an executed exploit or race.

3. Make single-setting preservation coverage explicit

  • Problem / principle: Supported custom settings must survive independent builder overrides. This clarifies the initial composition-coverage request, rather than alleging a source regression.
  • Suggestion: Include host-only/custom-port and port-only/custom-host cases alongside ordering and absent-override cases; exhaustive permutations are unnecessary.
  • Evidence: Current assembly preserves the other field; tests should discriminate accidental reset-to-default behavior.

4. Qualify reverse-proxy guidance by network scope

  • Problem / principle: Jess-first deployment guidance should not recommend a configuration that makes a separately networked proxy unable to reach the application.
  • Evidence: The release summary calls loopback “the safe choice behind a reverse proxy” without that qualification.
  • Suggestion: Qualify this advice in the release text and API documentation: loopback requires the proxy to have appropriate reachability in the application's network namespace; a proxy in a separate namespace generally cannot reach that listener through its own loopback.

No migration recipe is requested for internal NeoHaskell constructors/record representations: the intended supported application API is the builder surface. Compilation and runtime tests have not been independently executed in this review environment.

… — bind input contract, loopback-exclusion evidence, single-setting coverage, proxy guidance

1. Input/failure contract: pure Web.validateBindHost / validateBindPort run in
   runWebTransport before any socket opens (empty host, URL/path, host:port,
   whitespace; port 0 and out-of-range rejected), error names the builder.
   Warp.runSettings now goes through Task.fromFailableIO @IOException and
   fails as 'WebTransport could not bind <host>:<port>' on the existing
   Task.finally cleanup path.
2. Loopback evidence: per-test unique health path (instance identity) plus a
   discriminating probe — the port stays bindable on a non-loopback address
   only for a loopback listener; wildcard-bind negative control proves the
   probe fails otherwise.
3. Single-setting preservation split into explicit host-only / port-only
   examples.
4. Reverse-proxy guidance qualified by network namespace in the release
   fragment, withHost/WebTransport.host Haddock, ADR-0079 and change 019.

Spec 019: C5 widened, C6 lists the single-setting examples, C7 (validation)
and C8 (startup failures, integration | http:real) added; 4 attestations.
BindSpec 14 -> 27 examples, 0 failures; lint no hints; spec-check,
criteria-tests, spec-drift, codemap-check, changelog --check OK.
@stepandra

Copy link
Copy Markdown
Contributor Author

Thanks — all four addressed in 676c06a (on top of 3115eae). Still a draft; not marking ready until you say so.

1. Input and failure behavior

Defined and enforced, no wrapper types, no lifecycle rewrite:

  • Host (Web.validateBindHost :: Text -> Maybe Text): accepted forms are the Warp wildcards ("*", "*4", "*6") and a bare literal address or hostname ("127.0.0.1", "::1", "localhost"). Rejected: empty/blank, anything containing / (URL or path — the "http://127.0.0.1" case), a host:port pair (exactly one colon; a literal IPv6 address always has ≥2), and whitespace.
  • Port (Web.validateBindPort :: Int -> Maybe Text): 1..65535. 0 is rejected on purpose — the OS would pick a port that nothing in the application reports back, so the server would be unreachable by design.
  • Earliest practical check: runWebTransport runs both validators right after applyBindOverrides, before Warp or any socket is touched. The error names the builder: Application.withHost: … / Application.withPort: …, and reaches the caller of Application.run/runWith.
  • Visible bind failure + cleanup: Warp.runSettings now goes through Task.fromFailableIO @IOException and is re-thrown as WebTransport could not bind <host>:<port>: <ioe>. That puts it on the same Task.finally cleanupAll path (JWKS manager, dispatcher, file-upload cleanup) as every other startup failure in runWithResolved; nothing new is owned by this operation beyond the socket Warp itself closes.

Tests: validateBindHost/validateBindPort table (7 examples), plus three runtime examples — invalid host fails before any socket, invalid port fails before any socket, port already in use fails with could not bind 127.0.0.1:<port>.

2. Loopback exclusion and instance identity

The probe now distinguishes both:

  • Identity: each running example registers a per-test unique health path (withHealthCheck @() (\_ -> "health-<uuid>")), so a 200 can only come from the application started in that example, not from another process on the same port.
  • Exclusion: after the 200, the test binds a fresh TCP socket to the same port on a non-loopback IPv4 address of the machine. For a 127.0.0.1 listener that bind succeeds (the port is free elsewhere); for a wildcard listener it fails with EADDRINUSE. Warp sets SO_REUSEADDR, so the discrimination is on address, not on the reuse flag.
  • Negative control: a second example starts the app with the default *4 bind and asserts the same probe fails — this is what rejects the false-pass case where the listener is actually wildcard.

Both are source-reasoned, bounded fixture checks (plain network sockets, no extra framework); no exploit or race is claimed.

3. Single-setting preservation

Split into explicit examples: absent/absent preserves a custom host and port; host-only override keeps the custom port (7070); port-only override keeps the custom host (::1). Plus the existing before/after-withTransport ordering case. These would catch an accidental reset-to-default of the other field.

4. Reverse-proxy guidance

Qualified in the release fragment, the withHost / WebTransport.host Haddock, ADR-0079 and change 019: loopback suits a proxy in the same network namespace (same host or container, shared pod network); a proxy in a separate namespace cannot reach a loopback listener through its own loopback, so keep a non-loopback bind and restrict with firewall/network policy. The agent prompt in the fragment now says the same.

Spec / gates

Change 019: C5 widened (identity + exclusion + negative control), C6 lists the single-setting examples, new C7 (validation) and C8 (startup failures, integration | http:real); four new attestations in test-surfaces.json.

Local: BindSpec 27 examples / 0 failures (was 14); ./dev lint no hints; dialect + expectation guards OK; spec-check, --criteria-tests, spec-drift, codemap-check, changelog --check OK. Full nhcore-test-service not re-run locally — CI will.

One thing worth your eye: the exclusion probe picks the machine's first routable IPv4 via a connect-only UDP socket (nothing is sent). If a runner has no non-loopback IPv4 it falls back to 127.0.0.1 and the positive case fails honestly rather than passing falsely; ubuntu-latest and the macOS runners always have one.

@NickSeagull

Copy link
Copy Markdown
Member

I found one more thing to fix:

  1. Host validation uses Text.words, which treats " 127.0.0.1 " as one word. That value passes validation even though the release note promises whitespace is rejected. Reject surrounding whitespace and cover that case, or reconcile the documented contract with the intended behavior.

Thanks!

This branch has not been deployed

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants