diff --git a/.claude/agents/opus-coder.md b/.claude/agents/opus-coder.md new file mode 100644 index 0000000..27b99ad --- /dev/null +++ b/.claude/agents/opus-coder.md @@ -0,0 +1,8 @@ +--- +name: opus-coder +description: Coding sessions routed per ~/.claude/rules/model-routing.md — all substantial and mechanical implementation work runs on Opus 5 at high effort; Fable stays for decomposition and review. +model: opus +effort: high +--- + +You implement one milestone session from a spec in this repository. The spec you are pointed at is the binding contract. Own only the files your session is assigned; do not commit; verify your work with the repo's build and test commands; report what you changed, what you ran, and every result faithfully, including failures. diff --git a/AGENTS.md b/AGENTS.md index 67b62f7..bdebdcf 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -64,3 +64,17 @@ One trap: running a cached test binary by hand with `--listen=-` aborts with `internal test runner failure: EndOfStream`. That is not a teardown bug; the IPC runner is talking to a closed stdin because no build runner is on the other end. Run the binary with no arguments to get the plain stdio report. + +## Regenerating the contract samples + +`web/src/lib/contractSamples.gen.ts` is a committed golden of canonicalized +API responses, byte-compared against the live server by a `-Dintegration` +test and type-checked by `tsc`. After a deliberate API contract change, +regenerate it with: + +``` +zig build test -Dintegration -Dcontract-samples-out="$PWD/web/src/lib/contractSamples.gen.ts" +``` + +then update `web/src/lib/types.ts` to match and commit both. Never edit the +generated file by hand. diff --git a/TECH_DEBT.md b/TECH_DEBT.md index f354c75..bc9f6e4 100644 --- a/TECH_DEBT.md +++ b/TECH_DEBT.md @@ -134,34 +134,34 @@ The repo's history proves this theme's stakes: commit 35f2324 fixed a process-ki ## Theme 4: Operator-facing contract drift (11 findings) -- **[medium] src/upstream/pool.zig:110 — no overall exchange deadline; the key named `total_timeout_ms` bounds one attempt.** +- **[medium — CLOSED m17] src/upstream/pool.zig:110 — no overall exchange deadline; the key named `total_timeout_ms` bounds one attempt.** With N upstreams all timing out, a query takes N × 5000ms; docs/reference/configuration.md calls the key a "per-query budget" while cli.md correctly says "per-attempt" — the two reference docs contradict each other, PLAN still promises a total budget, and validate.zig's total>=read cross-check relates knobs of different subsystems. The per-attempt code is intentional per milestone-3; the name, docs, and PLAN are the drift. Fix: either one race around the failover loop, or rename to attempt_timeout_ms and correct docs/PLAN/validator. -- **[medium] src/config/validate.zig:609 — the validator accepts hostname `tls://` upstreams that the DoT client refuses by design on every dial.** +- **[medium — CLOSED m17] src/config/validate.zig:609 — the validator accepts hostname `tls://` upstreams that the DoT client refuses by design on every dial.** Literal documented spec drift: milestone-3 explicitly told the orchestrator to carry the IP-literal requirement into the phase-4 validator, and it was dropped. Worse, docs/reference/configuration.md:348 presents a hostname NextDNS `tls://` config that can never complete an exchange. Fix: apply the same IP-literal check parseResolver already enforces, at the spot where checkCollections already holds the parsed Endpoint. -- **[medium] src/config/validate.zig:391 — no upper bound on the two fields that literally size boot allocations.** +- **[medium — CLOSED m17] src/config/validate.zig:391 — no upper bound on the two fields that literally size boot allocations.** `query_log_buffer_max = 4000000000` validates clean, `nxdns check` prints OK, `nxdns run` OOMs classified as a runtime failure. Timeouts and TTLs have maxima in the same file. Fix: sanity caps (or a byte-budget check) matching the file's established idiom; a full memory-fit guarantee is out of reach and not needed. -- **[medium] web/src/lib/queries.ts:251 — the upstream management UI was specced in milestone 8 ("the Settings page must edit them") and never built.** +- **[medium — CLOSED m17] web/src/lib/queries.ts:251 — the upstream management UI was specced in milestone 8 ("the Settings page must edit them") and never built.** The full client layer (API wrappers, query, three mutation factories) exists with zero consumers; no route or page edits the resolver pool, so URLs/priority/enabled/tls_name are curl-only. Same pattern in miniature: ruleUpdateMutation and five get* wrappers are unused. Fix: build the editor the m8 spec called for, or delete the vestigial factories and record the scope decision. -- **[medium] src/web/api_limiter.zig:137 — behind the documented reverse proxy, the API limiter is bypassed and the SSE cap collapses.** +- **[medium — CLOSED m17] src/web/api_limiter.zig:137 — behind the documented reverse proxy, the API limiter is bypassed and the SSE cap collapses.** Proxy-on-same-box makes every request loopback; default `localhost_exempt = true` then disables the only brake on repeated argon2 verifications (the spec-mandated brute-force defense), while remote users share one address's 3-stream SSE cap. Documented nowhere. Fix: an opt-in trusted-proxy setting (parse X-Forwarded-For only from configured proxies) or at minimum a documented warning to set `api_localhost_exempt=false` when proxying. -- **[medium] web/src/lib/types.ts:1 — the REST contract lives in three hand-synced copies with a route-name-only drift guard.** +- **[medium — CLOSED m17] web/src/lib/types.ts:1 — the REST contract lives in three hand-synced copies with a route-name-only drift guard.** Zig handlers, openapi.yaml, and types.ts are each maintained by hand; the only automated guard checks route paths textually. The Zig side is genuinely well contract-tested against a real server, but the server↔yaml and server↔types.ts field-level seams are unguarded, and every frontend test stubs fetch. The milestone-9 "no codegen" ruling excludes generation, not guarding. Fix: one integration layer asserting the frontend's consumed shapes against real responses per endpoint. -- **[low] src/dns/packet.zig:354 — BADVERS is unexpressible: addOptEcho hardcodes extended_rcode = 0 and no caller checks the EDNS version.** +- **[low — CLOSED m17] src/dns/packet.zig:354 — BADVERS is unexpressible: addOptEcho hardcodes extended_rcode = 0 and no caller checks the EDNS version.** A version-1 query answered locally gets NOERROR with a version-0 OPT instead of RCODE=16 (RFC 6891 MUST). Negligible operational impact (real clients send version 0; forwarded replies pass through), but the write-direction mechanism is missing entirely. Fix: an extended_rcode parameter on addOptEcho plus a version check in the handler. -- **[low] src/server/rate_limiter.zig:5 — module docs still defer their own concurrency contract to "Phase 7", which shipped.** +- **[low — CLOSED m17] src/server/rate_limiter.zig:5 — module docs still defer their own concurrency contract to "Phase 7", which shipped.** The authoritative thread-safety statement points at a future that resolved (handler.limiter_mutex, app.runMaintenance); shutdown.zig references a Phase-8 restart endpoint that never shipped and now cannot. Fix: rewrite the headers to state the as-built contract. -- **[low] docs/how-to/install-with-systemd.md:206 — `nxdns 0.1.0-dev` hardcoded in four doc transcripts with no drift guard.** +- **[low — CLOSED m17] docs/how-to/install-with-systemd.md:206 — `nxdns 0.1.0-dev` hardcoded in four doc transcripts with no drift guard.** Correct today; goes silently wrong at the first tag. The milestone-14 spec already mandates the placeholder + guard. Caveat for implementation: milestone-13 requires every doc command block to be executed verbatim, so decide how placeholders coexist with executed transcripts. -- **[low] src/platform/logging.zig:497 — manual file_pos tracking assumes exclusive ownership of the log path, undocumented for operators.** +- **[low — CLOSED m17] src/platform/logging.zig:497 — manual file_pos tracking assumes exclusive ownership of the log path, undocumented for operators.** logrotate copytruncate produces a sparse NUL-prefixed file; the docs describe built-in rotation, even claiming the file is "opened for append" (it is not — positional writes), without warning off external rotation. Fix: a warning in files-and-directories.md at minimum; a stat/length re-check before write for robustness. ## Theme 5: Protocol robustness (5 findings) diff --git a/build.zig b/build.zig index 5f2d0f4..ca5a51e 100644 --- a/build.zig +++ b/build.zig @@ -19,6 +19,15 @@ pub fn build(b: *std.Build) void { const integration = b.option(bool, "integration", "Run hermetic integration tests (loopback sockets only)") orelse false; const live = b.option(bool, "live", "Run tests that reach external network hosts") orelse false; const fuzz = b.option(bool, "fuzz", "Build the fuzz targets with the LLVM backend (required for --fuzz)") orelse false; + // Milestone-17 ruling 5. The embedded golden carries no source-tree path, + // so regeneration names the destination explicitly: + // zig build test -Dintegration \ + // -Dcontract-samples-out="$PWD/web/src/lib/contractSamples.gen.ts" + const contract_samples_out = b.option( + []const u8, + "contract-samples-out", + "Absolute path the contract-sample generator writes instead of comparing", + ) orelse ""; const version_string = b.option([]const u8, "version-string", "Version reported by `nxdns version`") orelse "0.1.0-dev"; const git_commit = b.option([]const u8, "git-commit", "Git commit reported by `nxdns version`") orelse "unknown"; const web_dist = b.option( @@ -56,6 +65,7 @@ pub fn build(b: *std.Build) void { options.addOption([]const u8, "version_string", version_string); options.addOption([]const u8, "git_commit", git_commit); options.addOption([]const u8, "zig_version_string", builtin.zig_version_string); + options.addOption([]const u8, "contract_samples_out", contract_samples_out); const exe = addExecutable(b, target, optimize, options, web_assets); b.installArtifact(exe); @@ -118,6 +128,11 @@ pub fn build(b: *std.Build) void { tests.root_module.addAnonymousImport("docs_files", .{ .root_source_file = b.path("docs/docs.zig"), }); + // An anonymous-import root must be Zig, so the committed TypeScript golden + // is reached through a one-decl wrapper beside it. + tests.root_module.addAnonymousImport("contract_samples", .{ + .root_source_file = b.path("web/src/lib/contract_samples.zig"), + }); tests.root_module.addAnonymousImport("web_assets", .{ .root_source_file = web_assets }); const test_step = b.step("test", "Run the test suite"); test_step.dependOn(&b.addRunArtifact(tests).step); @@ -263,6 +278,9 @@ pub fn build(b: *std.Build) void { aarch64_tests.root_module.addAnonymousImport("docs_files", .{ .root_source_file = b.path("docs/docs.zig"), }); + aarch64_tests.root_module.addAnonymousImport("contract_samples", .{ + .root_source_file = b.path("web/src/lib/contract_samples.zig"), + }); aarch64_tests.root_module.addAnonymousImport("web_assets", .{ .root_source_file = web_assets }); const aarch64_run = b.addRunArtifact(aarch64_tests); aarch64_run.skip_foreign_checks = true; diff --git a/docs/docs.zig b/docs/docs.zig index 5e8180f..7143da1 100644 --- a/docs/docs.zig +++ b/docs/docs.zig @@ -5,3 +5,47 @@ pub const reference_api_md = @embedFile("reference/api.md"); pub const reference_configuration_md = @embedFile("reference/configuration.md"); pub const reference_cli_md = @embedFile("reference/cli.md"); + +pub const tutorial_first_run_md = @embedFile("tutorial/first-run.md"); + +pub const howto_back_up_and_restore_md = @embedFile("how-to/back-up-and-restore.md"); +pub const howto_enable_doh_and_dot_md = @embedFile("how-to/enable-doh-and-dot.md"); +pub const howto_install_with_docker_md = @embedFile("how-to/install-with-docker.md"); +pub const howto_install_with_systemd_md = @embedFile("how-to/install-with-systemd.md"); +pub const howto_measure_performance_md = @embedFile("how-to/measure-performance.md"); +pub const howto_set_up_admin_authentication_md = @embedFile("how-to/set-up-admin-authentication.md"); +pub const howto_troubleshoot_md = @embedFile("how-to/troubleshoot.md"); +pub const howto_upgrade_md = @embedFile("how-to/upgrade.md"); + +pub const Page = struct { + /// Repo-relative path, so a failing assertion names the file to edit. + path: []const u8, + text: []const u8, +}; + +/// Every page this module embeds. A guard that must hold for all of them walks +/// this table instead of naming each decl. +pub const pages: []const Page = &.{ + .{ .path = "docs/reference/api.md", .text = reference_api_md }, + .{ .path = "docs/reference/configuration.md", .text = reference_configuration_md }, + .{ .path = "docs/reference/cli.md", .text = reference_cli_md }, + .{ .path = "docs/tutorial/first-run.md", .text = tutorial_first_run_md }, + .{ .path = "docs/how-to/back-up-and-restore.md", .text = howto_back_up_and_restore_md }, + .{ .path = "docs/how-to/enable-doh-and-dot.md", .text = howto_enable_doh_and_dot_md }, + .{ .path = "docs/how-to/install-with-docker.md", .text = howto_install_with_docker_md }, + .{ .path = "docs/how-to/install-with-systemd.md", .text = howto_install_with_systemd_md }, + .{ .path = "docs/how-to/measure-performance.md", .text = howto_measure_performance_md }, + .{ .path = "docs/how-to/set-up-admin-authentication.md", .text = howto_set_up_admin_authentication_md }, + .{ .path = "docs/how-to/troubleshoot.md", .text = howto_troubleshoot_md }, + .{ .path = "docs/how-to/upgrade.md", .text = howto_upgrade_md }, +}; + +/// The pages that paste a transcript naming the running binary's version. Each +/// one must print the `` placeholder rather than the version of the +/// day the page was written. +pub const version_transcript_pages: []const Page = &.{ + .{ .path = "docs/tutorial/first-run.md", .text = tutorial_first_run_md }, + .{ .path = "docs/how-to/install-with-docker.md", .text = howto_install_with_docker_md }, + .{ .path = "docs/how-to/install-with-systemd.md", .text = howto_install_with_systemd_md }, + .{ .path = "docs/how-to/upgrade.md", .text = howto_upgrade_md }, +}; diff --git a/docs/how-to/back-up-and-restore.md b/docs/how-to/back-up-and-restore.md index 4339b04..357dff1 100644 --- a/docs/how-to/back-up-and-restore.md +++ b/docs/how-to/back-up-and-restore.md @@ -52,14 +52,18 @@ Without `--out` the export goes to stdout, where the file mode is your redirect's problem: ```sh -nxdns export --data-dir /tmp/nxdns-lab/data | head -6 +nxdns export --data-dir /tmp/nxdns-lab/data | head -10 ``` ``` // nxdns configuration // generated by `nxdns export` — the database is the source of truth .{ - .upstream = .{ .read_timeout_ms = 3000, .total_timeout_ms = 5000 }, + .upstream = .{ + .attempt_timeout_ms = 2500, + .read_timeout_ms = 3000, + .total_timeout_ms = 5000, + }, .dns = .{ .bind_ipv4 = "127.0.0.1", ``` diff --git a/docs/how-to/install-with-docker.md b/docs/how-to/install-with-docker.md index 3783934..5e24cf0 100644 --- a/docs/how-to/install-with-docker.md +++ b/docs/how-to/install-with-docker.md @@ -123,7 +123,7 @@ A healthy first start logs the seeding and the bound sockets: ``` info(config_bootstrap): seeded the database from '/etc/nxdns/config.zon' -info(nxdns): nxdns 0.1.0-dev serving on udp [::]:53 tcp [::]:53 tcp 0.0.0.0:53; 1 upstream(s); blocklist generation 1 +info(nxdns): nxdns serving on udp [::]:53 tcp [::]:53 tcp 0.0.0.0:53; 1 upstream(s); blocklist generation 1 info(web_server): web interface listening on 0.0.0.0:8080 ``` diff --git a/docs/how-to/install-with-systemd.md b/docs/how-to/install-with-systemd.md index f3bbd63..ed29ede 100644 --- a/docs/how-to/install-with-systemd.md +++ b/docs/how-to/install-with-systemd.md @@ -203,7 +203,7 @@ journalctl -u nxdns -f A healthy start logs a line naming every socket it bound: ``` -info(nxdns): nxdns 0.1.0-dev serving on udp [::]:53 tcp [::]:53 tcp 0.0.0.0:53; 1 upstream(s); blocklist generation 1 +info(nxdns): nxdns serving on udp [::]:53 tcp [::]:53 tcp 0.0.0.0:53; 1 upstream(s); blocklist generation 1 ``` nxdns writes to stderr and systemd captures that into the journal; logging diff --git a/docs/how-to/set-up-admin-authentication.md b/docs/how-to/set-up-admin-authentication.md index e878846..1ddd837 100644 --- a/docs/how-to/set-up-admin-authentication.md +++ b/docs/how-to/set-up-admin-authentication.md @@ -259,6 +259,22 @@ interface at the very least, and preferably set a password. wrong, and they would retype a password that can never verify. - Requests from the box itself skip the API rate limit by default (`web.api_localhost_exempt`). +- **If you put a reverse proxy in front of the admin interface, configure + `web.trusted_proxies` or turn `web.api_localhost_exempt` off.** A proxy on the + same box connects from loopback, so every request arrives exempt and the API + limiter — the only brake on guessing the admin password — stops applying to + anyone. Listing the proxy's address in `web.trusted_proxies` makes nxdns read + the client's address from the `X-Forwarded-For` the proxy appends, so the + limiter and the SSE connection cap bind each real client again: + + ```zig + .web = .{ + .trusted_proxies = "127.0.0.1", + }, + ``` + + The proxy must append its own entry to that header. A proxy that forwards a + client-supplied `X-Forwarded-For` unchanged is not one to trust. - `web.session_ttl_hours`, `web.api_rate_limit_per_min` and the rest are in the [configuration reference](../reference/configuration.md); the routes are in the [API reference](../reference/api.md). diff --git a/docs/how-to/upgrade.md b/docs/how-to/upgrade.md index 2adb15f..30867b8 100644 --- a/docs/how-to/upgrade.md +++ b/docs/how-to/upgrade.md @@ -139,7 +139,7 @@ OK: no problems found > > ``` > $ nxdns version -> nxdns 0.1.0-dev (unknown) +> nxdns (unknown) > zig 0.16.0 > ``` > diff --git a/docs/reference/api.md b/docs/reference/api.md index 1e4b041..1525f6a 100644 --- a/docs/reference/api.md +++ b/docs/reference/api.md @@ -71,6 +71,18 @@ to the capacity is admitted and the long-run rate holds. up, never zero). - Loopback addresses (127.0.0.0/8 and ::1) are exempt while `web.api_localhost_exempt` is true (the default). +- The address a bucket keys on is the socket peer, unless that peer is listed in + `web.trusted_proxies`. For a listed peer the address is instead the **last** + entry of the request's `X-Forwarded-For` — the entry the proxy appended, which + is the only one a client cannot write. A request from a trusted proxy with no + such header keys on the proxy itself; one whose last entry is not an IP + literal is answered 400, because the alternative is granting the proxy's own + loopback exemption to whoever sent it. Only `X-Forwarded-For` is read; + `Forwarded` (RFC 7239) and the PROXY protocol are not. +- Without `web.trusted_proxies`, a same-box reverse proxy makes every request + loopback, so the default exemption disables the limiter for all remote + clients. Set the proxy's address there, or set + `web.api_localhost_exempt = false`. - Exempt routes, which never consult a bucket: `/metrics` and `/api/health` (a Prometheus scrape must never see 429) and `/api/queries/live` (one long-lived stream must not drain its address's bucket; it is bounded by the SSE diff --git a/docs/reference/cli.md b/docs/reference/cli.md index 62e4b33..18d1167 100644 --- a/docs/reference/cli.md +++ b/docs/reference/cli.md @@ -43,7 +43,8 @@ first. What it checks, in order: bit is set, which does not change the exit code. 4. A live probe: one real A query for `example.com` through every enabled upstream, driving the same pool and failover machinery the server uses, with - `upstream.total_timeout_ms` as the per-attempt deadline. A FAIL line names + the same two deadlines: `upstream.attempt_timeout_ms` bounds one try against + one upstream, `upstream.total_timeout_ms` the whole probe. A FAIL line names the upstream and the concrete cause recorded in its health. This probe leaves the machine, so `check` needs network access to pass. It runs from the command line but not from unit tests. diff --git a/docs/reference/configuration.md b/docs/reference/configuration.md index 4b976a3..5a95006 100644 --- a/docs/reference/configuration.md +++ b/docs/reference/configuration.md @@ -53,8 +53,20 @@ Timeouts for talking to upstream resolvers. | Key | Type | Default | Unit | Validation | Consumed by | |---|---|---|---|---|---| +| `upstream.attempt_timeout_ms` | u32 | 2500 | ms | 100–120000, and not above `total_timeout_ms` | deadline on one attempt against one upstream inside the pool's failover loop (`src/upstream/pool.zig`), the whole attempt including the connect | | `upstream.read_timeout_ms` | u32 | 3000 | ms | 100–120000 | read deadline on conditional-forward-zone exchanges (`src/local/forward_client.zig`) | -| `upstream.total_timeout_ms` | u32 | 5000 | ms | 100–120000, and not below `read_timeout_ms` | per-query budget of the upstream pool (`src/upstream/pool.zig`), the whole attempt including the connect; also the `nxdns check` probe deadline | +| `upstream.total_timeout_ms` | u32 | 5000 | ms | 100–120000 | per-query budget of the upstream pool (`src/upstream/pool.zig`): every failover attempt together, not one of them; also the `nxdns check` probe deadline | + +The two pool budgets nest. `attempt_timeout_ms` bounds one try against one +upstream; when it expires the pool records the failure and moves to the next +candidate. `total_timeout_ms` bounds the whole loop, so a query against five +unreachable upstreams costs the total budget once, not five attempt budgets in +a row. When the total expires the in-flight attempt is canceled and the query +fails with a timeout. + +`read_timeout_ms` is unrelated to both. It bounds a different subsystem — the +conditional-forward-zone client — so no cross-check relates it to the pool's +budgets, and it is free to sit above either of them. ### dns @@ -86,9 +98,14 @@ What a blocked query gets back. | Key | Type | Default | Unit | Validation | Consumed by | |---|---|---|---|---|---| -| `cache.size` | u32 | 10000 | entries | none; 0 disables caching (puts short-circuit) | DNS answer cache capacity (`src/cache/dns_cache.zig`) | +| `cache.size` | u32 | 10000 | entries | 1–1000000 | DNS answer cache capacity (`src/cache/dns_cache.zig`) | | `cache.negative_ttl_max` | u32 | 3600 | seconds | at most 86400 | cap on cached negative answers; 0 disables negative caching | +The cache's slot array is allocated in full at startup, so `cache.size` carries +a ceiling: it is a sanity bound against a typo, not a promise that the value +fits in the box's memory. There is no "off" value — to run without a cache, set +`cache.size` to 1. + ### web The web interface and REST API. @@ -104,6 +121,7 @@ The web interface and REST API. | `web.api_rate_limit_per_min` | u32 | 300 | requests per minute | at least 1 | API token-bucket limiter (`src/web/api_limiter.zig`) | | `web.api_localhost_exempt` | bool | true | — | — | loopback requests skip the API limiter | | `web.sse_max_connections_per_ip` | u16 | 3 | connections | at least 1 | cap on concurrent SSE streams per client address | +| `web.trusted_proxies` | string | `""` | — | comma-separated IP literals | reverse proxies whose `X-Forwarded-For` nxdns believes (`src/web/server.zig`); empty trusts none | ### doh_server @@ -145,7 +163,7 @@ Process log and query log behavior. |---|---|---|---|---|---| | `logging.level` | enum `.err` \| `.warn` \| `.info` \| `.debug` | `.info` | — | one of the four tags; stored as `"error"` / `"warn"` / `"info"` / `"debug"` | log threshold (`src/platform/logging.zig`) | | `logging.retention_days` | u16 | 30 | days | at least 1 | query-log pruning cutoff (`src/storage/retention.zig`) and the client tracker's last-seen cutoff (`src/server/clients.zig`) | -| `logging.query_log_buffer_max` | u32 | 10000 | entries | at least 1 | in-memory query-log ring size and backpressure cap (`src/storage/logger.zig`) | +| `logging.query_log_buffer_max` | u32 | 10000 | entries | 1–1000000 | in-memory query-log ring size and backpressure cap (`src/storage/logger.zig`) | | `logging.hide_domains` | bool | false | — | — | the query log stores a hidden marker instead of the domain | | `logging.hide_client_ips` | bool | false | — | — | the query log stores a hidden marker instead of the client address | | `logging.output` | enum `.stderr` \| `.syslog` \| `.file` | `.stderr` | — | one of the three tags | log sink selection (`src/platform/logging.zig`); `.stderr` and `.syslog` both write to stderr (journald captures it), `.file` rotates | @@ -197,7 +215,7 @@ Upstream resolvers. At least one enabled upstream is required. | Field | Type | Default | Validation | |---|---|---|---| -| `url` | string | required | an `https://` (DoH) or `tls://` (DoT) endpoint accepted by `transport.Endpoint.parse`; unique | +| `url` | string | required | an `https://` (DoH) or `tls://` (DoT) endpoint accepted by `transport.Endpoint.parse`; unique; a `tls://` host must be an IP literal | | `priority` | i32 | 100 | — (lower is tried first) | | `enabled` | bool | true | — | | `tls_name` | string | `""` | DoT only — a `tls_name` on an `https://` upstream is an error; when set it must be a valid domain name | @@ -207,6 +225,19 @@ ascending priority and tried in order with failover. `tls_name` sets SNI and the certificate verification name for a `tls://` upstream written as an IP literal; empty means "verify by the URL host" (`src/upstream/dot_client.zig`). +A `tls://` upstream's host must be an IP literal. nxdns does not resolve an +upstream's own name — that is a bootstrap problem, and the DoT client refuses a +non-literal host on every dial — so the validator rejects the hostname form +rather than letting it fail at query time as an unreachable upstream. Write the +address and put the name in `tls_name`: + +```zig +.{ .url = "tls://9.9.9.9:853", .tls_name = "dns.quad9.net" }, +``` + +A DoH upstream is not affected: `https://` goes through the HTTP client, which +resolves normally, so a hostname there is the usual form. + ### clients Known clients with a fixed group assignment. @@ -344,18 +375,19 @@ path segment is the whole account identifier. The field path beside the message names the entry, so `blocklist_sources[1].url` still says which one to go and fix. -One credential this cannot remove: a NextDNS **DoT** upstream is -`tls://abcd12.dns.nextdns.io`, which carries the same identifier in the hostname. -Stripping it would leave no host at all and no line worth reading. So if you use -NextDNS over DoT, your profile id appears in the log. +What redaction cannot remove is the host, because a hostname is not a secret in +the general case — it is resolved publicly and offered as SNI on every +connection — and dropping it would leave a diagnostic that names nothing worth +reading. A vendor that puts an account identifier in the hostname therefore has +that identifier appear in any line naming the entry. NextDNS is the example: +`abcd12.dns.nextdns.io` is a profile id. -The host is kept because a hostname is not a secret in the general case — it is -resolved publicly and offered as SNI on every connection — and dropping it would -cost every operator a diagnostic to cover one vendor's choice. If that trade is -wrong for you, it is yours to make rather than ours: NextDNS also publishes a DoH -endpoint, `https://dns.nextdns.io/abcd12`, whose identifier sits in the path and -is redacted in full. Configure that form instead and nothing identifying reaches -the log. +Two things limit the exposure. A hostname `tls://` upstream is rejected by the +validator (see `upstreams` above), so the DoT form of that URL can only reach +the log once, in the line rejecting it — never on every failover. And NextDNS +publishes a DoH endpoint, `https://dns.nextdns.io/abcd12`, whose identifier sits +in the path and is redacted in full; configure that form and nothing identifying +reaches the log at all. The error set is `validate.ValidateError` in `src/config/validate.zig`: @@ -363,6 +395,7 @@ The error set is `validate.ValidateError` in `src/config/validate.zig`: | --- | --- | | `NoUpstreams` | no upstream has `enabled = true` | | `BadUpstreamUrl` | an upstream `url` is not an `https://` or `tls://` endpoint | +| `UpstreamHostNotIpLiteral` | a `tls://` upstream `url` names a host instead of an IP literal | | `DuplicateUpstreamUrl` | two upstreams share a `url` | | `BadTlsName` | a `tls_name` is not a valid domain name | | `TlsNameOnNonTlsUpstream` | a `tls_name` on a non-`tls://` upstream | @@ -378,13 +411,15 @@ The error set is `validate.ValidateError` in `src/config/validate.zig`: | `BadLocalRecordName` / `BadLocalRecordValue` / `DuplicateLocalRecord` | local record fields | | `BadForwardZone` / `DuplicateForwardZone` / `BadResolverUrl` | forward zone fields | | `BadPort` | a port field is 0 | -| `BadTimeout` | a timeout is outside 100–120000 ms, or `total` is below `read` | +| `BadTimeout` | a timeout is outside 100–120000 ms, or `attempt` is above `total` | | `BadTtl` | `blocking.ttl`, `cache.negative_ttl_max`, a record `ttl`, `web.session_ttl_hours` or `blocklist_update.interval_hours` outside its range | -| `BadRetention` | `logging.retention_days` or `logging.query_log_buffer_max` below 1 | +| `BadCacheSize` | `cache.size` outside 1–1000000 | +| `BadRetention` | `logging.retention_days` below 1, or `logging.query_log_buffer_max` outside 1–1000000 | | `BadLogRotation` | `logging.max_size_mb` or `logging.max_files` below 1 | | `BadDiskThresholds` | a threshold below 1, or `min_free_mb` above `warn_free_mb` | | `BadRateLimit` | `dns.rate_limit`, `dns.rate_window_seconds`, `web.api_rate_limit_per_min` or `web.sse_max_connections_per_ip` out of range | | `BadBindAddress` | a bind field is not an IP address, or not of the required family | +| `BadTrustedProxy` | an element of `web.trusted_proxies` is not an IP literal | | `MissingCertPath` / `MissingKeyPath` | an enabled TLS listener with an empty path | | `MissingLogPath` | `logging.output = .file` with an empty or relative `file_path` | | `PasswordAndHashBothSet` | both `web.password` and `web.password_hash` are set | diff --git a/docs/reference/files-and-directories.md b/docs/reference/files-and-directories.md index e6d8109..225e598 100644 --- a/docs/reference/files-and-directories.md +++ b/docs/reference/files-and-directories.md @@ -140,9 +140,25 @@ file is also sampled by the disk monitor. | Path | What it is | Mode | | --- | --- | --- | -| `logging.file_path` (default `/var/log/nxdns/nxdns.log`) | The live process log. Opened for append; created when it does not exist. | `0666 & ~umask` — nxdns never chmods it | +| `logging.file_path` (default `/var/log/nxdns/nxdns.log`) | The live process log. Opened write-only and written at a tracked offset; created when it does not exist. | `0666 & ~umask` — nxdns never chmods it | | `.1` … `.` | Rotated generations, newest first. | Inherited from the live file they were renamed from | +**Do not point external logrotate at this file.** nxdns owns the rotation of +its own log. There is no append mode in Zig 0.16, so the writer reads the file +length once when it opens the file and then writes every line at an offset it +tracks in the process. A rotator that moves the file behind it breaks that +offset, and nxdns does not notice until the next restart: + +- With `copytruncate` the offset survives the truncation, so the next line + lands where it would have without it. The file regrows with a sparse, + NUL-filled prefix as long as the log that was just rotated away. +- With rename-and-create nxdns keeps writing to the renamed inode. The new + file stays empty, the renamed one grows without bound, and + `logging.max_size_mb` bounds nothing on disk. + +If an external rotator has to own the file, set `logging.output = .stderr` and +let the collector capture the stream instead. + The log file is the one path here nxdns does not set a mode on. `config.db` and `querylog.db` are chmodded to 0600 whatever the umask; the log file is left at whatever the umask gives it. Under the shipped unit that is 0600, because diff --git a/docs/tutorial/first-run.md b/docs/tutorial/first-run.md index 7185433..095f73c 100644 --- a/docs/tutorial/first-run.md +++ b/docs/tutorial/first-run.md @@ -105,7 +105,7 @@ info(migrations): config.db migrated from schema version 0 to 2 info(config_bootstrap): seeded the database from '/home/you/nxdns-tutorial/config.zon' info(querylog_schema): created querylog database '/home/you/nxdns-tutorial/data/querylog.db' info(blocklist_manager): blocklist snapshot generation 1: 0 of 0 sources loaded, 199 bytes -info(nxdns): nxdns 0.1.0-dev serving on udp [::1]:15353 udp 127.0.0.1:15353 tcp [::1]:15353 tcp 127.0.0.1:15353; 1 upstream(s); blocklist generation 1 +info(nxdns): nxdns serving on udp [::1]:15353 udp 127.0.0.1:15353 tcp [::1]:15353 tcp 127.0.0.1:15353; 1 upstream(s); blocklist generation 1 info(web_server): web interface listening on 127.0.0.1:8080 ``` diff --git a/specs/milestone-13.md b/specs/milestone-13.md index a60d1a4..2115451 100644 --- a/specs/milestone-13.md +++ b/specs/milestone-13.md @@ -26,7 +26,9 @@ missing tutorial, and prove every instruction by running it. alone: the first real blocklist download aborted the process (commit 35f2324) and a stale SPA bundle crashed the settings page — both would have surfaced if the documented paths had been run. No session may report a page complete on the - strength of having read the code. + strength of having read the code. This ruling constrains **command lines**; + a pasted **output line** may carry a placeholder where the literal text + would go stale, as milestone-17 ruling 8 does with `nxdns `. 4. **Content is verified against `src/`, not copied from the old pages.** Defaults, flags, exit codes, paths and route names come from the code at HEAD. Report any diff --git a/specs/milestone-17.md b/specs/milestone-17.md index 3b1b5f9..f3e90d1 100644 --- a/specs/milestone-17.md +++ b/specs/milestone-17.md @@ -78,6 +78,17 @@ total budget. Decision: the total budget becomes real. file's ceiling-only idiom: `1..1_000_000` with the "must be at most" message shape. No full memory-fit guarantee — out of reach, not needed. +**Recorded (implementation):** three consequences this ruling did not call +out. (a) `cache.size = 0` was a documented "disables caching" mode; the +`1..` floor removes it — configuration.md now says there is no off value +(use 1). (b) The configuration.md NextDNS-DoT redaction passage lost its +premise (a hostname `tls://` URL is now unconfigurable) and was rewritten. +(c) Three config fixtures used `tls://dot.example:853` and now use an IP +literal with `tls_name`; the back-up-and-restore.md export transcript grew +to `head -10` with real re-captured output. Also: `Pool.init` takes a named +`pool.Timeouts { attempt, total }` struct, not two positional durations — +two same-typed adjacent parameters would be silently swappable. + ### 3. The upstream editor is built Milestone 8 ruling 9 requires upstream CRUD in the UI ("the Settings page @@ -179,6 +190,16 @@ address's 3-stream SSE cap. No X-Forwarded-For handling exists anywhere chain → 400); an integration test proving a proxied remote address is rate-limited while the proxy itself stays exempt. +**Recorded (implementation of ruling 4):** `Request.client_addr` is +`std.Io.net.IpAddress`, not `address.NetAddress` — `http_util.zig` is a +module root for the fuzz target and cannot import nxdns modules; any future +ruling that wants a domain type on `Request` hits this. `clientAddr` reads +only the final chain entry, never scanning backwards: an earlier entry is +client-controlled, so a fallback would hand the request an attacker-chosen +identity. Known residual, out of this ruling's list: the login log line in +`handlers/auth.zig` still prints the socket peer, which reads as loopback +behind a trusted proxy — carried to milestone 19. + ### 5. A field-level contract guard between server and frontend The REST contract lives in three hand-synced copies. The Zig side is @@ -217,6 +238,16 @@ Mechanism — captured samples validated by `tsc`, no new dependencies: checking, so a server field missing from types.ts, a types.ts field missing from the wire, and an out-of-union literal all fail `npm run typecheck` — which already runs in CI's frontend job. +**Recorded (implementation):** canonicalization gained two rules beyond +"keys sorted, numbers→0": repeated array elements dedupe by canonical text +(60 identical zeroed timeseries buckets collapse to one; differing rows all +stay), and two build-identity strings (`Version.git_commit`, +`Version.zig_version`) redact to `""` so the golden is not pinned to +one machine — the list is the named constant `volatile_string_keys`. The +renderer emits prettier's own fixpoint output, because CI runs +`format:check` on the tree. 43 samples: 38 endpoint + 5 error classes. The +regen command is documented in AGENTS.md. + - Embedding: an anonymous-import root must be Zig, not TypeScript, so a wrapper `web/src/lib/contract_samples.zig` exports `pub const bytes = @embedFile("contractSamples.gen.ts");` and build.zig @@ -262,6 +293,13 @@ finding. `edns.extendedRcode`) and version 0 in the reply OPT; a version-0 query is unaffected. +**Recorded (implementation):** the write-side splitter is +`edns.splitRcode` with `pub const badvers: u12 = 16`; the BADVERS reply +echoes the question when `qdcount == 1`, following the neighboring +FORMERR sites, so a client can bind the reply to its query; a malformed +OPT whose version byte is 1 answers FORMERR, because `parseOpt` fails +before the version check can read the byte. + ### 7. The stale phase comments state the as-built contract - `rate_limiter.zig:5-6` ("Phase 7 decides the locking"): rewrite to the @@ -404,8 +442,12 @@ Deleted surface: the seven unused `get*` wrappers, `getMetrics`, types.ts type fails `npm run typecheck` (proven, then reverted). - [ ] A version-1 EDNS query answers composite RCODE 16 with version 0; `nxdns_dns_badvers_total` appears in /metrics. -- [ ] Grep: "Phase 7" and "Phase 8" appear in no source doc comment; - the rewritten headers state the mutex and the maintenance task. +- [ ] "Phase 7"/"Phase 8" no longer appear in `rate_limiter.zig`, + `shutdown.zig`, or `pool.zig`; the rewritten headers state the + mutex and the maintenance task. (Narrowed during implementation: + the original repo-wide grep exceeded ruling 7's three files — 24 + stale phase references remain in files this milestone's sessions + do not own; the sweep moved to milestone 19.) - [ ] Grep: `0.1.0-dev` appears in no docs/ page; the new drift test passes and was proven able to fail (reinsert the literal, watch it fail, revert). diff --git a/specs/milestone-19.md b/specs/milestone-19.md index 9d1904f..a032fcb 100644 --- a/specs/milestone-19.md +++ b/specs/milestone-19.md @@ -313,6 +313,29 @@ pinned by two tests. Restructure: `try { await onSubmit(...) } catch (error) { if (error instanceof ApiError) return; throw error; }`, resets after the try. The page's `` keeps rendering the mutation error as today. +### Carried in from milestone 17: the stale phase-comment sweep + +Milestone 17 ruling 7 rewrote the "Phase 7/8" headers in three files; its +repo-wide acceptance grep then found 24 more stale phase references in +files its sessions did not own, recorded here for this milestone: +safesearch.zig:58, forward_client.zig:7, dns_cache.zig:10/:42/:435, +cli.zig:332, filter_integration_test.zig:1391, logger.zig:178/:303, +retention.zig:7/:137/:151, disk_monitor.zig:7, querylog_schema.zig:71, +db.zig:272, manager.zig:292/:613/:1145, and the six repository files +(groups_repo.zig:7, rules_repo.zig:15, sources_repo.zig:9, +upstreams_repo.zig:7, local_repo.zig:3, clients_repo.zig:12-13). Each +comment is rewritten to state the as-built truth it gestures at (not +deleted, unless it says nothing beyond the phase number). Line numbers +are hints from m17-time HEAD; re-grep before acting. Acceptance: "Phase +7" and "Phase 8" appear in no source doc comment repo-wide; spec files +and TECH_DEBT.md are exempt. The session owning each file in the plan +below picks up its share; files owned by no session fall to S4. + +Also carried from milestone 17 (ruling 4 residual): the login log line in +`src/web/handlers/auth.zig` prints the socket peer, which reads as +loopback for every login behind a trusted proxy; it should print +`request.client_addr`. + ## Sessions S1-S4 run in parallel; no two sessions write the same file. diff --git a/specs/milestone-8.md b/specs/milestone-8.md index 55ce406..72f27d4 100644 --- a/specs/milestone-8.md +++ b/specs/milestone-8.md @@ -54,6 +54,14 @@ anything else against /home/mokhtar/app/zig tag 0.16.0. re-materializes). Client prefixes: `GET/PUT /api/client-prefixes` as a whole-list resource (tiny table, atomic replace). No POST for clients — creation is by DNS activity or import (PLAN:540 gives clients no POST deliberately). + + > **Placement deviation (milestone 17 ruling 3).** "The Settings page must + > edit them" above names the wrong page. Milestone 9 dropped the obligation + > entirely; milestone 17 restored it as a dedicated **Upstreams** page with + > its own nav entry (`web/src/features/upstreams/`, route `/upstreams`), + > matching the house resource-page pattern that every other collection + > follows. The Settings page keeps the scalar settings keys only. The API + > contract in this ruling is unchanged. 10. **Repo layer**: every list row the API serves carries its row id; each mutated resource gains `getX(db, id)`, `updateX(db, id, item)`, `deleteX(db, id)` (strict: 0 rows touched → error.NotFound), written in the house repo idiom with prepared statements. Existing diff --git a/src/app.zig b/src/app.zig index 6a547ab..0e23735 100644 --- a/src/app.zig +++ b/src/app.zig @@ -283,7 +283,10 @@ fn serve(r: cli.Runner, args: cli.RunArgs) !u8 { var pool: pool_mod.Pool = .init( upstreams.active(), .{}, - .{ .raw = model.totalTimeout(cfg.upstream), .clock = .awake }, + .{ + .attempt = .{ .raw = model.attemptTimeout(cfg.upstream), .clock = .awake }, + .total = .{ .raw = model.totalTimeout(cfg.upstream), .clock = .awake }, + }, @truncate(@as(u96, @bitCast(std.Io.Clock.real.now(io).nanoseconds))), ); diff --git a/src/cli.zig b/src/cli.zig index b886bd8..f6e25c5 100644 --- a/src/cli.zig +++ b/src/cli.zig @@ -922,9 +922,11 @@ fn probeUpstreams(r: Runner, cfg: model.Config) !usize { const response_buf = try r.gpa.alloc(u8, transport.max_message_len); defer r.gpa.free(response_buf); - const attempt_timeout: std.Io.Clock.Duration = .{ - .raw = model.totalTimeout(cfg.upstream), - .clock = .awake, + // The same pair the server runs with, so a probe that passes here says + // something about the deadlines a real query will get. + const timeouts: pool.Timeouts = .{ + .attempt = .{ .raw = model.attemptTimeout(cfg.upstream), .clock = .awake }, + .total = .{ .raw = model.totalTimeout(cfg.upstream), .clock = .awake }, }; // The pool jitters backoff from this; one probe per upstream never reaches // backoff, so the value only has to be a value. @@ -978,7 +980,7 @@ fn probeUpstreams(r: Runner, cfg: model.Config) !usize { .enabled = true, .health = .init, }}; - var single: pool.Pool = .init(&entries, .{}, attempt_timeout, seed); + var single: pool.Pool = .init(&entries, .{}, timeouts, seed); if (single.exchange(r.io, probe_query, response_buf)) |_| { try r.out.print("OK upstreams[{d}] {f}\n", .{ i, safe_url.redact(server.url) }); diff --git a/src/config/export.zig b/src/config/export.zig index a4f0534..6fcb486 100644 --- a/src/config/export.zig +++ b/src/config/export.zig @@ -161,7 +161,7 @@ const seed_source: [:0]const u8 = \\ .groups = .{ .{ .name = "default" }, .{ .name = "kids", .safe_search = true } }, \\ .upstreams = .{ \\ .{ .url = "https://dns.example/dns-query", .priority = 10 }, - \\ .{ .url = "tls://dot.example:853", .priority = 20, .enabled = false, .tls_name = "dot.example" }, + \\ .{ .url = "tls://192.0.2.53:853", .priority = 20, .enabled = false, .tls_name = "dot.example" }, \\ }, \\ .clients = .{ .{ .ip = "fd00::1", .name = "tablet", .group = "kids" } }, \\ .client_prefixes = .{ .{ .prefix = "192.168.1.0/24", .group = "kids", .priority = 50 } }, diff --git a/src/config/import.zig b/src/config/import.zig index 2b0f5c4..5c47c0c 100644 --- a/src/config/import.zig +++ b/src/config/import.zig @@ -507,7 +507,7 @@ const full_source: [:0]const u8 = \\ .groups = .{ .{ .name = "default" }, .{ .name = "kids", .safe_search = true } }, \\ .upstreams = .{ \\ .{ .url = "https://dns.example/dns-query", .priority = 10 }, - \\ .{ .url = "tls://dot.example:853", .priority = 20, .enabled = false, .tls_name = "dot.example" }, + \\ .{ .url = "tls://192.0.2.53:853", .priority = 20, .enabled = false, .tls_name = "dot.example" }, \\ }, \\ .clients = .{ .{ .ip = "FD00:0:0:0:0:0:0:1", .name = "tablet", .group = "kids" } }, \\ .client_prefixes = .{ .{ .prefix = "192.168.1.0/24", .group = "kids", .priority = 50 } }, diff --git a/src/config/model.zig b/src/config/model.zig index 6f2f8ff..8c2a3e9 100644 --- a/src/config/model.zig +++ b/src/config/model.zig @@ -42,7 +42,14 @@ pub const Config = struct { }; pub const Upstream = struct { + /// Bounds one attempt against one upstream inside the pool's failover loop. + attempt_timeout_ms: u32 = 2500, + /// The forward-zone client's read deadline, and nothing else. It bounds a + /// different subsystem from the two above (`src/local/forward_client.zig`), + /// so no cross-check relates it to them. read_timeout_ms: u32 = 3000, + /// The whole-exchange budget: every failover attempt together, not one of + /// them. The pool races the entire loop against it. total_timeout_ms: u32 = 5000, }; @@ -91,6 +98,39 @@ pub const Web = struct { /// abuse the limiter defends against (PLAN §10). api_localhost_exempt: bool = true, sse_max_connections_per_ip: u16 = 3, + /// Comma-separated IP literals. A request arriving from one of these peers + /// is identified by the last entry of its `X-Forwarded-For` header instead + /// of by the socket peer, so the API limiter and the per-address SSE cap + /// bind the real client rather than the proxy. Empty means no proxy is + /// trusted and the socket peer is always the client. One string rather than + /// a list because the settings codec stores scalars only. + trusted_proxies: []const u8 = "", +}; + +/// Walks `web.trusted_proxies`. The validator and the web server both read the +/// setting, so what counts as one element — comma-separated, surrounding spaces +/// and tabs trimmed — is spelled out once here. An empty setting yields nothing; +/// an empty element (a stray comma) is yielded, so the validator can name it +/// rather than silently drop it. +pub fn trustedProxies(text: []const u8) TrustedProxyIterator { + return .{ .rest = text, .done = text.len == 0 }; +} + +pub const TrustedProxyIterator = struct { + rest: []const u8, + done: bool, + + pub fn next(self: *TrustedProxyIterator) ?[]const u8 { + if (self.done) return null; + const end = std.mem.findScalar(u8, self.rest, ',') orelse { + const last = self.rest; + self.done = true; + return std.mem.trim(u8, last, " \t"); + }; + const element = self.rest[0..end]; + self.rest = self.rest[end + 1 ..]; + return std.mem.trim(u8, element, " \t"); + } }; pub const TlsEndpoint = struct { @@ -300,6 +340,10 @@ pub fn readTimeout(u: Upstream) std.Io.Duration { return .{ .nanoseconds = @as(i96, u.read_timeout_ms) * std.time.ns_per_ms }; } +pub fn attemptTimeout(u: Upstream) std.Io.Duration { + return .{ .nanoseconds = @as(i96, u.attempt_timeout_ms) * std.time.ns_per_ms }; +} + pub fn totalTimeout(u: Upstream) std.Io.Duration { return .{ .nanoseconds = @as(i96, u.total_timeout_ms) * std.time.ns_per_ms }; } @@ -480,6 +524,7 @@ const expected_keys = [_][]const u8{ "logging.output", "logging.query_log_buffer_max", "logging.retention_days", + "upstream.attempt_timeout_ms", "upstream.read_timeout_ms", "upstream.total_timeout_ms", "web.api_localhost_exempt", @@ -490,6 +535,7 @@ const expected_keys = [_][]const u8{ "web.port", "web.session_ttl_hours", "web.sse_max_connections_per_ip", + "web.trusted_proxies", }; fn lessThanKey(_: void, a: SettingPair, b: SettingPair) bool { @@ -530,7 +576,7 @@ test "toSettings never emits web.password" { test "toSettings and fromSettings round-trip a non-default config" { const gpa = testing.allocator; const original: Config = .{ - .upstream = .{ .read_timeout_ms = 222, .total_timeout_ms = 333 }, + .upstream = .{ .attempt_timeout_ms = 111, .read_timeout_ms = 222, .total_timeout_ms = 333 }, .dns = .{ .bind_ipv4 = "127.0.0.1", .bind_ipv6 = "::1", @@ -549,6 +595,7 @@ test "toSettings and fromSettings round-trip a non-default config" { .api_rate_limit_per_min = 29, .api_localhost_exempt = false, .sse_max_connections_per_ip = 31, + .trusted_proxies = "10.0.0.9,fd00::9", }, .doh_server = .{ .enabled = true, @@ -685,6 +732,10 @@ test "RecordType stores the uppercase DDL spelling" { } test "unit conversions" { + try testing.expectEqual( + @as(i96, 2500) * std.time.ns_per_ms, + attemptTimeout(.{}).nanoseconds, + ); try testing.expectEqual( @as(i96, 3000) * std.time.ns_per_ms, readTimeout(.{}).nanoseconds, @@ -703,6 +754,7 @@ test "unit conversions" { test "unit conversions at the field maximum do not overflow" { const max_upstream: Upstream = .{ + .attempt_timeout_ms = std.math.maxInt(u32), .read_timeout_ms = std.math.maxInt(u32), .total_timeout_ms = std.math.maxInt(u32), }; @@ -710,6 +762,10 @@ test "unit conversions at the field maximum do not overflow" { @as(i96, std.math.maxInt(u32)) * std.time.ns_per_ms, readTimeout(max_upstream).nanoseconds, ); + try testing.expectEqual( + @as(i96, std.math.maxInt(u32)) * std.time.ns_per_ms, + attemptTimeout(max_upstream).nanoseconds, + ); try testing.expectEqual( @as(i64, std.math.maxInt(u16)) * 3600, sessionTtlSeconds(.{ .session_ttl_hours = std.math.maxInt(u16) }), diff --git a/src/config/validate.zig b/src/config/validate.zig index 29e98fb..1952cb8 100644 --- a/src/config/validate.zig +++ b/src/config/validate.zig @@ -63,6 +63,7 @@ const Prefix = address.Prefix; pub const ValidateError = error{ NoUpstreams, BadUpstreamUrl, + UpstreamHostNotIpLiteral, DuplicateUpstreamUrl, BadTlsName, TlsNameOnNonTlsUpstream, @@ -89,11 +90,13 @@ pub const ValidateError = error{ BadPort, BadTimeout, BadTtl, + BadCacheSize, BadRetention, BadLogRotation, BadDiskThresholds, BadRateLimit, BadBindAddress, + BadTrustedProxy, MissingCertPath, MissingKeyPath, MissingLogPath, @@ -310,17 +313,31 @@ const max_ttl_seconds = 86_400; const max_record_ttl_seconds = 604_800; const max_rate_window_seconds = 3_600; +/// The ceiling on every entry count nxdns allocates in full at boot: the query +/// log's ring and the DNS cache's slot array. Neither is grown on demand, so an +/// unchecked `maxInt(u32)` is a multi-terabyte allocation request during +/// startup — a `maxInt(u32)` query-log ring alone asks for about 1.8 TB. This +/// is a sanity bound, not a memory-fit guarantee: what actually fits depends on +/// the box, and nxdns does not try to know that. +const max_boot_entries = 1_000_000; + fn checkScalars(cfg: Config, diags: *Diagnostics) error{OutOfMemory}!void { const up = cfg.upstream; + try checkTimeout(diags, up.attempt_timeout_ms, "upstream.attempt_timeout_ms"); try checkTimeout(diags, up.read_timeout_ms, "upstream.read_timeout_ms"); try checkTimeout(diags, up.total_timeout_ms, "upstream.total_timeout_ms"); - if (up.total_timeout_ms < up.read_timeout_ms) { + // The only cross-check that relates two knobs of one subsystem: the pool + // races one attempt against `attempt` and the whole failover loop against + // `total`, so an attempt budget above the total one can never be reached. + // `read_timeout_ms` belongs to the forward-zone client and is deliberately + // unrelated to both. + if (up.attempt_timeout_ms > up.total_timeout_ms) { try diags.add( error.BadTimeout, - "upstream.total_timeout_ms", + "upstream.attempt_timeout_ms", .{}, - "total budget {d}ms is below read {d}ms", - .{ up.total_timeout_ms, up.read_timeout_ms }, + "attempt budget {d}ms is above the total budget {d}ms", + .{ up.attempt_timeout_ms, up.total_timeout_ms }, ); } @@ -350,6 +367,19 @@ fn checkScalars(cfg: Config, diags: *Diagnostics) error{OutOfMemory}!void { ); } + if (cfg.cache.size < 1) { + try diags.add(error.BadCacheSize, "cache.size", .{}, "must be at least 1", .{}); + } + if (cfg.cache.size > max_boot_entries) { + try diags.add( + error.BadCacheSize, + "cache.size", + .{}, + "must be at most {d}, got {d}", + .{ max_boot_entries, cfg.cache.size }, + ); + } + if (cfg.cache.negative_ttl_max > max_ttl_seconds) { try diags.add( error.BadTtl, @@ -381,6 +411,7 @@ fn checkScalars(cfg: Config, diags: *Diagnostics) error{OutOfMemory}!void { if (cfg.web.sse_max_connections_per_ip < 1) { try diags.add(error.BadRateLimit, "web.sse_max_connections_per_ip", .{}, "must be at least 1", .{}); } + try checkTrustedProxies(diags, cfg.web.trusted_proxies); try checkTlsEndpoint(diags, cfg.doh_server, "doh_server"); try checkTlsEndpoint(diags, cfg.dot_server, "dot_server"); @@ -391,6 +422,15 @@ fn checkScalars(cfg: Config, diags: *Diagnostics) error{OutOfMemory}!void { if (cfg.logging.query_log_buffer_max < 1) { try diags.add(error.BadRetention, "logging.query_log_buffer_max", .{}, "must be at least 1", .{}); } + if (cfg.logging.query_log_buffer_max > max_boot_entries) { + try diags.add( + error.BadRetention, + "logging.query_log_buffer_max", + .{}, + "must be at most {d}, got {d}", + .{ max_boot_entries, cfg.logging.query_log_buffer_max }, + ); + } if (cfg.logging.max_size_mb < 1) { try diags.add(error.BadLogRotation, "logging.max_size_mb", .{}, "must be at least 1", .{}); } @@ -478,6 +518,25 @@ fn checkBind( } } +/// Every element must be an IP literal, because the web server compares the +/// socket peer against them byte for byte — a hostname here would silently +/// trust nothing, and an operator who wrote one would believe their proxy was +/// trusted while the API limiter kept seeing loopback. +fn checkTrustedProxies(diags: *Diagnostics, text: []const u8) error{OutOfMemory}!void { + var it = model.trustedProxies(text); + while (it.next()) |element| { + _ = NetAddress.parse(element) catch { + try diags.add( + error.BadTrustedProxy, + "web.trusted_proxies", + .{}, + "{f} is not an IP address; the list is comma-separated IP literals", + .{safe_url.quoteText(element)}, + ); + }; + } +} + fn checkTlsEndpoint( diags: *Diagnostics, endpoint: model.TlsEndpoint, @@ -573,6 +632,39 @@ fn checkTlsName( }; } +/// A `tls://` upstream's host must be an IP literal, because `DotClient` refuses +/// anything else on every dial (`upstream/dot_client.zig`'s `resolveAddress`: +/// resolving an upstream's own name is a bootstrap problem nxdns declines to +/// have). Without this check a hostname `tls://` config validates clean and then +/// fails at query time as a peer fault, which reads as "the upstream is down" +/// rather than "this line is wrong". +/// +/// The mirror is exact: `NetAddress.parse` is `net.IpAddress.parse(text, 0)` +/// (`platform/address.zig`), the same call the client makes, so the two cannot +/// disagree about what a literal is. Note this is not `parseResolver` above — +/// that one validates a forward zone's resolver, a different field. +/// +/// A `tls://` host is a hostname often enough that the message says what to do +/// instead rather than only what is wrong. +fn checkDotHost( + diags: *Diagnostics, + server: model.UpstreamServer, + endpoint: transport.Endpoint, + index: usize, +) error{OutOfMemory}!void { + if (endpoint.scheme != .dot) return; + _ = NetAddress.parse(endpoint.host) catch { + try diags.add( + error.UpstreamHostNotIpLiteral, + "upstreams[{d}].url", + .{index}, + "tls:// upstreams take an IP literal host; {f} names a host nxdns will not resolve. " ++ + "Use the address and put the name in tls_name, or configure the https:// form.", + .{safe_url.redactQuoted(server.url)}, + ); + }; +} + fn checkCollections(cfg: Config, diags: *Diagnostics, scratch: Allocator) error{OutOfMemory}!void { var group_names: IndexSet = .empty; var has_default = false; @@ -608,6 +700,7 @@ fn checkCollections(cfg: Config, diags: *Diagnostics, scratch: Allocator) error{ // `BadUpstreamUrl`: what its scheme would have been is unknown. if (transport.Endpoint.parse(server.url)) |endpoint| { try checkTlsName(diags, server, endpoint.scheme, i); + try checkDotHost(diags, server, endpoint, i); } else |_| { try diags.add( error.BadUpstreamUrl, @@ -1114,6 +1207,36 @@ test "error.DuplicateUpstreamUrl" { try expectProblem(cfg, error.DuplicateUpstreamUrl, "upstreams[1].url"); } +test "error.UpstreamHostNotIpLiteral on a hostname tls:// upstream" { + // The form an operator copies off the NextDNS setup page. It parses, so it + // used to validate clean and then fail every exchange as a peer fault. + var cfg = baseConfig(); + cfg.upstreams = &.{.{ .url = "tls://abcd12.dns.nextdns.io" }}; + try expectProblem(cfg, error.UpstreamHostNotIpLiteral, "upstreams[0].url"); + + // A tls_name does not excuse it: the name is what the certificate is + // matched against, not what nxdns dials. + var named = baseConfig(); + named.upstreams = &.{.{ .url = "tls://dns.quad9.net:853", .tls_name = "dns.quad9.net" }}; + try expectProblem(named, error.UpstreamHostNotIpLiteral, "upstreams[0].url"); +} + +test "an https:// upstream may name a host" { + // The check is scheme-specific: DoH resolves through the HTTP client, so a + // hostname there is the normal form. + var cfg = baseConfig(); + cfg.upstreams = &.{.{ .url = "https://dns.nextdns.io/abcd12" }}; + try expectClean(cfg); +} + +test "an IPv6 literal tls:// upstream validates cleanly" { + // `Endpoint.parse` strips the brackets, so the host reaching the check is + // exactly what the client hands to the address parser. + var cfg = baseConfig(); + cfg.upstreams = &.{.{ .url = "tls://[2620:fe::fe]:853", .tls_name = "dns.quad9.net" }}; + try expectClean(cfg); +} + test "a tls_name on a tls:// upstream validates cleanly" { var cfg = baseConfig(); cfg.upstreams = &.{.{ .url = "tls://1.1.1.1:853", .tls_name = "one.one.one.one" }}; @@ -1701,9 +1824,37 @@ test "error.BadTimeout" { cfg.upstream.read_timeout_ms = 10; try expectProblem(cfg, error.BadTimeout, "upstream.read_timeout_ms"); + var attempt = baseConfig(); + attempt.upstream.attempt_timeout_ms = 10; + try expectProblem(attempt, error.BadTimeout, "upstream.attempt_timeout_ms"); + + // An attempt budget above the total one can never be reached: the pool + // cancels the whole loop when the total expires. var budget = baseConfig(); - budget.upstream = .{ .read_timeout_ms = 4000, .total_timeout_ms = 1000 }; - try expectProblem(budget, error.BadTimeout, "upstream.total_timeout_ms"); + budget.upstream = .{ .attempt_timeout_ms = 4000, .total_timeout_ms = 1000 }; + try expectProblem(budget, error.BadTimeout, "upstream.attempt_timeout_ms"); + + // `read_timeout_ms` bounds the forward-zone client, a different subsystem, + // so it is deliberately free to sit above the pool's total budget. + var unrelated = baseConfig(); + unrelated.upstream = .{ .attempt_timeout_ms = 500, .read_timeout_ms = 9000, .total_timeout_ms = 1000 }; + try expectClean(unrelated); +} + +test "error.BadCacheSize" { + var zero = baseConfig(); + zero.cache.size = 0; + try expectProblem(zero, error.BadCacheSize, "cache.size"); + + // The slot array is allocated in full at boot, so an unchecked maxInt is a + // startup that asks the kernel for more memory than the box has. + var huge = baseConfig(); + huge.cache.size = std.math.maxInt(u32); + try expectProblem(huge, error.BadCacheSize, "cache.size"); + + var edge = baseConfig(); + edge.cache.size = max_boot_entries; + try expectClean(edge); } test "error.BadTtl" { @@ -1720,6 +1871,16 @@ test "error.BadRetention" { var cfg = baseConfig(); cfg.logging.retention_days = 0; try expectProblem(cfg, error.BadRetention, "logging.retention_days"); + + var zero_buffer = baseConfig(); + zero_buffer.logging.query_log_buffer_max = 0; + try expectProblem(zero_buffer, error.BadRetention, "logging.query_log_buffer_max"); + + // The ring is allocated in full at boot; an Entry is a few hundred bytes, + // so maxInt(u32) asks for terabytes before the first query arrives. + var huge_buffer = baseConfig(); + huge_buffer.logging.query_log_buffer_max = 4_000_000_000; + try expectProblem(huge_buffer, error.BadRetention, "logging.query_log_buffer_max"); } test "error.BadLogRotation" { @@ -1756,6 +1917,26 @@ test "error.BadBindAddress on an IPv4 literal in dns.bind_ipv6" { try expectProblem(cfg, error.BadBindAddress, "dns.bind_ipv6"); } +test "error.BadTrustedProxy" { + var cfg = baseConfig(); + cfg.web.trusted_proxies = "127.0.0.1, proxy.example"; + try expectProblem(cfg, error.BadTrustedProxy, "web.trusted_proxies"); + + // A stray comma is named rather than dropped, so the operator sees the typo. + var trailing = baseConfig(); + trailing.web.trusted_proxies = "127.0.0.1,"; + try expectProblem(trailing, error.BadTrustedProxy, "web.trusted_proxies"); +} + +test "a list of IP literals, and an empty one, both validate" { + var cfg = baseConfig(); + cfg.web.trusted_proxies = " 127.0.0.1 , ::1,10.0.0.1 "; + try expectClean(cfg); + + cfg.web.trusted_proxies = ""; + try expectClean(cfg); +} + test "error.MissingCertPath" { var cfg = baseConfig(); cfg.doh_server = .{ .enabled = true, .cert_path = "" }; diff --git a/src/dns/edns.zig b/src/dns/edns.zig index 8ff95f8..f10768e 100644 --- a/src/dns/edns.zig +++ b/src/dns/edns.zig @@ -275,6 +275,25 @@ pub fn extendedRcode(header_rcode: types.Rcode, opt: ?OptRecord) u12 { return (@as(u12, o.extended_rcode) << 4) | low; } +/// The write-side pair of `extendedRcode`: splits a 12-bit RCODE into the four +/// bits the header carries and the eight the OPT record carries. A reply that +/// needs a code above 15 (BADVERS, RFC 6891 §6.1.3) has no other way to say it. +pub fn splitRcode(rcode: u12) SplitRcode { + return .{ + .header = @enumFromInt(@as(u4, @truncate(rcode))), + .extended = @truncate(rcode >> 4), + }; +} + +pub const SplitRcode = struct { + header: types.Rcode, + extended: u8, +}; + +/// RCODE 16, BADVERS (RFC 6891 §6.1.3): the query asked for an EDNS version +/// this server does not implement. +pub const badvers: u12 = 16; + const testing = std.testing; /// An OPT record with a 4096-byte payload size, DO set, and no options. @@ -700,3 +719,25 @@ test "extendedRcode composes the twelve bits" { high.extended_rcode = 0xff; try testing.expectEqual(@as(u12, 0xfff), extendedRcode(@enumFromInt(15), high)); } + +test "splitRcode splits BADVERS the way RFC 6891 does" { + const split = splitRcode(badvers); + try testing.expectEqual(types.Rcode.no_error, split.header); + try testing.expectEqual(@as(u8, 1), split.extended); +} + +test "splitRcode round-trips every twelve-bit rcode through extendedRcode" { + var rcode: u12 = 0; + while (true) : (rcode += 1) { + const split = splitRcode(rcode); + const opt: OptRecord = .{ + .udp_payload_size = 4096, + .extended_rcode = split.extended, + .version = 0, + .do_bit = false, + .options = .{ .offset = 0, .len = 0 }, + }; + try testing.expectEqual(rcode, extendedRcode(split.header, opt)); + if (rcode == std.math.maxInt(u12)) break; + } +} diff --git a/src/dns/packet.zig b/src/dns/packet.zig index e2a7266..39e2e09 100644 --- a/src/dns/packet.zig +++ b/src/dns/packet.zig @@ -261,10 +261,11 @@ pub fn decrementTtls(bytes: []u8, elapsed_seconds: u32) ParseError!?u32 { /// and which answers to add is the caller's policy. /// /// Sections must be filled in wire order, so every `addAnswer` call has to -/// precede `addOptEcho` — the OPT record belongs to the additional section, and -/// an answer written after it would land in the wrong section. `addOptEcho` -/// also runs at most once, because RFC 6891 §6.1.1 allows one OPT record per -/// message. Both rules are programmer errors, so both are assertions. +/// precede the OPT record — it belongs to the additional section, and an answer +/// written after it would land in the wrong section. Only one of `addOptEcho` +/// and `addOptWithRcode` runs, and only once, because RFC 6891 §6.1.1 allows +/// one OPT record per message. Both rules are programmer errors, so both are +/// assertions. pub const ResponseBuilder = struct { writer: Writer, header: header.Header, @@ -348,10 +349,23 @@ pub const ResponseBuilder = struct { /// comes back unchanged and the DO bit passes through. No options are /// echoed — nxdns implements none of them. pub fn addOptEcho(self: *ResponseBuilder, request_opt: edns.OptRecord, do_bit: bool) Error!void { + return self.addOptWithRcode(request_opt, do_bit, 0); + } + + /// `addOptEcho` with the upper eight bits of a 12-bit RCODE, which only the + /// OPT record can carry (RFC 6891 §6.1.3). Pair it with the header half from + /// `edns.splitRcode`. The reply's EDNS version stays 0 either way: it states + /// the highest version this server implements, not the version asked for. + pub fn addOptWithRcode( + self: *ResponseBuilder, + request_opt: edns.OptRecord, + do_bit: bool, + extended_rcode: u8, + ) Error!void { std.debug.assert(!self.opt_added); const opt: edns.OptRecord = .{ .udp_payload_size = request_opt.udp_payload_size, - .extended_rcode = 0, + .extended_rcode = extended_rcode, .version = 0, .do_bit = do_bit, .options = .{ .offset = 0, .len = 0 }, @@ -791,6 +805,37 @@ test "ResponseBuilder passes the DO bit through" { } } +test "addOptWithRcode carries the upper eight bits of a twelve-bit rcode" { + const request = try parse(query_bytes); + const request_opt = try edns.parseOpt(query_bytes, findOptRecord(request).?); + const split = edns.splitRcode(edns.badvers); + + var buf: [128]u8 = undefined; + var b = try ResponseBuilder.init(&buf, request.header, firstQuestion(request).?); + b.setRcode(split.header); + try b.addOptWithRcode(request_opt, false, split.extended); + const bytes = b.finish(); + + const p = try parse(bytes); + const opt = try edns.parseOpt(bytes, findOptRecord(p).?); + try testing.expectEqual(@as(u8, 0), opt.version); + try testing.expectEqual(@as(u12, edns.badvers), edns.extendedRcode(p.header.flags.rcode, opt)); +} + +test "addOptEcho leaves the extended rcode clear" { + const request = try parse(query_bytes); + const request_opt = try edns.parseOpt(query_bytes, findOptRecord(request).?); + + var buf: [128]u8 = undefined; + var b = try ResponseBuilder.init(&buf, request.header, firstQuestion(request).?); + try b.addOptEcho(request_opt, false); + const bytes = b.finish(); + + const p = try parse(bytes); + const opt = try edns.parseOpt(bytes, findOptRecord(p).?); + try testing.expectEqual(@as(u8, 0), opt.extended_rcode); +} + test "ResponseBuilder sets an rcode and an empty answer section" { const request = try parse(query_bytes); diff --git a/src/docs_drift_test.zig b/src/docs_drift_test.zig index fc5d9ca..33de814 100644 --- a/src/docs_drift_test.zig +++ b/src/docs_drift_test.zig @@ -4,6 +4,7 @@ //! contract as openapi.zig's route guard. const std = @import("std"); +const build_options = @import("build_options"); const docs = @import("docs_files"); const cli = @import("cli.zig"); const routes = @import("web/routes.zig"); @@ -61,3 +62,33 @@ test "every cli subcommand has its own reference heading in docs/reference/cli.m } } } + +test "no doc page pastes a concrete version into a transcript" { + const gpa = std.testing.allocator; + // Anchored on the `nxdns ` prefix the transcripts print, not on the bare + // version: a release numbered 1.0.0 would otherwise collide with the + // 1.0.0.1 upstream address the pages use as an example. + const built = try std.fmt.allocPrint(gpa, "nxdns {s}", .{build_options.version_string}); + defer gpa.free(built); + + for (docs.pages) |page| { + // The literal milestone 14 flagged, checked by name as well, so the + // guard still bites on a page written against the old default after + // `-Dversion-string` moves on. + for ([_][]const u8{ built, "0.1.0-dev" }) |needle| { + if (std.mem.indexOf(u8, page.text, needle) != null) { + std.debug.print("version literal '{s}' in {s}: use the placeholder\n", .{ needle, page.path }); + return error.VersionLiteralInDocPage; + } + } + } +} + +test "every version transcript prints the placeholder" { + for (docs.version_transcript_pages) |page| { + if (std.mem.indexOf(u8, page.text, "nxdns ") == null) { + std.debug.print("transcript placeholder 'nxdns ' missing from {s}\n", .{page.path}); + return error.VersionPlaceholderMissingFromDocPage; + } + } +} diff --git a/src/server/handler.zig b/src/server/handler.zig index f2e35b9..602f636 100644 --- a/src/server/handler.zig +++ b/src/server/handler.zig @@ -133,6 +133,9 @@ pub const Handler = struct { dropped_malformed: std.atomic.Value(u64) = .init(0), formerr: std.atomic.Value(u64) = .init(0), notimp: std.atomic.Value(u64) = .init(0), + /// Queries asking for an EDNS version this server does not implement, + /// answered with BADVERS (RFC 6891 §6.1.3). + badvers: std.atomic.Value(u64) = .init(0), servfail: std.atomic.Value(u64) = .init(0), truncated: std.atomic.Value(u64) = .init(0), refused: std.atomic.Value(u64) = .init(0), @@ -239,6 +242,19 @@ pub const Handler = struct { else null; + // RFC 6891 §6.1.3: a query naming an EDNS version this server does not + // implement is answered with BADVERS, and the reply's OPT reports the + // highest version the server does implement. The check precedes the + // opcode check because the version governs the whole EDNS exchange, + // whatever the query asks for. + if (opt) |o| { + if (o.version != 0) { + bump(&self.stats.badvers); + const echo = if (hdr.qdcount == 1) packet.firstQuestion(p) else null; + return .{ .reply = buildBadvers(hdr, echo, o, response_buf) }; + } + } + if (hdr.flags.opcode != .query) { return synthesize(hdr, null, opt, .not_imp, &self.stats.notimp, response_buf); } @@ -887,6 +903,22 @@ fn build( return b.finish(); } +/// Encodes a BADVERS reply. `build` cannot: RCODE 16 does not fit the header's +/// four bits, so the code splits across the header and the OPT record and the +/// reply carries an OPT even though it echoes none of the query's options. +fn buildBadvers( + hdr: header.Header, + q: ?question.Question, + request_opt: edns.OptRecord, + buf: []u8, +) []u8 { + const split = edns.splitRcode(edns.badvers); + var b = packet.ResponseBuilder.init(buf, hdr, q) catch unreachable; + b.setRcode(split.header); + b.addOptWithRcode(request_opt, request_opt.do_bit, split.extended) catch unreachable; + return b.finish(); +} + fn bump(counter: *std.atomic.Value(u64)) void { _ = counter.fetchAdd(1, .monotonic); } @@ -972,8 +1004,22 @@ const response_bytes = const opt_len = 11; const query_with_opt_len = query_bytes.len + opt_len; -/// `query_bytes` plus an OPT record advertising `payload_size`. +/// The EDNS version byte, counted from the start of the OPT record: past the +/// root owner name, TYPE, CLASS and the extended-RCODE byte. +const opt_version_offset = 6; + +/// `query_bytes` plus an OPT record advertising `payload_size`, EDNS version 0. fn queryWithOpt(buf: *[query_with_opt_len]u8, payload_size: u16, do_bit: bool) []const u8 { + return queryWithOptVersion(buf, payload_size, do_bit, 0); +} + +/// `queryWithOpt` for a chosen EDNS version. +fn queryWithOptVersion( + buf: *[query_with_opt_len]u8, + payload_size: u16, + do_bit: bool, + version: u8, +) []const u8 { @memcpy(buf[0..query_bytes.len], query_bytes); std.mem.writeInt(u16, buf[10..12], 1, .big); // arcount @@ -981,6 +1027,7 @@ fn queryWithOpt(buf: *[query_with_opt_len]u8, payload_size: u16, do_bit: bool) [ @memset(opt, 0); opt[2] = @intFromEnum(types.Type.opt); std.mem.writeInt(u16, opt[3..5], payload_size, .big); + opt[opt_version_offset] = version; if (do_bit) opt[7] = 0x80; // the DO bit is bit 15 of the TTL word return buf; } @@ -1414,6 +1461,80 @@ test "the truncated reply echoes the OPT record" { try testing.expectEqual(@as(u64, 1), h.stats.truncated.load(.monotonic)); } +test "an EDNS version this server does not implement gets BADVERS" { + var t: TestIo = .init(); + defer t.deinit(); + + var fake: FakeUpstream = .{ .reply = response_bytes }; + var h = bare(fake.client()); + + var query_buf: [query_with_opt_len]u8 = undefined; + const query = queryWithOptVersion(&query_buf, 1232, true, 1); + + var buf: [udp_limit_min]u8 = undefined; + const reply = try expectReply(udp(&h, t.io(), query, &buf)); + + const p = try packet.parse(reply); + const opt = try edns.parseOpt(reply, packet.findOptRecord(p).?); + // RCODE 16 lives in neither half alone: the header carries 0 and the OPT + // record carries 1. + try testing.expectEqual(types.Rcode.no_error, p.header.flags.rcode); + try testing.expectEqual(@as(u12, 16), edns.extendedRcode(p.header.flags.rcode, opt)); + try testing.expectEqual(@as(u8, 0), opt.version); + try testing.expectEqual(true, opt.do_bit); + try testing.expectEqual(@as(u16, 1232), opt.udp_payload_size); + try testing.expectEqual(@as(u16, 0x1234), p.header.id); + try testing.expectEqual(true, p.header.flags.qr); + try testing.expectEqual(@as(u16, 1), p.header.qdcount); + try testing.expectEqual(@as(u16, 0), p.header.ancount); + try testing.expectEqual(@as(u64, 1), h.stats.badvers.load(.monotonic)); + // The query never reached the upstream. + try testing.expectEqual(@as(u64, 0), h.stats.queries.load(.monotonic)); +} + +test "an EDNS version 0 query is not answered with BADVERS" { + var t: TestIo = .init(); + defer t.deinit(); + + var fake: FakeUpstream = .{ .reply = response_bytes }; + var h = bare(fake.client()); + + var query_buf: [query_with_opt_len]u8 = undefined; + const query = queryWithOptVersion(&query_buf, 1232, false, 0); + + var buf: [udp_limit_min]u8 = undefined; + const reply = try expectReply(udp(&h, t.io(), query, &buf)); + + // The query is forwarded and the upstream's own answer comes back, so the + // BADVERS arm never ran. + const p = try packet.parse(reply); + try testing.expectEqual(types.Rcode.no_error, p.header.flags.rcode); + try testing.expectEqual(@as(u16, 1), p.header.ancount); + try testing.expectEqual(@as(u64, 0), h.stats.badvers.load(.monotonic)); + try testing.expectEqual(@as(u64, 1), h.stats.queries.load(.monotonic)); +} + +test "a malformed OPT record is FORMERR before the version check reads it" { + var t: TestIo = .init(); + defer t.deinit(); + + var fake: FakeUpstream = .{ .reply = response_bytes }; + var h = bare(fake.client()); + + // The option list is broken and the EDNS version is 1. `parseOpt` fails + // first, so nothing trusts the version byte of an OPT that will not parse. + var query_buf: [query_with_bad_option.len]u8 = (query_with_bad_option ++ "").*; + query_buf[query_bytes.len + opt_version_offset] = 1; + + var buf: [udp_limit_min]u8 = undefined; + const reply = try expectReply(udp(&h, t.io(), &query_buf, &buf)); + + const p = try packet.parse(reply); + try testing.expectEqual(types.Rcode.form_err, p.header.flags.rcode); + try testing.expectEqual(@as(u64, 0), h.stats.badvers.load(.monotonic)); + try testing.expectEqual(@as(u64, 1), h.stats.formerr.load(.monotonic)); +} + test "an OPT record with a non-root owner name gets FORMERR" { var t: TestIo = .init(); defer t.deinit(); diff --git a/src/server/rate_limiter.zig b/src/server/rate_limiter.zig index 5d70493..8ff59f4 100644 --- a/src/server/rate_limiter.zig +++ b/src/server/rate_limiter.zig @@ -2,8 +2,11 @@ //! the timestamp, so this file holds no clock, no `std.Io` operation and no //! socket. `check` neither allocates nor fails. //! -//! Not thread-safe. Phase 7 decides the locking when it wires the limiter into -//! the query path. +//! Not thread-safe. The handler owns the locking: every call goes through +//! `Handler.limiter_mutex` (handler.zig:124), taken uncancelably on the query +//! path — `handle` has no error union to carry `error.Canceled` out of — and +//! cancelably in the `runMaintenance` sweep that ages the table out +//! (app.zig, `maintenanceOnce`), which does. //! //! The window is fixed, not sliding (PLAN §10 reserves the token bucket for the //! API limiter). A fixed window admits at most twice the limit across a window @@ -116,7 +119,7 @@ pub const RateLimiter = struct { /// Drops every entry whose window ended more than one full window before /// `now`, that is `now - start_ns > 2 * window_ns`. Returns how many it - /// dropped. Phase 7 schedules it. + /// dropped. `app.runMaintenance` schedules it. pub fn sweep(self: *RateLimiter, now: std.Io.Timestamp) u32 { const stale_after = 2 * self.window_ns; var stale_count: u32 = 0; diff --git a/src/server/resolver_integration_test.zig b/src/server/resolver_integration_test.zig index 4274344..f741dc9 100644 --- a/src/server/resolver_integration_test.zig +++ b/src/server/resolver_integration_test.zig @@ -77,9 +77,12 @@ const test_cfg: health.Config = .{ .max_backoff_ms = 60_000, }; -/// Nothing in this test is slow, so this budget only exists to stop a wedged +/// Nothing in this test is slow, so these budgets only exist to stop a wedged /// attempt from hanging the run. -const attempt_timeout: std.Io.Clock.Duration = .{ .raw = .fromSeconds(10), .clock = .awake }; +const pool_timeouts: pool.Timeouts = .{ + .attempt = .{ .raw = .fromSeconds(10), .clock = .awake }, + .total = .{ .raw = .fromSeconds(30), .clock = .awake }, +}; fn queryWithId(buf: *[query_bytes.len]u8, id: u16) []const u8 { buf.* = query_bytes.*; @@ -247,7 +250,7 @@ test "the whole resolver answers over udp and tcp and fails over to a healthy up testEntry("https://bad.example/dns-query", bad.client(), 10), testEntry("tls://good.example", good.client(), 20), }; - var upstreams: pool.Pool = .init(&entries, test_cfg, attempt_timeout, 1); + var upstreams: pool.Pool = .init(&entries, test_cfg, pool_timeouts, 1); var h = bareHandler(upstreams.client()); diff --git a/src/server/shutdown.zig b/src/server/shutdown.zig index 9110199..ebd7c2b 100644 --- a/src/server/shutdown.zig +++ b/src/server/shutdown.zig @@ -58,7 +58,8 @@ pub fn wait(io: std.Io) std.Io.Cancelable!void { } /// The programmatic equivalent of the signal: what a test uses to shut the app -/// down, and what a Phase 8 restart endpoint would call. +/// down. No restart endpoint calls it — none exists, and none is planned; the +/// web API echoes `restart_required` and leaves the restart to the operator. pub fn trigger(io: std.Io) void { event.set(io); } diff --git a/src/storage/storage_integration_test.zig b/src/storage/storage_integration_test.zig index b69b83c..27409ef 100644 --- a/src/storage/storage_integration_test.zig +++ b/src/storage/storage_integration_test.zig @@ -295,7 +295,7 @@ const rich_config = \\ .groups = .{ .{ .name = "default" }, .{ .name = "kids", .safe_search = true } }, \\ .upstreams = .{ \\ .{ .url = "https://dns.example/dns-query", .priority = 10 }, - \\ .{ .url = "tls://dot.example:853", .priority = 20, .enabled = false }, + \\ .{ .url = "tls://192.0.2.53:853", .priority = 20, .enabled = false }, \\ }, \\ .clients = .{ .{ .ip = "fd00::1", .name = "tablet", .group = "kids" } }, \\ .client_prefixes = .{ .{ .prefix = "192.168.1.0/24", .group = "kids", .priority = 50 } }, diff --git a/src/upstream/pool.zig b/src/upstream/pool.zig index 64e171e..0dedc0c 100644 --- a/src/upstream/pool.zig +++ b/src/upstream/pool.zig @@ -1,9 +1,17 @@ -//! Priority-ordered sequential failover across upstream endpoints, with a -//! per-attempt deadline, health tracking and backoff (PLAN §9). +//! Priority-ordered sequential failover across upstream endpoints, with two +//! deadlines, health tracking and backoff (PLAN §9). //! //! The pool is itself a `transport.Client`, so the handler above it sees one //! interface and knows nothing about how many upstreams exist. //! +//! Two deadlines, nested. `timeouts.attempt` bounds one exchange against one +//! entry; `timeouts.total` bounds the whole failover loop, every attempt +//! together. The outer one is what the client asking the question actually +//! waits for: without it, N unreachable upstreams cost N × attempt, and the +//! resolver above has already given up. The outer expiry is `error.Timeout` +//! unconditionally — the fast-failure paths (no entry enabled) return long +//! before the budget, so an expiry always means an attempt was in flight. +//! //! Two passes, not one. Pass one walks the enabled entries that health says are //! available. Pass two runs only when pass one attempted nothing, and skips the //! backoff check: every endpoint being in backoff must not turn into SERVFAIL @@ -81,7 +89,7 @@ pub const Entry = struct { }; /// A copy of one entry's health, taken under the mutex. Feeds -/// `GET /api/upstream/health` in Phase 8. +/// `GET /api/upstream/health`. pub const Snapshot = struct { /// Whole, not redacted. `GET /api/upstream/health` returns this to a session /// that `GET /api/upstreams` already serves the same url to in full, so @@ -102,19 +110,30 @@ pub const Snapshot = struct { backoff_until: ?std.Io.Timestamp, }; +/// The two budgets, named rather than positional: they are the same type, so +/// two parameters in a row could be swapped at a call site and still compile, +/// and the swap would be invisible until an operator watched a query take five +/// attempts of two and a half seconds each. +pub const Timeouts = struct { + /// Bounds one exchange against one entry. + attempt: std.Io.Clock.Duration, + /// Bounds the whole failover loop. + total: std.Io.Clock.Duration, +}; + pub const Pool = struct { /// Caller-owned, sorted ascending by priority in `init`. entries: []Entry, cfg: health.Config, - /// On the `.awake` clock, so a suspended Pi does not burn the budget. - attempt_timeout: std.Io.Clock.Duration, + /// Both on the `.awake` clock, so a suspended Pi does not burn a budget. + timeouts: Timeouts, mutex: std.Io.Mutex, rng: std.Random.DefaultPrng, pub fn init( entries: []Entry, cfg: health.Config, - attempt_timeout: std.Io.Clock.Duration, + timeouts: Timeouts, seed: u64, ) Pool { std.debug.assert(entries.len > 0); @@ -123,7 +142,7 @@ pub const Pool = struct { return .{ .entries = entries, .cfg = cfg, - .attempt_timeout = attempt_timeout, + .timeouts = timeouts, .mutex = .init, .rng = .init(seed), }; @@ -150,12 +169,52 @@ pub const Pool = struct { /// `response_buf` is handed to each attempt in turn, so a failed attempt /// may have written into it. The returned slice is only meaningful on /// success; on error the buffer's contents are undefined. + /// + /// The failover loop runs raced against `timeouts.total`. Losing that race + /// cancels the loop, which unwinds whatever attempt was in flight: the + /// `Entry.busy` lock is taken cancelably on purpose and released by defer, + /// and the health bookkeeping around a completed exchange is uncancelable, + /// so a canceled attempt leaves no lock held and no counter half-written. pub fn exchange( self: *Pool, io: std.Io, query: []const u8, response_buf: []u8, ) transport.ExchangeError![]u8 { + var outcomes: [2]LoopOutcome = undefined; + var race: std.Io.Select(LoopOutcome) = .init(io, &outcomes); + defer race.cancelDiscard(); + + race.concurrent(.loop, exchangeLoopLen, .{ + self, io, query, response_buf, + }) catch |err| switch (err) { + error.ConcurrencyUnavailable => return error.SystemResources, + }; + race.concurrent(.expiry, expire, .{ io, self.timeouts.total }) catch |err| switch (err) { + error.ConcurrencyUnavailable => return error.SystemResources, + }; + + switch (try race.await()) { + .loop => |result| return response_buf[0..try result], + .expiry => |result| { + // A canceled sleep means this whole task is being torn down, + // not that the budget ran out. + try result; + return error.Timeout; + }, + } + } + + /// The two-pass failover loop, as a raceable task. It returns the reply's + /// length rather than its slice for the reason `exchangeLen` in the tests + /// below does: `Io.concurrent` stores the future's return value, so the + /// bytes are read back out of the caller's `response_buf` by `exchange`. + fn exchangeLoopLen( + self: *Pool, + io: std.Io, + query: []const u8, + response_buf: []u8, + ) transport.ExchangeError!usize { const now = std.Io.Clock.awake.now(io); var last_fault: ?transport.ExchangeError = null; var attempted = false; @@ -170,8 +229,9 @@ pub const Pool = struct { // waiting its turn on a busy upstream has done nothing that a // cancellation could corrupt, so it gives up here rather than // queueing behind an exchange it will not use. The wait itself - // is bounded by the holder's `attempt_timeout`; the waiter's own - // budget only starts once it has the lock. + // is bounded by the holder's `timeouts.attempt`; the waiter's + // own budget only starts once it has the lock, and the whole + // loop is bounded by `timeouts.total` regardless. try entry.busy.lock(io); defer entry.busy.unlock(io); @@ -204,7 +264,7 @@ pub const Pool = struct { }; self.recordSuccess(io, entry, completed_at); - return response; + return response.len; } if (attempted) break; } @@ -259,7 +319,7 @@ pub const Pool = struct { }) catch |err| switch (err) { error.ConcurrencyUnavailable => return error.SystemResources, }; - race.concurrent(.expiry, expire, .{ io, self.attempt_timeout }) catch |err| switch (err) { + race.concurrent(.expiry, expire, .{ io, self.timeouts.attempt }) catch |err| switch (err) { error.ConcurrencyUnavailable => return error.SystemResources, }; @@ -312,6 +372,11 @@ const Outcome = union(enum) { expiry: std.Io.Cancelable!void, }; +const LoopOutcome = union(enum) { + loop: transport.ExchangeError!usize, + expiry: std.Io.Cancelable!void, +}; + fn expire(io: std.Io, duration: std.Io.Clock.Duration) std.Io.Cancelable!void { return duration.sleep(io); } @@ -410,7 +475,12 @@ const test_cfg: health.Config = .{ .max_backoff_ms = 60_000, }; -const test_timeout: std.Io.Clock.Duration = .{ .raw = .fromSeconds(10), .clock = .awake }; +/// Long enough that nothing in a test reaches either budget unless the test is +/// about a budget, and short enough that a stuck test still ends. +const test_timeouts: Timeouts = .{ + .attempt = .{ .raw = .fromSeconds(10), .clock = .awake }, + .total = .{ .raw = .fromSeconds(30), .clock = .awake }, +}; fn expectFailureLine(expected: []const u8, url: []const u8, err: transport.ExchangeError) !void { var buf: [8 * safe_url.max_len]u8 = undefined; @@ -451,7 +521,7 @@ test "Pool satisfies the Client interface" { var fake: Fake = .{ .behavior = .{ .reply = response_bytes } }; var entries = [_]Entry{testEntry("https://a.example/dns-query", &fake, 10)}; - var pool: Pool = .init(&entries, test_cfg, test_timeout, 1); + var pool: Pool = .init(&entries, test_cfg, test_timeouts, 1); var buf: [512]u8 = undefined; const reply = try pool.client().exchange(io, query_bytes, &buf); @@ -470,7 +540,7 @@ test "entries are tried in ascending priority order" { testEntry("https://high.example/dns-query", &high, 100), testEntry("https://low.example/dns-query", &low, 10), }; - var pool: Pool = .init(&entries, test_cfg, test_timeout, 1); + var pool: Pool = .init(&entries, test_cfg, test_timeouts, 1); try testing.expectEqual(@as(i32, 10), entries[0].priority); @@ -491,7 +561,7 @@ test "a peer fault fails over to the next entry and is recorded" { testEntry("https://bad.example/dns-query", &bad, 10), testEntry("https://good.example/dns-query", &good, 20), }; - var pool: Pool = .init(&entries, test_cfg, test_timeout, 1); + var pool: Pool = .init(&entries, test_cfg, test_timeouts, 1); var buf: [512]u8 = undefined; const reply = try pool.exchange(io, query_bytes, &buf); @@ -514,7 +584,7 @@ test "an entry in backoff is skipped while another is available" { testEntry("https://bad.example/dns-query", &bad, 10), testEntry("https://good.example/dns-query", &good, 20), }; - var pool: Pool = .init(&entries, test_cfg, test_timeout, 1); + var pool: Pool = .init(&entries, test_cfg, test_timeouts, 1); var buf: [512]u8 = undefined; // Two failures reach `failure_threshold` and open a backoff window. @@ -539,7 +609,7 @@ test "every entry in backoff is still probed" { testEntry("https://first.example/dns-query", &first, 10), testEntry("https://second.example/dns-query", &second, 20), }; - var pool: Pool = .init(&entries, test_cfg, test_timeout, 1); + var pool: Pool = .init(&entries, test_cfg, test_timeouts, 1); var buf: [512]u8 = undefined; try testing.expectError(error.BadResponse, pool.exchange(io, query_bytes, &buf)); @@ -564,7 +634,7 @@ test "a local resource error short-circuits and records nothing" { testEntry("https://broke.example/dns-query", &broke, 10), testEntry("https://good.example/dns-query", &good, 20), }; - var pool: Pool = .init(&entries, test_cfg, test_timeout, 1); + var pool: Pool = .init(&entries, test_cfg, test_timeouts, 1); var buf: [512]u8 = undefined; try testing.expectError(error.OutOfMemory, pool.exchange(io, query_bytes, &buf)); @@ -584,7 +654,7 @@ test "a cancellation short-circuits and records nothing" { testEntry("https://canceled.example/dns-query", &canceled, 10), testEntry("https://good.example/dns-query", &good, 20), }; - var pool: Pool = .init(&entries, test_cfg, test_timeout, 1); + var pool: Pool = .init(&entries, test_cfg, test_timeouts, 1); var buf: [512]u8 = undefined; try testing.expectError(error.Canceled, pool.exchange(io, query_bytes, &buf)); @@ -606,8 +676,12 @@ test "an attempt that outruns the budget is a recorded Timeout" { testEntry("https://slow.example/dns-query", &slow, 10), testEntry("https://good.example/dns-query", &good, 20), }; - const budget: std.Io.Clock.Duration = .{ .raw = .fromMilliseconds(20), .clock = .awake }; - var pool: Pool = .init(&entries, test_cfg, budget, 1); + // A tight attempt budget under a loose total one, so the attempt deadline + // is the only one that can fire and the failover after it has room to run. + var pool: Pool = .init(&entries, test_cfg, .{ + .attempt = .{ .raw = .fromMilliseconds(20), .clock = .awake }, + .total = .{ .raw = .fromSeconds(30), .clock = .awake }, + }, 1); var buf: [512]u8 = undefined; const reply = try pool.exchange(io, query_bytes, &buf); @@ -619,7 +693,47 @@ test "an attempt that outruns the budget is a recorded Timeout" { try testing.expectEqual(@as(u64, 1), entries[1].health.total_successes); } -test "every entry disabled yields ConnectFailed" { +test "two stalling upstreams cost the total budget, not one budget each" { + var threaded: std.Io.Threaded = .init(testing.allocator, .{}); + defer threaded.deinit(); + const io = threaded.io(); + + // Both entries stall past their own attempt budget, so without the outer + // race the exchange would take two attempt budgets and then some. The + // total is set below two attempts, which is what makes the assertion mean + // something: only the outer deadline can end this call in time. + var first: Fake = .{ .behavior = .{ .slow = .{ + .duration = .{ .raw = .fromSeconds(30), .clock = .awake }, + .reply = response_bytes, + } } }; + var second: Fake = .{ .behavior = .{ .slow = .{ + .duration = .{ .raw = .fromSeconds(30), .clock = .awake }, + .reply = response_bytes, + } } }; + var entries = [_]Entry{ + testEntry("https://first.example/dns-query", &first, 10), + testEntry("https://second.example/dns-query", &second, 20), + }; + var pool: Pool = .init(&entries, test_cfg, .{ + .attempt = .{ .raw = .fromMilliseconds(200), .clock = .awake }, + .total = .{ .raw = .fromMilliseconds(60), .clock = .awake }, + }, 1); + + var buf: [512]u8 = undefined; + const started = std.Io.Clock.awake.now(io); + try testing.expectError(error.Timeout, pool.exchange(io, query_bytes, &buf)); + const elapsed_ns = std.Io.Clock.awake.now(io).nanoseconds - started.nanoseconds; + + // Under one attempt budget, so the outer deadline is provably what fired. + // The bound is generous against a loaded CI box; the failure it catches is + // a whole extra attempt, not a scheduling hiccup. + try testing.expect(elapsed_ns < @as(i96, 200) * std.time.ns_per_ms); + // The second entry was never reached: the loop was canceled inside the + // first attempt. + try testing.expectEqual(@as(usize, 0), second.calls); +} + +test "every entry disabled yields ConnectFailed without waiting out the total budget" { var threaded: std.Io.Threaded = .init(testing.allocator, .{}); defer threaded.deinit(); const io = threaded.io(); @@ -631,10 +745,20 @@ test "every entry disabled yields ConnectFailed" { testEntry("https://two.example/dns-query", &two, 20), }; for (&entries) |*entry| entry.enabled = false; - var pool: Pool = .init(&entries, test_cfg, test_timeout, 1); + // A total budget long enough that waiting it out would be unmistakable. + // The outer expiry returns `error.Timeout` unconditionally, so this pins + // that the fast-failure path still wins its own race. + var pool: Pool = .init(&entries, test_cfg, .{ + .attempt = .{ .raw = .fromSeconds(10), .clock = .awake }, + .total = .{ .raw = .fromSeconds(30), .clock = .awake }, + }, 1); var buf: [512]u8 = undefined; + const started = std.Io.Clock.awake.now(io); try testing.expectError(error.ConnectFailed, pool.exchange(io, query_bytes, &buf)); + const elapsed_ns = std.Io.Clock.awake.now(io).nanoseconds - started.nanoseconds; + + try testing.expect(elapsed_ns < @as(i96, 5) * std.time.ns_per_s); try testing.expectEqual(@as(usize, 0), one.calls); try testing.expectEqual(@as(usize, 0), two.calls); } @@ -650,7 +774,7 @@ test "snapshot reports the counters in pool order" { testEntry("https://bad.example/dns-query", &bad, 10), testEntry("https://good.example/dns-query", &good, 20), }; - var pool: Pool = .init(&entries, test_cfg, test_timeout, 1); + var pool: Pool = .init(&entries, test_cfg, test_timeouts, 1); var buf: [512]u8 = undefined; _ = try pool.exchange(io, query_bytes, &buf); @@ -705,7 +829,7 @@ test "concurrent exchanges through one entry do not overlap" { .reply = response_bytes, } } }; var entries = [_]Entry{testEntry("https://only.example/dns-query", &fake, 10)}; - var pool: Pool = .init(&entries, test_cfg, test_timeout, 1); + var pool: Pool = .init(&entries, test_cfg, test_timeouts, 1); var buf_a: [512]u8 = undefined; var buf_b: [512]u8 = undefined; @@ -752,7 +876,7 @@ test "an entry that enters backoff while a task waits on it is not attempted" { testEntry("https://slow-bad.example/dns-query", &slow_bad, 10), testEntry("https://good.example/dns-query", &good, 20), }; - var pool: Pool = .init(&entries, cfg, test_timeout, 1); + var pool: Pool = .init(&entries, cfg, test_timeouts, 1); var buf_a: [512]u8 = undefined; var buf_b: [512]u8 = undefined; diff --git a/src/web/handlers/live.zig b/src/web/handlers/live.zig index 076f2c9..5a91df7 100644 --- a/src/web/handlers/live.zig +++ b/src/web/handlers/live.zig @@ -15,7 +15,9 @@ //! //! The route is rate-limit exempt (a long-lived stream must not drain its //! address's token bucket) but pays the per-address SSE connection cap, which -//! binds loopback too: hub slots are a fixed resource. +//! binds loopback too: hub slots are a fixed resource. The address it keys on is +//! `client_addr`, so behind a trusted proxy each remote client holds its own +//! budget rather than all of them sharing the proxy's. const std = @import("std"); @@ -79,12 +81,15 @@ pub fn stream( const hub = state.hub orelse return http_util.respondError(request, .service_unavailable, "live stream unavailable"); - const peer = address.NetAddress.fromIp(request.peer); + // The effective client, not the socket peer: behind a trusted proxy every + // stream would otherwise share one address's budget. Acquire and release + // read the same value, so a release can never miss the slot it took. + const client = address.NetAddress.fromIp(request.client_addr); if (state.limiter) |limiter| { - if (!limiter.tryAcquireSse(io, std.Io.Clock.awake.now(io), peer)) + if (!limiter.tryAcquireSse(io, std.Io.Clock.awake.now(io), client)) return http_util.respondError(request, .too_many_requests, "too many live streams from this address"); } - defer if (state.limiter) |limiter| limiter.releaseSse(io, peer); + defer if (state.limiter) |limiter| limiter.releaseSse(io, client); const id = hub.subscribe(io) orelse return http_util.respondError(request, .service_unavailable, "live stream is full"); diff --git a/src/web/handlers/settings.zig b/src/web/handlers/settings.zig index 2e218f6..e6f9dc9 100644 --- a/src/web/handlers/settings.zig +++ b/src/web/handlers/settings.zig @@ -213,6 +213,7 @@ const WebView = struct { api_rate_limit_per_min: u32, api_localhost_exempt: bool, sse_max_connections_per_ip: u16, + trusted_proxies: []const u8, /// Derived, not stored: the hash itself is never serialized, and the UI /// still has to know whether a password is set. auth_enabled: bool, @@ -246,6 +247,7 @@ pub fn view(cfg: model.Config) View { .api_rate_limit_per_min = cfg.web.api_rate_limit_per_min, .api_localhost_exempt = cfg.web.api_localhost_exempt, .sse_max_connections_per_ip = cfg.web.sse_max_connections_per_ip, + .trusted_proxies = cfg.web.trusted_proxies, .auth_enabled = auth.authEnabled(cfg.web), }, .doh_server = cfg.doh_server, diff --git a/src/web/handlers/upstream_health.zig b/src/web/handlers/upstream_health.zig index 922a0c6..c8c0f55 100644 --- a/src/web/handlers/upstream_health.zig +++ b/src/web/handlers/upstream_health.zig @@ -88,7 +88,10 @@ fn testEntry(url: []const u8, enabled: bool) pool_mod.Entry { } fn testPool(entries: []pool_mod.Entry) pool_mod.Pool { - return .init(entries, .{}, .{ .raw = .fromMilliseconds(50), .clock = .awake }, 1); + return .init(entries, .{}, .{ + .attempt = .{ .raw = .fromMilliseconds(50), .clock = .awake }, + .total = .{ .raw = .fromMilliseconds(100), .clock = .awake }, + }, 1); } test "every upstream is copied, counted and owned by the arena" { diff --git a/src/web/http_util.zig b/src/web/http_util.zig index 580893b..76ea239 100644 --- a/src/web/http_util.zig +++ b/src/web/http_util.zig @@ -38,6 +38,12 @@ pub const max_cookie_len: usize = 1024; /// reads. Both are short; an over-long one is treated as absent. pub const max_header_value_len: usize = 128; +/// How much of an `x-forwarded-for` header is kept. A trusted proxy appends its +/// own entry last, so the entry that names the real client always sits in the +/// final bytes; the copy therefore keeps the tail rather than the head (see +/// `server.copyHeaderSuffix`). 256 bytes hold an IPv6 entry many times over. +pub const max_xff_len: usize = 256; + /// A path deeper than this matches no route, so parsing can stop there. pub const max_path_segments: usize = 8; @@ -248,6 +254,17 @@ pub const Request = struct { accept_encoding: []const u8, if_none_match: []const u8, peer: net.IpAddress, + /// Who the request is from, as far as every policy decision is concerned: + /// the socket peer, unless that peer is a configured trusted proxy and the + /// request carried an `x-forwarded-for`, in which case it is that header's + /// last entry. Computed once per request by the router, because the header + /// it derives from dies on the first body read. + /// + /// Only the address is meaningful — a forwarded entry carries no port, so + /// the port reads 0 there. This file stays free of nxdns imports (the fuzz + /// target builds it as a module root), so the address type is the std one; + /// `address.NetAddress.fromIp` is what the policy sites key on. + client_addr: net.IpAddress, /// Reset between requests on the same connection. Nothing allocated here /// survives the response. arena: Allocator, diff --git a/src/web/metrics.zig b/src/web/metrics.zig index 7587241..4d50b49 100644 --- a/src/web/metrics.zig +++ b/src/web/metrics.zig @@ -730,6 +730,22 @@ test "every HELP line has a TYPE line and a sample, and every sample a name" { try testing.expectEqual(helps, samples); // `nxdns_up` plus every DNS counter: the families a bare state still has. try testing.expectEqual(1 + dns_stat_fields.len + 7, samples); + // The reflective walk names the counters, so a renamed `Handler.Stats` + // field silently renames a scraped series. Pin the ones an operator alerts + // on by name. + for ([_][]const u8{ + "nxdns_dns_queries_total", + "nxdns_dns_formerr_total", + "nxdns_dns_notimp_total", + "nxdns_dns_badvers_total", + "nxdns_dns_servfail_total", + "nxdns_dns_refused_total", + "nxdns_dns_blocked_total", + }) |metric| { + var line_buf: [128]u8 = undefined; + const line = try std.fmt.bufPrint(&line_buf, "# TYPE {s} counter\n", .{metric}); + try testing.expect(std.mem.containsAtLeast(u8, text, 1, line)); + } } test "an unwired collaborator omits its family rather than reporting zeros" { diff --git a/src/web/openapi.yaml b/src/web/openapi.yaml index c3f18d5..48aafa7 100644 --- a/src/web/openapi.yaml +++ b/src/web/openapi.yaml @@ -2092,8 +2092,9 @@ components: properties: upstream: type: object - required: [read_timeout_ms, total_timeout_ms] + required: [attempt_timeout_ms, read_timeout_ms, total_timeout_ms] properties: + attempt_timeout_ms: { type: integer } read_timeout_ms: { type: integer } total_timeout_ms: { type: integer } dns: @@ -2121,7 +2122,7 @@ components: negative_ttl_max: { type: integer } web: type: object - required: [enabled, bind, port, session_ttl_hours, api_rate_limit_per_min, api_localhost_exempt, sse_max_connections_per_ip, auth_enabled] + required: [enabled, bind, port, session_ttl_hours, api_rate_limit_per_min, api_localhost_exempt, sse_max_connections_per_ip, trusted_proxies, auth_enabled] properties: enabled: { type: boolean } bind: { type: string } @@ -2130,6 +2131,12 @@ components: api_rate_limit_per_min: { type: integer } api_localhost_exempt: { type: boolean } sse_max_connections_per_ip: { type: integer } + trusted_proxies: + type: string + description: | + Comma-separated IP literals. A request from one of these peers + is identified by the last entry of its X-Forwarded-For header. + Empty trusts no proxy. auth_enabled: type: boolean description: Derived, read-only; true iff a password hash is stored. @@ -2208,6 +2215,7 @@ components: upstream: type: object properties: + attempt_timeout_ms: { type: integer } read_timeout_ms: { type: integer } total_timeout_ms: { type: integer } dns: @@ -2243,6 +2251,7 @@ components: api_rate_limit_per_min: { type: integer } api_localhost_exempt: { type: boolean } sse_max_connections_per_ip: { type: integer } + trusted_proxies: { type: string } doh_server: $ref: "#/components/schemas/TlsListenerPatch" dot_server: diff --git a/src/web/server.zig b/src/web/server.zig index e1ef8b3..c6bd499 100644 --- a/src/web/server.zig +++ b/src/web/server.zig @@ -61,7 +61,7 @@ const log = std.log.scoped(.web_server); const recv_buffer_len = 8 * 1024; const send_buffer_len = 4 * 1024; -/// Ruling 7. 64 slots at ~15.7 KiB each is ~1 MiB of fixed connection state. +/// Ruling 7. 64 slots at ~16 KiB each is ~1 MiB of fixed connection state. pub const default_max_connections: u16 = 64; /// How much per-request arena a connection keeps between requests. Enough that @@ -206,10 +206,13 @@ pub fn sessionAuth(state: *WebState, io: std.Io, request: *const http_util.Reque /// Ruling 19. No limiter wired means no limit: the limiter is a defence the /// operator configures, and its absence must not refuse traffic. +/// +/// Keyed on `client_addr`, not on the socket peer: behind a trusted proxy every +/// peer is the proxy, and one bucket for every remote user is no limiter at all. pub fn bucketLimit(state: *WebState, io: std.Io, request: *const http_util.Request) LimitVerdict { const limiter = state.limiter orelse return .ok; const now = std.Io.Clock.awake.now(io); - return limiter.check(io, now, address.NetAddress.fromIp(request.peer)); + return limiter.check(io, now, address.NetAddress.fromIp(request.client_addr)); } /// Seam double: refuses nothing. For tests and for a server with no admin @@ -281,6 +284,7 @@ pub const Server = struct { cookie_buf: [http_util.max_cookie_len]u8, accept_encoding_buf: [http_util.max_header_value_len]u8, if_none_match_buf: [http_util.max_header_value_len]u8, + xff_buf: [http_util.max_xff_len]u8, /// Per-request working memory, reset between requests on the same /// connection so a keep-alive client cannot grow it without bound. arena: std.heap.ArenaAllocator, @@ -523,6 +527,19 @@ pub const Server = struct { const accept_encoding = copyHeader(request, "accept-encoding", &conn.accept_encoding_buf); const if_none_match = copyHeader(request, "if-none-match", &conn.if_none_match_buf); + const peer = address.NetAddress.fromIp(conn.peer); + const forwarded_for = copyHeaderSuffix(request, "x-forwarded-for", &conn.xff_buf); + const client_addr = switch (clientAddr(self.state.web.trusted_proxies, peer, forwarded_for)) { + .addr => |addr| addr, + .bad_forwarded_for => { + var view = bareRequest(request, conn, arena); + return http_util.respondError(&view, .bad_request, "malformed x-forwarded-for"); + }, + }; + // A forwarded entry has no port of its own; the peer keeps the one it + // connected from. + const client_ip = if (client_addr.eql(peer)) conn.peer else client_addr.toIp(0); + // Decoding is destructive, so it runs on a copy: W8's asset lookup needs // the raw path to match embedded file names byte for byte. const decodable = arena.dupe(u8, raw_path) catch return error.OutOfMemory; @@ -542,6 +559,7 @@ pub const Server = struct { .accept_encoding = accept_encoding, .if_none_match = if_none_match, .peer = conn.peer, + .client_addr = client_ip, .arena = arena, }; return router.dispatch(self.state, io, &view); @@ -560,6 +578,10 @@ pub const Server = struct { .accept_encoding = "", .if_none_match = "", .peer = conn.peer, + // These responses are decided before the forwarded-for header is + // read, or because reading it failed; the socket peer is all that + // is known. + .client_addr = conn.peer, .arena = arena, }; } @@ -629,6 +651,68 @@ fn copyHeader(request: *http.Server.Request, name: []const u8, buf: []u8) []cons return buf[0..value.len]; } +/// Copies the **last** `buf.len` bytes of one header value into `buf`. Null +/// means the header is absent, which is a different answer from an empty value. +/// +/// The tail is what matters for `x-forwarded-for`, and `copyHeader`'s +/// empty-on-overflow rule would be a security hole here: an empty value reads as +/// "no header", the effective client falls back to the socket peer, and behind +/// the same-box proxy this feature exists for that peer is loopback — which +/// `web.api_localhost_exempt` exempts from the API limiter by default. A client +/// would regain the exemption by sending an oversized header. Keeping the tail +/// closes that: the proxy appends its entry last, so the entry that names the +/// real client is always in the final bytes. +fn copyHeaderSuffix(request: *http.Server.Request, name: []const u8, buf: []u8) ?[]const u8 { + const value = headerValue(request, name) orelse return null; + const tail = if (value.len > buf.len) value[value.len - buf.len ..] else value; + @memcpy(buf[0..tail.len], tail); + return buf[0..tail.len]; +} + +/// Who a request is from, or the one way deriving that can fail. +const ClientAddr = union(enum) { + addr: address.NetAddress, + bad_forwarded_for, +}; + +/// Milestone-17 ruling 4. The socket peer, unless it is a trusted proxy that +/// forwarded the request, in which case the last entry of `x-forwarded-for` — +/// the one that proxy appended, and the only entry in the chain a client cannot +/// write. +/// +/// A missing header falls back to the peer: a trusted proxy that adds no header +/// is a proxy nobody asked to trust anything about, and the peer is still true. +/// A header that is present but whose last entry is not an IP literal fails +/// closed with a 400 instead. Only a misconfigured proxy can produce that — a +/// client's spoofed entries can never terminate the chain — and falling back +/// there would hand the request the loopback identity this ruling exists to +/// take away. +fn clientAddr( + trusted_proxies: []const u8, + peer: address.NetAddress, + forwarded_for: ?[]const u8, +) ClientAddr { + if (!trustsPeer(trusted_proxies, peer)) return .{ .addr = peer }; + const chain = forwarded_for orelse return .{ .addr = peer }; + + const start = if (std.mem.lastIndexOfScalar(u8, chain, ',')) |comma| comma + 1 else 0; + const last = std.mem.trim(u8, chain[start..], " \t"); + const addr = address.NetAddress.parse(last) catch return .bad_forwarded_for; + return .{ .addr = addr }; +} + +/// Whether `peer` is one of the configured trusted proxies. An element that is +/// not an IP literal matches nothing; `validate.zig` is what tells the operator +/// about it, and refusing to serve over a typo here would be a worse answer. +fn trustsPeer(trusted_proxies: []const u8, peer: address.NetAddress) bool { + var it = model.trustedProxies(trusted_proxies); + while (it.next()) |element| { + const trusted = address.NetAddress.parse(element) catch continue; + if (trusted.eql(peer)) return true; + } + return false; +} + /// The first value sent under `name`, borrowed from the request head. fn headerValue(request: *http.Server.Request, name: []const u8) ?[]const u8 { var it = request.iterateHeaders(); @@ -825,6 +909,75 @@ fn testRequest() http_util.Request { .accept_encoding = "", .if_none_match = "", .peer = .{ .ip4 = .loopback(0) }, + .client_addr = .{ .ip4 = .loopback(0) }, .arena = testing.allocator, }; } + +fn ip(text: []const u8) address.NetAddress { + return address.NetAddress.parse(text) catch unreachable; +} + +test "with no trusted proxy configured the socket peer is the client" { + const peer = ip("192.0.2.1"); + try testing.expect(clientAddr("", peer, "203.0.113.9").addr.eql(peer)); +} + +test "a spoofed forwarded-for from an untrusted peer is ignored" { + const peer = ip("192.0.2.1"); + try testing.expect(clientAddr("10.0.0.1", peer, "203.0.113.9").addr.eql(peer)); +} + +test "a trusted peer is identified by the last forwarded-for entry" { + const peer = ip("10.0.0.1"); + const derived = clientAddr("10.0.0.1, 10.0.0.2", peer, "203.0.113.9, 198.51.100.7"); + try testing.expect(derived.addr.eql(ip("198.51.100.7"))); +} + +test "a trusted peer that forwards nothing stays itself" { + const peer = ip("10.0.0.1"); + try testing.expect(clientAddr("10.0.0.1", peer, null).addr.eql(peer)); +} + +test "an IPv6 proxy and an IPv6 forwarded entry work the same way" { + const peer = ip("fd00::1"); + const derived = clientAddr("fd00::1", peer, "2001:db8::5, 2001:db8::7"); + try testing.expect(derived.addr.eql(ip("2001:db8::7"))); +} + +test "an oversized forwarded-for keeps the entry the proxy appended" { + var header: std.ArrayList(u8) = .empty; + defer header.deinit(testing.allocator); + while (header.items.len < 4096) try header.appendSlice(testing.allocator, "203.0.113.9, "); + try header.appendSlice(testing.allocator, "198.51.100.7"); + + // What the connection would copy: the tail alone, the head thrown away. + const tail = header.items[header.items.len - http_util.max_xff_len ..]; + const derived = clientAddr("10.0.0.1", ip("10.0.0.1"), tail); + try testing.expect(derived.addr.eql(ip("198.51.100.7"))); +} + +test "a trusted peer whose forwarded-for has no valid last entry is refused" { + const peer = ip("10.0.0.1"); + try testing.expectEqual( + ClientAddr.bad_forwarded_for, + std.meta.activeTag(clientAddr("10.0.0.1", peer, "203.0.113.9, not-an-ip")), + ); + try testing.expectEqual( + ClientAddr.bad_forwarded_for, + std.meta.activeTag(clientAddr("10.0.0.1", peer, "")), + ); + // An oversized header whose tail cuts the last entry in half is the same + // failure, and must not degrade to the exemptible socket peer. + try testing.expectEqual( + ClientAddr.bad_forwarded_for, + std.meta.activeTag(clientAddr("10.0.0.1", peer, "8.51.100.7000")), + ); +} + +test "a trusted-proxy element that is not an IP literal trusts nobody" { + const peer = ip("10.0.0.1"); + try testing.expect(!trustsPeer("proxy.example", peer)); + try testing.expect(trustsPeer("proxy.example, 10.0.0.1", peer)); + try testing.expect(!trustsPeer("", peer)); +} diff --git a/src/web/web_integration_test.zig b/src/web/web_integration_test.zig index a323c6c..3b84d84 100644 --- a/src/web/web_integration_test.zig +++ b/src/web/web_integration_test.zig @@ -20,6 +20,7 @@ const std = @import("std"); const build_options = @import("build_options"); +const contract_samples = @import("contract_samples"); const http = std.http; const net = std.Io.net; const Allocator = std.mem.Allocator; @@ -261,6 +262,7 @@ const EnvOptions = struct { rate_per_min: u32 = 100_000, localhost_exempt: bool = true, sse_max_per_ip: u16 = 3, + trusted_proxies: []const u8 = "", fallback: ?router.HandlerFn = null, }; @@ -356,7 +358,10 @@ const Env = struct { .enabled = true, .health = .init, }}; - self.pool = .init(&self.pool_entries, .{}, .{ .raw = .fromMilliseconds(50), .clock = .awake }, 1); + self.pool = .init(&self.pool_entries, .{}, .{ + .attempt = .{ .raw = .fromMilliseconds(50), .clock = .awake }, + .total = .{ .raw = .fromMilliseconds(100), .clock = .awake }, + }, 1); self.state = .{ .gpa = gpa, @@ -365,6 +370,7 @@ const Env = struct { .api_rate_limit_per_min = options.rate_per_min, .api_localhost_exempt = options.localhost_exempt, .sse_max_connections_per_ip = options.sse_max_per_ip, + .trusted_proxies = options.trusted_proxies, }, .live_hash = .init(options.password_hash), .pause = &self.pauser, @@ -528,7 +534,7 @@ const TlsEndpointView = struct { /// response never carries `web.password` or `web.password_hash` (ruling 16). const SettingsView = struct { settings: struct { - upstream: struct { read_timeout_ms: u32, total_timeout_ms: u32 }, + upstream: struct { attempt_timeout_ms: u32, read_timeout_ms: u32, total_timeout_ms: u32 }, dns: struct { bind_ipv4: []const u8, bind_ipv6: []const u8, @@ -546,6 +552,7 @@ const SettingsView = struct { api_rate_limit_per_min: u32, api_localhost_exempt: bool, sse_max_connections_per_ip: u16, + trusted_proxies: []const u8, auth_enabled: bool, }, doh_server: TlsEndpointView, @@ -1040,6 +1047,139 @@ test "W10 a drained bucket answers 429 with Retry-After and spares monitoring" { try bounded(env.io(), default_budget, rateLimited, .{ env.io(), env }); } +fn proxiedRateLimit(io: std.Io, env: *Env) anyerror!void { + var body_buf: [4096]u8 = undefined; + var conn: Conn = undefined; + try conn.connect(io, env.addr); + defer conn.close(io); + + // The socket peer is loopback and `api_localhost_exempt` is on, so without + // the forwarded-for header the bucket is never consulted: capacity is 1 and + // three requests in a row all pass. + for (0..3) |_| { + try conn.request("GET", "/api/version", null, null); + const exempt = try conn.receive(&body_buf); + try testing.expectEqual(@as(u16, 200), exempt.status); + } + + // The same connection, now carrying what the trusted proxy appends: the + // remote client is no longer loopback, so it spends its own token and the + // second request is refused. + try conn.request("GET", "/api/version", "x-forwarded-for: 203.0.113.9", null); + var response = try conn.receive(&body_buf); + try testing.expectEqual(@as(u16, 200), response.status); + + try conn.request("GET", "/api/version", "x-forwarded-for: 203.0.113.9", null); + response = try conn.receive(&body_buf); + try testing.expectEqual(@as(u16, 429), response.status); + + // A second remote client behind the same proxy has its own bucket. + try conn.request("GET", "/api/version", "x-forwarded-for: 198.51.100.4", null); + response = try conn.receive(&body_buf); + try testing.expectEqual(@as(u16, 200), response.status); + + // A chain whose last entry the proxy did not write is a 400, never a + // silent fall back to the exempt loopback peer. + try conn.request("GET", "/api/version", "x-forwarded-for: 203.0.113.9, nonsense", null); + response = try conn.receive(&body_buf); + try testing.expectEqual(@as(u16, 400), response.status); + try testing.expect(std.mem.containsAtLeast(u8, response.body, 1, "x-forwarded-for")); + + // The proxy itself is still exempt: its own unforwarded requests pass + // after every bucket above was drained. + try conn.request("GET", "/api/version", null, null); + response = try conn.receive(&body_buf); + try testing.expectEqual(@as(u16, 200), response.status); +} + +test "W10 milestone 17: a proxied client is rate limited while the proxy stays exempt" { + if (!build_options.integration) return error.SkipZigTest; + + const gpa = testing.allocator; + var env = try Env.create(gpa, .{ + .rate_per_min = 1, + .localhost_exempt = true, + .trusted_proxies = "127.0.0.1, ::1", + }); + defer env.destroy(); + + try bounded(env.io(), default_budget, proxiedRateLimit, .{ env.io(), env }); +} + +fn spoofedForwardedFor(io: std.Io, env: *Env) anyerror!void { + var body_buf: [4096]u8 = undefined; + var conn: Conn = undefined; + try conn.connect(io, env.addr); + defer conn.close(io); + + // No proxy is trusted, so the header is inert: the loopback peer keeps its + // exemption and no remote bucket is ever touched. + for (0..3) |_| { + try conn.request("GET", "/api/version", "x-forwarded-for: 203.0.113.9", null); + const response = try conn.receive(&body_buf); + try testing.expectEqual(@as(u16, 200), response.status); + } + + // Not even a chain nxdns would refuse from a trusted proxy. + try conn.request("GET", "/api/version", "x-forwarded-for: nonsense", null); + const ignored = try conn.receive(&body_buf); + try testing.expectEqual(@as(u16, 200), ignored.status); +} + +fn proxiedSseBudget(io: std.Io, env: *Env) anyerror!void { + var seen: std.ArrayList(u8) = .empty; + defer seen.deinit(env.gpa); + + // One stream per address, and every connection arrives from loopback: keyed + // on the socket peer these two would be one client and the second would be + // refused. + var first: Conn = undefined; + try first.connect(io, env.addr); + defer first.close(io); + try first.request("GET", "/api/queries/live", "x-forwarded-for: 203.0.113.9", null); + try testing.expectEqual(@as(u16, 200), (try first.receiveHead()).status); + try first.readChunkedUntil(&seen, env.gpa, "retry: 3000"); + + var second: Conn = undefined; + try second.connect(io, env.addr); + defer second.close(io); + try second.request("GET", "/api/queries/live", "x-forwarded-for: 198.51.100.4", null); + try testing.expectEqual(@as(u16, 200), (try second.receiveHead()).status); + try second.readChunkedUntil(&seen, env.gpa, "retry: 3000"); + + // The first client's own budget is spent, though. + var again: Conn = undefined; + try again.connect(io, env.addr); + defer again.close(io); + var body_buf: [1024]u8 = undefined; + try again.request("GET", "/api/queries/live", "x-forwarded-for: 203.0.113.9", null); + const refused = try again.receive(&body_buf); + try testing.expectEqual(@as(u16, 429), refused.status); +} + +test "W10 milestone 17: each proxied client holds its own SSE budget" { + if (!build_options.integration) return error.SkipZigTest; + + const gpa = testing.allocator; + var env = try Env.create(gpa, .{ + .sse_max_per_ip = 1, + .trusted_proxies = "127.0.0.1, ::1", + }); + defer env.destroy(); + + try bounded(env.io(), default_budget, proxiedSseBudget, .{ env.io(), env }); +} + +test "W10 milestone 17: a forwarded-for from an untrusted peer changes nothing" { + if (!build_options.integration) return error.SkipZigTest; + + const gpa = testing.allocator; + var env = try Env.create(gpa, .{ .rate_per_min = 1, .localhost_exempt = true }); + defer env.destroy(); + + try bounded(env.io(), default_budget, spoofedForwardedFor, .{ env.io(), env }); +} + // --------------------------------------------------------------------------- // SSE (ruling 20): preamble, event frame, heartbeat, per-address cap // --------------------------------------------------------------------------- @@ -1748,6 +1888,421 @@ test "drift guard a bites: methods swapped between two documented paths fail the try testing.expectEqual(@as(usize, 2), swapped_routes); } +// --------------------------------------------------------------------------- +// contract samples: the frontend's consumed shapes against real responses +// (milestone-17 ruling 5) +// --------------------------------------------------------------------------- +// +// The Zig side of the REST contract is already guarded: the table above parses +// every live response strictly, and the openapi guards below cover the routes. +// `web/src/lib/types.ts` was guarded by nothing — every frontend test stubs +// fetch, and types.ts is narrower than the wire in places (literal unions like +// `Health.status`), so re-parsing into Zig structs can never catch an +// out-of-union string. +// +// This walk drives the real `Env` server through every `.json` route the +// frontend reaches through an `api.ts` wrapper — the GETs and the JSON-returning +// writes — and renders the canonicalized bodies into a committed TypeScript +// file. TypeScript object literals get excess-property checking, so a server +// field missing from types.ts, a types.ts field missing from the wire, and an +// out-of-union literal all fail `npm run typecheck`. + +/// One captured response. `ts_type` is the type argument `api.ts` hands to its +/// own `request` for this endpoint — derived from that file, never invented. +const ContractSample = struct { + name: []const u8, + ts_type: []const u8, + method: []const u8, + target: []const u8, + body: ?[]const u8 = null, + status: u16, +}; + +/// Execution order is table order, and it is load-bearing twice over: a list +/// route runs after the create that gave it a row (an empty array witnesses no +/// field at all), and `POST /api/blocklists/update` runs while the only source +/// row is disabled, so the pass syncs its status without fetching anything. +const contract_sample_walk = [_]ContractSample{ + .{ .name = "get_health", .ts_type = "Health", .method = "GET", .target = "/api/health", .status = 200 }, + .{ .name = "get_version", .ts_type = "Version", .method = "GET", .target = "/api/version", .status = 200 }, + .{ .name = "login", .ts_type = "LoginResponse", .method = "POST", .target = "/api/auth/login", .body = "{\"password\":\"\"}", .status = 200 }, + .{ .name = "logout", .ts_type = "LogoutResponse", .method = "POST", .target = "/api/auth/logout", .body = "{}", .status = 200 }, + + // Blocklists. The row is created disabled so the refresh below has a status + // to report and still downloads nothing. + .{ .name = "create_blocklist", .ts_type = "BlocklistEcho", .method = "POST", .target = "/api/blocklists", .body = "{\"url\":\"https://lists.example/ads.txt\",\"name\":\"ads\",\"enabled\":false}", .status = 201 }, + .{ .name = "list_blocklists", .ts_type = "{ blocklists: Blocklist[] }", .method = "GET", .target = "/api/blocklists", .status = 200 }, + .{ .name = "update_blocklist", .ts_type = "BlocklistEcho", .method = "PUT", .target = "/api/blocklists/1", .body = "{\"url\":\"https://lists.example/ads.txt\",\"name\":\"ads2\",\"enabled\":false}", .status = 200 }, + .{ .name = "update_blocklists_now", .ts_type = "{ sources: SourceStatus[] }", .method = "POST", .target = "/api/blocklists/update", .body = "{}", .status = 202 }, + + // Groups. The migrated schema seeds `default` as id 1; the POST creates 2. + .{ .name = "list_groups", .ts_type = "{ groups: Group[] }", .method = "GET", .target = "/api/groups", .status = 200 }, + .{ .name = "create_group", .ts_type = "Group", .method = "POST", .target = "/api/groups", .body = "{\"name\":\"kids\"}", .status = 201 }, + .{ .name = "update_group", .ts_type = "Group", .method = "PUT", .target = "/api/groups/2", .body = "{\"name\":\"teens\",\"safe_search\":true}", .status = 200 }, + .{ .name = "put_group_sources", .ts_type = "{ source_ids: number[] }", .method = "PUT", .target = "/api/groups/1/sources", .body = "{\"source_ids\":[1]}", .status = 200 }, + .{ .name = "get_group_sources", .ts_type = "{ source_ids: number[] }", .method = "GET", .target = "/api/groups/1/sources", .status = 200 }, + + // Rules, then the lookup that the rule makes answer `blocked`. + .{ .name = "create_rule", .ts_type = "RuleEcho", .method = "POST", .target = "/api/rules", .body = "{\"group_id\":1,\"pattern\":\"ads.example\",\"kind\":\"exact\",\"action\":\"block\"}", .status = 201 }, + .{ .name = "list_rules", .ts_type = "{ rules: Rule[] }", .method = "GET", .target = "/api/rules", .status = 200 }, + .{ .name = "update_rule", .ts_type = "RuleEcho", .method = "PUT", .target = "/api/rules/1", .body = "{\"group_id\":1,\"pattern\":\"*.ads.example\",\"kind\":\"wildcard\",\"action\":\"block\"}", .status = 200 }, + .{ .name = "get_lookup", .ts_type = "LookupResult", .method = "GET", .target = "/api/lookup?domain=sub.ads.example", .status = 200 }, + + // Local records. + .{ .name = "create_local_record", .ts_type = "LocalRecord", .method = "POST", .target = "/api/local-records", .body = "{\"name\":\"nas.lan\",\"rtype\":\"A\",\"value\":\"192.168.1.10\"}", .status = 201 }, + .{ .name = "list_local_records", .ts_type = "{ local_records: LocalRecord[] }", .method = "GET", .target = "/api/local-records", .status = 200 }, + .{ .name = "update_local_record", .ts_type = "LocalRecord", .method = "PUT", .target = "/api/local-records/1", .body = "{\"name\":\"nas.lan\",\"rtype\":\"A\",\"value\":\"192.168.1.11\",\"ttl\":120}", .status = 200 }, + + // Forward zones. + .{ .name = "create_forward_zone", .ts_type = "ForwardZone", .method = "POST", .target = "/api/forward-zones", .body = "{\"zone\":\"lan\",\"resolver\":\"udp://10.0.0.1:53\"}", .status = 201 }, + .{ .name = "list_forward_zones", .ts_type = "{ forward_zones: ForwardZone[] }", .method = "GET", .target = "/api/forward-zones", .status = 200 }, + .{ .name = "update_forward_zone", .ts_type = "ForwardZone", .method = "PUT", .target = "/api/forward-zones/1", .body = "{\"zone\":\"lan\",\"resolver\":\"udp://10.0.0.2:53\"}", .status = 200 }, + + // Clients (row id 1 is seeded — clients have no POST, ruling 9). + .{ .name = "list_clients", .ts_type = "{ clients: Client[] }", .method = "GET", .target = "/api/clients", .status = 200 }, + .{ .name = "update_client", .ts_type = "Client", .method = "PUT", .target = "/api/clients/1", .body = "{\"name\":\"laptop-renamed\",\"group_id\":1}", .status = 200 }, + .{ .name = "put_client_prefixes", .ts_type = "{ client_prefixes: ClientPrefix[] }", .method = "PUT", .target = "/api/client-prefixes", .body = "{\"client_prefixes\":[{\"prefix\":\"192.168.1.0/24\",\"group_id\":1}]}", .status = 200 }, + .{ .name = "list_client_prefixes", .ts_type = "{ client_prefixes: ClientPrefix[] }", .method = "GET", .target = "/api/client-prefixes", .status = 200 }, + + // Upstreams. Row id 1 is seeded; the PUT leaves its url alone so the + // conflict sample below can collide with it. + .{ .name = "list_upstreams", .ts_type = "{ upstreams: Upstream[] }", .method = "GET", .target = "/api/upstreams", .status = 200 }, + .{ .name = "create_upstream", .ts_type = "UpstreamEcho", .method = "POST", .target = "/api/upstreams", .body = "{\"url\":\"https://dns2.example/dns-query\"}", .status = 201 }, + .{ .name = "update_upstream", .ts_type = "UpstreamEcho", .method = "PUT", .target = "/api/upstreams/1", .body = "{\"url\":\"https://dns.example/dns-query\",\"priority\":5}", .status = 200 }, + .{ .name = "get_upstream_health", .ts_type = "UpstreamHealth", .method = "GET", .target = "/api/upstream/health", .status = 200 }, + + // Query log and stats. `limit=5` reaches seeded row 21, the blocked one, so + // the page carries both the null-bearing and the populated row shape. + .{ .name = "get_queries", .ts_type = "QueriesPage", .method = "GET", .target = "/api/queries?limit=5", .status = 200 }, + .{ .name = "get_stats", .ts_type = "StatsTotals", .method = "GET", .target = "/api/stats?period=1h", .status = 200 }, + .{ .name = "get_stats_timeseries", .ts_type = "StatsTimeseries", .method = "GET", .target = "/api/stats/timeseries?period=1h", .status = 200 }, + + // Pause: the GET before the POST, so one sample carries `until: null` and + // the other the deadline. + .{ .name = "get_pause", .ts_type = "PauseState", .method = "GET", .target = "/api/pause", .status = 200 }, + .{ .name = "post_pause", .ts_type = "PauseState", .method = "POST", .target = "/api/pause", .body = "{\"paused\":true,\"duration_seconds\":600}", .status = 200 }, + + // Settings: the GET before any write, so `restart_required` is empty there + // and populated in the PUT's echo. + .{ .name = "get_settings", .ts_type = "SettingsEnvelope", .method = "GET", .target = "/api/settings", .status = 200 }, + .{ .name = "put_settings", .ts_type = "SettingsEnvelope", .method = "PUT", .target = "/api/settings", .body = "{\"dns\":{\"port\":5353}}", .status = 200 }, + + // One sample per shared error class this environment can produce. 401 and + // 429 need their own environments and follow below. + .{ .name = "error_bad_request", .ts_type = "ErrorEnvelope", .method = "PUT", .target = "/api/settings", .body = "{\"logging\":{\"level\":\"chatty\"}}", .status = 400 }, + .{ .name = "error_conflict", .ts_type = "ErrorEnvelope", .method = "POST", .target = "/api/upstreams", .body = "{\"url\":\"https://dns.example/dns-query\"}", .status = 409 }, + .{ .name = "error_not_found", .ts_type = "ErrorEnvelope", .method = "GET", .target = "/api/nope", .status = 404 }, +}; + +/// A session-authenticated environment answers this without a cookie. +const unauthorized_sample: ContractSample = .{ + .name = "error_unauthorized", + .ts_type = "ErrorEnvelope", + .method = "GET", + .target = "/api/groups", + .status = 401, +}; + +/// The second request on a one-token bucket. +const rate_limited_sample: ContractSample = .{ + .name = "error_rate_limited", + .ts_type = "ErrorEnvelope", + .method = "GET", + .target = "/api/version", + .status = 429, +}; + +/// `prettier` settings from web/package.json: tabs four columns wide, 120 +/// columns. The generated file has to be a fixpoint of the repo's formatter or +/// CI's `npm run format:check` fails on it. +const ts_print_width = 120; +const ts_tab_width = 4; + +/// Build identity, not contract data: `git_commit` comes from `-Dgit-commit` +/// and `zig_version` from the compiler that built the test, so keeping either +/// verbatim would pin the golden to one machine. Neither name occurs anywhere +/// else in the contract. +const volatile_string_keys = [_][]const u8{ "git_commit", "zig_version" }; + +fn writeTabs(w: *std.Io.Writer, depth: usize) !void { + for (0..depth) |_| try w.writeByte('\t'); +} + +/// True when `prettier` would print this object key without quotes. +fn isTsIdentifier(text: []const u8) bool { + if (text.len == 0) return false; + if (!std.ascii.isAlphabetic(text[0]) and text[0] != '_' and text[0] != '$') return false; + for (text[1..]) |byte| { + if (!std.ascii.isAlphanumeric(byte) and byte != '_' and byte != '$') return false; + } + return true; +} + +fn lessThanKey(_: void, a: []const u8, b: []const u8) bool { + return std.mem.order(u8, a, b) == .lt; +} + +/// Object keys sorted, every number 0, strings and booleans verbatim. The +/// canonical form is what makes the golden byte-stable across runs: the seed is +/// fixed, so only the numbers (row ids, timestamps, uptimes) move. +fn writeCanonical(arena: Allocator, w: *std.Io.Writer, value: std.json.Value, depth: usize) anyerror!void { + switch (value) { + .null => try w.writeAll("null"), + .bool => |flag| try w.writeAll(if (flag) "true" else "false"), + .integer, .float, .number_string => try w.writeAll("0"), + .string => |text| try std.json.Stringify.value(text, .{}, w), + .array => |list| try writeCanonicalArray(arena, w, list.items, depth), + .object => |map| try writeCanonicalObject(arena, w, map, depth), + } +} + +fn writeCanonicalObject( + arena: Allocator, + w: *std.Io.Writer, + map: std.json.ObjectMap, + depth: usize, +) anyerror!void { + if (map.count() == 0) return w.writeAll("{}"); + + const keys = try arena.dupe([]const u8, map.keys()); + std.mem.sort([]const u8, keys, {}, lessThanKey); + + // An object that starts with a newline stays expanded under `prettier`, so + // expanding every one of them is a fixpoint without measuring anything. + try w.writeAll("{\n"); + for (keys) |key| { + try writeTabs(w, depth + 1); + if (isTsIdentifier(key)) try w.writeAll(key) else try std.json.Stringify.value(key, .{}, w); + try w.writeAll(": "); + var volatile_key = false; + for (volatile_string_keys) |name_| volatile_key = volatile_key or std.mem.eql(u8, name_, key); + if (volatile_key) { + try w.writeAll("\"\""); + } else { + try writeCanonical(arena, w, map.get(key).?, depth + 1); + } + try w.writeAll(",\n"); + } + try writeTabs(w, depth); + try w.writeAll("}"); +} + +fn writeCanonicalArray( + arena: Allocator, + w: *std.Io.Writer, + items: []const std.json.Value, + depth: usize, +) anyerror!void { + if (items.len == 0) return w.writeAll("[]"); + + // Elements that canonicalize identically witness the same shape, so only + // the first of each is kept: a 60-bucket timeseries is 60 copies of one + // object and would bury everything else in the file. + var kept: std.ArrayList([]const u8) = .empty; + var all_primitive = true; + for (items) |item| { + switch (item) { + .array, .object => all_primitive = false, + else => {}, + } + var one: std.Io.Writer.Allocating = .init(arena); + try writeCanonical(arena, &one.writer, item, depth + 1); + const text = one.written(); + var seen = false; + for (kept.items) |prior| seen = seen or std.mem.eql(u8, prior, text); + if (!seen) try kept.append(arena, text); + } + + if (all_primitive) { + var width = depth * ts_tab_width + 2; + for (kept.items, 0..) |text, index| width += text.len + @as(usize, if (index == 0) 0 else 2); + if (width <= ts_print_width) { + try w.writeAll("["); + for (kept.items, 0..) |text, index| { + if (index != 0) try w.writeAll(", "); + try w.writeAll(text); + } + return w.writeAll("]"); + } + } + + try w.writeAll("[\n"); + for (kept.items) |text| { + try writeTabs(w, depth + 1); + try w.writeAll(text); + try w.writeAll(",\n"); + } + try writeTabs(w, depth); + try w.writeAll("]"); +} + +/// The pinned regeneration command, quoted verbatim in the file header and in +/// the failure message. +const regen_command = + "zig build test -Dintegration -Dcontract-samples-out=\"$PWD/" ++ contract_samples.path ++ "\""; + +/// Every capitalised identifier in the sample table's type expressions, sorted: +/// exactly the import list the generated file needs. +fn writeSampleImports(arena: Allocator, w: *std.Io.Writer) !void { + var names: std.ArrayList([]const u8) = .empty; + for (contract_sample_walk ++ [_]ContractSample{ unauthorized_sample, rate_limited_sample }) |sample| { + var index: usize = 0; + while (index < sample.ts_type.len) { + if (!std.ascii.isUpper(sample.ts_type[index])) { + index += 1; + continue; + } + var end = index; + while (end < sample.ts_type.len and std.ascii.isAlphanumeric(sample.ts_type[end])) end += 1; + const word = sample.ts_type[index..end]; + var seen = false; + for (names.items) |prior| seen = seen or std.mem.eql(u8, prior, word); + if (!seen) try names.append(arena, word); + index = end; + } + } + std.mem.sort([]const u8, names.items, {}, lessThanKey); + + try w.writeAll("import type {\n"); + for (names.items) |word| try w.print("\t{s},\n", .{word}); + try w.writeAll("} from \"@/lib/types\";\n"); +} + +fn writeSampleHeader(arena: Allocator, w: *std.Io.Writer) !void { + try w.writeAll( + \\// Generated file — do not edit by hand. + \\// + \\// Every value below is a real response from the web server, captured by the + \\// contract-sample test in src/web/web_integration_test.zig and canonicalized: + \\// object keys sorted, every number 0, strings and booleans as the deterministic + \\// seed produced them, repeated array elements collapsed to the first. The type + \\// annotations are the ones api.ts hands to its own `request`, so `tsc` + \\// refuses a field the wire does not send, a wire field types.ts does not + \\// declare, and a string outside a literal union. + \\// + \\// Regenerate with: + \\// + ); + try w.print(" {s}\n\n", .{regen_command}); + try writeSampleImports(arena, w); +} + +/// Sends one sample's request on `conn` and appends its canonical rendering. +fn captureSample( + gpa: Allocator, + conn: *Conn, + out: *std.Io.Writer, + sample: ContractSample, + body_buf: []u8, +) anyerror!void { + try conn.request(sample.method, sample.target, null, sample.body); + const response = try conn.receive(body_buf); + if (response.status != sample.status) { + std.debug.print( + "contract sample {s} ({s} {s}): expected {d}, got {d} body {s}\n", + .{ sample.name, sample.method, sample.target, sample.status, response.status, response.body }, + ); + return error.TestUnexpectedResult; + } + + var arena_state: std.heap.ArenaAllocator = .init(gpa); + defer arena_state.deinit(); + const arena = arena_state.allocator(); + + const value = std.json.parseFromSliceLeaky(std.json.Value, arena, response.body, .{}) catch |err| { + std.debug.print("contract sample {s}: body is not JSON ({t}): {s}\n", .{ sample.name, err, response.body }); + return err; + }; + + try out.print("\nexport const sample_{s}: {s} = ", .{ sample.name, sample.ts_type }); + try writeCanonical(arena, out, value, 0); + try out.writeAll(";\n"); +} + +fn sampleWalk(io: std.Io, env: *Env, out: *std.Io.Writer) anyerror!void { + var conn: Conn = undefined; + try conn.connect(io, env.addr); + defer conn.close(io); + + var body_buf: [128 * 1024]u8 = undefined; + for (contract_sample_walk) |sample| try captureSample(env.gpa, &conn, out, sample, &body_buf); +} + +fn sampleUnauthorized(io: std.Io, env: *Env, out: *std.Io.Writer) anyerror!void { + var conn: Conn = undefined; + try conn.connect(io, env.addr); + defer conn.close(io); + + var body_buf: [4096]u8 = undefined; + try captureSample(env.gpa, &conn, out, unauthorized_sample, &body_buf); +} + +fn sampleRateLimited(io: std.Io, env: *Env, out: *std.Io.Writer) anyerror!void { + var conn: Conn = undefined; + try conn.connect(io, env.addr); + defer conn.close(io); + + var body_buf: [4096]u8 = undefined; + // Capacity 1: the first counted request spends the only token. + try conn.request("GET", "/api/version", null, null); + const spent = try conn.receive(&body_buf); + try testing.expectEqual(@as(u16, 200), spent.status); + + try captureSample(env.gpa, &conn, out, rate_limited_sample, &body_buf); +} + +test "W10 milestone 17: the committed contract samples still describe live responses" { + if (!build_options.integration) return error.SkipZigTest; + + const gpa = testing.allocator; + + var arena_state: std.heap.ArenaAllocator = .init(gpa); + defer arena_state.deinit(); + + var rendered: std.Io.Writer.Allocating = .init(gpa); + defer rendered.deinit(); + try writeSampleHeader(arena_state.allocator(), &rendered.writer); + + { + var env = try Env.create(gpa, .{}); + defer env.destroy(); + try bounded(env.io(), default_budget, sampleWalk, .{ env.io(), env, &rendered.writer }); + } + { + var hash_buf: [256]u8 = undefined; + const hash = try hashTestPassword(gpa, &hash_buf); + var env = try Env.create(gpa, .{ .password_hash = hash }); + defer env.destroy(); + try bounded(env.io(), default_budget, sampleUnauthorized, .{ env.io(), env, &rendered.writer }); + } + { + var env = try Env.create(gpa, .{ .rate_per_min = 1, .localhost_exempt = false }); + defer env.destroy(); + try bounded(env.io(), default_budget, sampleRateLimited, .{ env.io(), env, &rendered.writer }); + } + + if (build_options.contract_samples_out.len != 0) { + var write_threaded: std.Io.Threaded = .init(gpa, .{}); + defer write_threaded.deinit(); + try std.Io.Dir.cwd().writeFile(write_threaded.io(), .{ + .sub_path = build_options.contract_samples_out, + .data = rendered.written(), + }); + std.debug.print("wrote {s}\n", .{build_options.contract_samples_out}); + return; + } + + if (!std.mem.eql(u8, contract_samples.bytes, rendered.written())) { + std.debug.print( + "{s} no longer matches the live responses.\n" ++ + "The server and the frontend's types.ts have drifted, or the seed changed.\n" ++ + "Regenerate, then read the diff and `npm run typecheck`:\n {s}\n", + .{ contract_samples.path, regen_command }, + ); + return error.TestUnexpectedResult; + } +} + test "drift guard b: openapi.yaml documents exactly as many operations as the router serves" { const yaml = openapi.yaml; const paths_start = std.mem.indexOf(u8, yaml, "\npaths:\n") orelse return error.TestUnexpectedResult; diff --git a/web/src/features/settings/RestartBanner.tsx b/web/src/features/settings/RestartBanner.tsx index 05dc4ae..bb68ebf 100644 --- a/web/src/features/settings/RestartBanner.tsx +++ b/web/src/features/settings/RestartBanner.tsx @@ -8,7 +8,7 @@ export default function RestartBanner() { role="status" className="flex items-center gap-3 border-b border-amber-300 bg-amber-50 px-4 py-2 text-sm text-amber-900 dark:border-amber-800 dark:bg-amber-950 dark:text-amber-100" > - Settings saved. Restart nxdns to apply. + Changes saved. Restart nxdns to apply. + {onCancel !== undefined && ( + + )} + + + + ); +} diff --git a/web/src/features/upstreams/UpstreamsPage.test.tsx b/web/src/features/upstreams/UpstreamsPage.test.tsx new file mode 100644 index 0000000..bc00ee9 --- /dev/null +++ b/web/src/features/upstreams/UpstreamsPage.test.tsx @@ -0,0 +1,222 @@ +import { fireEvent, render, screen, waitFor } from "@testing-library/react"; +import { QueryClientProvider } from "@tanstack/react-query"; +import { RouterProvider, createMemoryHistory } from "@tanstack/react-router"; +import { AuthProvider } from "@/auth/store"; +import { dismissRestartBanner } from "@/features/settings/restartBanner"; +import { createQueryClient } from "@/lib/queryClient"; +import { createAppRouter } from "@/routes"; + +const UPSTREAMS = { + upstreams: [ + { id: 1, url: "udp://1.1.1.1:53", priority: 100, enabled: true, tls_name: "" }, + { id: 2, url: "tls://9.9.9.9:853", priority: 200, enabled: false, tls_name: "dns.quad9.net" }, + ], +}; + +const RESPONSES: Record = { + "/api/upstreams": UPSTREAMS, + "/api/version": { version: "0.0.0-test", git_commit: "0000000", zig_version: "0.16.0", uptime_seconds: 1 }, +}; + +interface Call { + url: string; + method: string; + body: unknown; +} + +let calls: Call[]; +let writeResponse: (() => Response) | null; + +beforeEach(() => { + calls = []; + writeResponse = null; + dismissRestartBanner(); + vi.stubGlobal( + "fetch", + vi.fn(async (input: RequestInfo | URL, init?: RequestInit) => { + const url = String(input); + const method = init?.method ?? "GET"; + if (method !== "GET") { + calls.push({ + url, + method, + body: typeof init?.body === "string" ? JSON.parse(init.body) : undefined, + }); + if (writeResponse !== null) return writeResponse(); + if (method === "DELETE") return new Response(null, { status: 204 }); + return new Response( + JSON.stringify({ + id: 3, + url: "udp://8.8.8.8:53", + priority: 100, + enabled: true, + tls_name: "", + restart_required: true, + }), + { status: 200, headers: { "content-type": "application/json" } }, + ); + } + const payload = RESPONSES[url]; + if (payload === undefined) return new Response(JSON.stringify({ error: "not stubbed" }), { status: 404 }); + return new Response(JSON.stringify(payload), { + status: 200, + headers: { "content-type": "application/json" }, + }); + }), + ); +}); + +afterEach(() => { + vi.unstubAllGlobals(); + vi.restoreAllMocks(); +}); + +async function renderUpstreamsRoute() { + const queryClient = createQueryClient(); + const router = createAppRouter(createMemoryHistory({ initialEntries: ["/upstreams"] }), queryClient); + render( + + + + + , + ); + await screen.findByRole("heading", { name: "Upstreams" }); +} + +test("renders the upstream table and the add form", async () => { + await renderUpstreamsRoute(); + + expect(screen.getByText("udp://1.1.1.1:53")).toBeTruthy(); + expect(screen.getByText("tls://9.9.9.9:853")).toBeTruthy(); + expect(screen.getByText("100")).toBeTruthy(); + expect(screen.getByText("200")).toBeTruthy(); + expect(screen.getByText("dns.quad9.net")).toBeTruthy(); + + const enabledToggle = screen.getByLabelText("udp://1.1.1.1:53 enabled") as HTMLInputElement; + expect(enabledToggle.checked).toBe(true); + const disabledToggle = screen.getByLabelText("tls://9.9.9.9:853 enabled") as HTMLInputElement; + expect(disabledToggle.checked).toBe(false); + + expect(screen.getByRole("heading", { name: "Add upstream" })).toBeTruthy(); + expect(screen.getByText(/reflects the running pool/)).toBeTruthy(); +}); + +test("adding an upstream posts every field and raises the restart banner", async () => { + await renderUpstreamsRoute(); + + fireEvent.change(screen.getByLabelText("URL"), { target: { value: "udp://8.8.8.8:53" } }); + fireEvent.change(screen.getByLabelText("Priority"), { target: { value: "150" } }); + fireEvent.click(screen.getByRole("button", { name: "Add upstream" })); + + await waitFor(() => expect(calls).toHaveLength(1)); + expect(calls[0]).toEqual({ + url: "/api/upstreams", + method: "POST", + body: { url: "udp://8.8.8.8:53", priority: 150, enabled: true, tls_name: "" }, + }); + + const banner = await screen.findByRole("status"); + expect(banner.textContent).toContain("Changes saved. Restart nxdns to apply."); +}); + +test("toggling enabled resends the whole row", async () => { + await renderUpstreamsRoute(); + + fireEvent.click(screen.getByLabelText("tls://9.9.9.9:853 enabled")); + + await waitFor(() => expect(calls).toHaveLength(1)); + expect(calls[0]).toEqual({ + url: "/api/upstreams/2", + method: "PUT", + body: { url: "tls://9.9.9.9:853", priority: 200, enabled: true, tls_name: "dns.quad9.net" }, + }); + await screen.findByRole("status"); +}); + +test("delete asks for confirmation and skips the request when refused", async () => { + await renderUpstreamsRoute(); + + vi.spyOn(window, "confirm").mockReturnValue(false); + fireEvent.click(screen.getAllByRole("button", { name: "Delete" })[0]!); + expect(calls).toHaveLength(0); + + vi.spyOn(window, "confirm").mockReturnValue(true); + fireEvent.click(screen.getAllByRole("button", { name: "Delete" })[0]!); + await waitFor(() => expect(calls).toHaveLength(1)); + expect(calls[0]!.method).toBe("DELETE"); + expect(calls[0]!.url).toBe("/api/upstreams/1"); + await screen.findByRole("status"); +}); + +test("a 409 on create renders the conflict text inline", async () => { + await renderUpstreamsRoute(); + + writeResponse = () => + new Response(JSON.stringify({ error: "an upstream with that url already exists" }), { + status: 409, + headers: { "content-type": "application/json" }, + }); + + fireEvent.change(screen.getByLabelText("URL"), { target: { value: "udp://1.1.1.1:53" } }); + fireEvent.click(screen.getByRole("button", { name: "Add upstream" })); + + const alert = await screen.findByRole("alert"); + expect(alert.textContent).toBe("an upstream with that url already exists"); + expect(screen.queryByRole("status")).toBeNull(); +}); + +test("a 409 on toggle renders the last-enabled conflict above the form", async () => { + await renderUpstreamsRoute(); + + writeResponse = () => + new Response(JSON.stringify({ error: "the last enabled upstream cannot be disabled" }), { + status: 409, + headers: { "content-type": "application/json" }, + }); + + fireEvent.click(screen.getByLabelText("udp://1.1.1.1:53 enabled")); + + const alert = await screen.findByRole("alert"); + expect(alert.textContent).toBe("the last enabled upstream cannot be disabled"); + expect(screen.queryByRole("status")).toBeNull(); +}); + +test("a 409 on delete renders the last-enabled conflict", async () => { + await renderUpstreamsRoute(); + + vi.spyOn(window, "confirm").mockReturnValue(true); + writeResponse = () => + new Response(JSON.stringify({ error: "the last enabled upstream cannot be removed" }), { + status: 409, + headers: { "content-type": "application/json" }, + }); + + fireEvent.click(screen.getAllByRole("button", { name: "Delete" })[0]!); + + const alert = await screen.findByRole("alert"); + expect(alert.textContent).toBe("the last enabled upstream cannot be removed"); + expect(screen.queryByRole("status")).toBeNull(); +}); + +test("editing a row seeds the form and PUTs the replaced row", async () => { + await renderUpstreamsRoute(); + + fireEvent.click(screen.getAllByRole("button", { name: "Edit" })[1]!); + await screen.findByRole("heading", { name: "Edit tls://9.9.9.9:853" }); + + expect((screen.getByLabelText("URL") as HTMLInputElement).value).toBe("tls://9.9.9.9:853"); + expect((screen.getByLabelText("Priority") as HTMLInputElement).value).toBe("200"); + expect((screen.getByLabelText("TLS name") as HTMLInputElement).value).toBe("dns.quad9.net"); + + fireEvent.change(screen.getByLabelText("Priority"), { target: { value: "10" } }); + fireEvent.click(screen.getByRole("button", { name: "Save changes" })); + + await waitFor(() => expect(calls).toHaveLength(1)); + expect(calls[0]).toEqual({ + url: "/api/upstreams/2", + method: "PUT", + body: { url: "tls://9.9.9.9:853", priority: 10, enabled: false, tls_name: "dns.quad9.net" }, + }); + await screen.findByRole("heading", { name: "Add upstream" }); +}); diff --git a/web/src/features/upstreams/UpstreamsPage.tsx b/web/src/features/upstreams/UpstreamsPage.tsx new file mode 100644 index 0000000..ddc2ce6 --- /dev/null +++ b/web/src/features/upstreams/UpstreamsPage.tsx @@ -0,0 +1,131 @@ +import { useState } from "react"; +import { useMutation, useQueryClient, useSuspenseQuery } from "@tanstack/react-query"; +import InlineError from "@/lib/InlineError"; +import { upstreamCreateMutation, upstreamDeleteMutation, upstreamUpdateMutation, upstreamsQuery } from "@/lib/queries"; +import type { Upstream, UpstreamInput } from "@/lib/types"; +import { raiseRestartBanner } from "../settings/restartBanner"; +import UpstreamForm from "./UpstreamForm"; + +const TH_CLASS = "border-b border-zinc-300 px-3 py-2 text-left font-medium dark:border-zinc-700"; +const TD_CLASS = "border-b border-zinc-200 px-3 py-2 dark:border-zinc-800"; + +export default function UpstreamsPage() { + const queryClient = useQueryClient(); + const { data: upstreams } = useSuspenseQuery(upstreamsQuery()); + const [editing, setEditing] = useState(null); + + const create = useMutation(upstreamCreateMutation(queryClient)); + const save = useMutation(upstreamUpdateMutation(queryClient)); + const toggle = useMutation(upstreamUpdateMutation(queryClient)); + const remove = useMutation(upstreamDeleteMutation(queryClient)); + + async function submitForm(input: UpstreamInput) { + if (editing === null) { + await create.mutateAsync(input); + } else { + await save.mutateAsync({ id: editing.id, input }); + setEditing(null); + } + raiseRestartBanner(); + } + + function toggleEnabled(u: Upstream) { + toggle.mutate( + { + id: u.id, + input: { url: u.url, priority: u.priority, enabled: !u.enabled, tls_name: u.tls_name }, + }, + { onSuccess: () => raiseRestartBanner() }, + ); + } + + function deleteUpstream(u: Upstream) { + if (window.confirm(`Delete upstream "${u.url}"? Queries stop being forwarded to it.`)) { + remove.mutate(u.id, { onSuccess: () => raiseRestartBanner() }); + } + } + + const formError = editing === null ? create.error : save.error; + const tableError = remove.error ?? toggle.error; + + return ( +
+

Upstreams

+

+ The pool builds its clients at startup, so an edit here takes effect at the next restart. The upstream + health table on the Dashboard reflects the running pool, not this list. +

+ + {upstreams.length === 0 ? ( +

No upstreams yet. Add one below.

+ ) : ( +
+ + + + + + + + + + + + {upstreams.map((u) => ( + + + + + + + + ))} + +
URLPriorityEnabledTLS name + Actions +
+ + {u.url} + + {u.priority} + toggleEnabled(u)} + /> + {u.tls_name === "" ? "—" : u.tls_name} +
+ + +
+
+
+ )} + + + setEditing(null)} + /> +
+ ); +} diff --git a/web/src/lib/api.ts b/web/src/lib/api.ts index 620f875..4a7e83c 100644 --- a/web/src/lib/api.ts +++ b/web/src/lib/api.ts @@ -81,12 +81,6 @@ async function request(path: string, init?: { method?: string; body?: unknown return (await res.json()) as T; } -async function requestText(path: string): Promise { - const res = await fetch(path, { credentials: "same-origin" }); - if (!res.ok) throw await toApiError(res); - return res.text(); -} - type QueryValue = string | number | boolean | undefined; function qs(params: Record): string { @@ -100,10 +94,8 @@ function qs(params: Record): string { // Monitoring + meta -export const getMetrics = (): Promise => requestText("/metrics"); export const getHealth = (): Promise => request("/api/health"); export const getVersion = (): Promise => request("/api/version"); -export const getOpenapiYaml = (): Promise => requestText("/api/openapi.yaml"); // Auth @@ -133,7 +125,6 @@ export const getUpstreamHealth = (): Promise => request("/api/up export const listGroups = async (): Promise => (await request<{ groups: Group[] }>("/api/groups")).groups; export const createGroup = (input: GroupInput): Promise => request("/api/groups", { method: "POST", body: input }); -export const getGroup = (id: number): Promise => request(`/api/groups/${id}`); export const updateGroup = (id: number, input: GroupInput): Promise => request(`/api/groups/${id}`, { method: "PUT", body: input }); export const deleteGroup = (id: number): Promise => request(`/api/groups/${id}`, { method: "DELETE" }); @@ -156,7 +147,6 @@ export const createBlocklist = (input: BlocklistInput): Promise = request("/api/blocklists", { method: "POST", body: input }); export const updateBlocklistsNow = async (): Promise => (await request<{ sources: SourceStatus[] }>("/api/blocklists/update", { method: "POST", body: {} })).sources; -export const getBlocklist = (id: number): Promise => request(`/api/blocklists/${id}`); export const updateBlocklist = (id: number, input: BlocklistInput): Promise => request(`/api/blocklists/${id}`, { method: "PUT", body: input }); export const deleteBlocklist = (id: number): Promise => request(`/api/blocklists/${id}`, { method: "DELETE" }); @@ -166,7 +156,6 @@ export const deleteBlocklist = (id: number): Promise => request(`/api/bloc export const listRules = async (): Promise => (await request<{ rules: Rule[] }>("/api/rules")).rules; export const createRule = (input: RuleInput): Promise => request("/api/rules", { method: "POST", body: input }); -export const getRule = (id: number): Promise => request(`/api/rules/${id}`); export const updateRule = (id: number, input: RuleInput): Promise => request(`/api/rules/${id}`, { method: "PUT", body: input }); export const deleteRule = (id: number): Promise => request(`/api/rules/${id}`, { method: "DELETE" }); @@ -177,7 +166,6 @@ export const listLocalRecords = async (): Promise => (await request<{ local_records: LocalRecord[] }>("/api/local-records")).local_records; export const createLocalRecord = (input: LocalRecordInput): Promise => request("/api/local-records", { method: "POST", body: input }); -export const getLocalRecord = (id: number): Promise => request(`/api/local-records/${id}`); export const updateLocalRecord = (id: number, input: LocalRecordInput): Promise => request(`/api/local-records/${id}`, { method: "PUT", body: input }); export const deleteLocalRecord = (id: number): Promise => @@ -189,7 +177,6 @@ export const listForwardZones = async (): Promise => (await request<{ forward_zones: ForwardZone[] }>("/api/forward-zones")).forward_zones; export const createForwardZone = (input: ForwardZoneInput): Promise => request("/api/forward-zones", { method: "POST", body: input }); -export const getForwardZone = (id: number): Promise => request(`/api/forward-zones/${id}`); export const updateForwardZone = (id: number, input: ForwardZoneInput): Promise => request(`/api/forward-zones/${id}`, { method: "PUT", body: input }); export const deleteForwardZone = (id: number): Promise => @@ -199,7 +186,6 @@ export const deleteForwardZone = (id: number): Promise => export const listClients = async (): Promise => (await request<{ clients: Client[] }>("/api/clients")).clients; -export const getClient = (id: number): Promise => request(`/api/clients/${id}`); export const updateClient = (id: number, edit: ClientEdit): Promise => request(`/api/clients/${id}`, { method: "PUT", body: edit }); export const deleteClient = (id: number): Promise => request(`/api/clients/${id}`, { method: "DELETE" }); @@ -220,7 +206,6 @@ export const listUpstreams = async (): Promise => (await request<{ upstreams: Upstream[] }>("/api/upstreams")).upstreams; export const createUpstream = (input: UpstreamInput): Promise => request("/api/upstreams", { method: "POST", body: input }); -export const getUpstream = (id: number): Promise => request(`/api/upstreams/${id}`); export const updateUpstream = (id: number, input: UpstreamInput): Promise => request(`/api/upstreams/${id}`, { method: "PUT", body: input }); export const deleteUpstream = (id: number): Promise => request(`/api/upstreams/${id}`, { method: "DELETE" }); diff --git a/web/src/lib/contractSamples.gen.ts b/web/src/lib/contractSamples.gen.ts new file mode 100644 index 0000000..f618ff2 --- /dev/null +++ b/web/src/lib/contractSamples.gen.ts @@ -0,0 +1,701 @@ +// Generated file — do not edit by hand. +// +// Every value below is a real response from the web server, captured by the +// contract-sample test in src/web/web_integration_test.zig and canonicalized: +// object keys sorted, every number 0, strings and booleans as the deterministic +// seed produced them, repeated array elements collapsed to the first. The type +// annotations are the ones api.ts hands to its own `request`, so `tsc` +// refuses a field the wire does not send, a wire field types.ts does not +// declare, and a string outside a literal union. +// +// Regenerate with: +// zig build test -Dintegration -Dcontract-samples-out="$PWD/web/src/lib/contractSamples.gen.ts" + +import type { + Blocklist, + BlocklistEcho, + Client, + ClientPrefix, + ErrorEnvelope, + ForwardZone, + Group, + Health, + LocalRecord, + LoginResponse, + LogoutResponse, + LookupResult, + PauseState, + QueriesPage, + Rule, + RuleEcho, + SettingsEnvelope, + SourceStatus, + StatsTimeseries, + StatsTotals, + Upstream, + UpstreamEcho, + UpstreamHealth, + Version, +} from "@/lib/types"; + +export const sample_get_health: Health = { + disk: { + db_bytes: 0, + free_bytes: 0, + log_bytes: 0, + sample_failures: 0, + state: "ok", + }, + queries_dropped: 0, + refreshes_gated: 0, + snapshot_generation: 0, + status: "ok", + upstreams: { + available: 0, + total: 0, + }, + writer_failed: false, +}; + +export const sample_get_version: Version = { + git_commit: "", + uptime_seconds: 0, + version: "w10-test", + zig_version: "", +}; + +export const sample_login: LoginResponse = { + auth_required: false, + authenticated: true, +}; + +export const sample_logout: LogoutResponse = { + authenticated: false, +}; + +export const sample_create_blocklist: BlocklistEcho = { + enabled: false, + id: 0, + is_suggested: false, + name: "ads", + url: "https://lists.example/ads.txt", +}; + +export const sample_list_blocklists: { blocklists: Blocklist[] } = { + blocklists: [ + { + checksum: null, + domain_count: 0, + enabled: false, + id: 0, + is_suggested: false, + last_updated: null, + name: "ads", + skipped_regex_count: 0, + url: "https://lists.example/ads.txt", + wildcard_count: 0, + }, + ], +}; + +export const sample_update_blocklist: BlocklistEcho = { + enabled: false, + id: 0, + is_suggested: false, + name: "ads2", + url: "https://lists.example/ads.txt", +}; + +export const sample_update_blocklists_now: { sources: SourceStatus[] } = { + sources: [ + { + domains: 0, + id: 0, + last_attempt: 0, + last_error: "", + last_success: 0, + loaded: false, + skipped_regex: 0, + state: "never_fetched", + url: "https://lists.example/ads.txt", + wildcards: 0, + }, + ], +}; + +export const sample_list_groups: { groups: Group[] } = { + groups: [ + { + id: 0, + name: "default", + safe_search: false, + }, + ], +}; + +export const sample_create_group: Group = { + id: 0, + name: "kids", + safe_search: false, +}; + +export const sample_update_group: Group = { + id: 0, + name: "teens", + safe_search: true, +}; + +export const sample_put_group_sources: { source_ids: number[] } = { + source_ids: [0], +}; + +export const sample_get_group_sources: { source_ids: number[] } = { + source_ids: [0], +}; + +export const sample_create_rule: RuleEcho = { + action: "block", + group_id: 0, + id: 0, + kind: "exact", + pattern: "ads.example", +}; + +export const sample_list_rules: { rules: Rule[] } = { + rules: [ + { + action: "block", + created_at: 0, + group: "default", + group_id: 0, + id: 0, + kind: "exact", + pattern: "ads.example", + }, + ], +}; + +export const sample_update_rule: RuleEcho = { + action: "block", + group_id: 0, + id: 0, + kind: "wildcard", + pattern: "*.ads.example", +}; + +export const sample_get_lookup: LookupResult = { + blocked: true, + domain: "sub.ads.example", + forward_zone: null, + group_id: 0, + local_records: false, + matched: "*.ads.example", + reason: "rule_block_wildcard", + safe_search_rewrite: null, + source_url: null, +}; + +export const sample_create_local_record: LocalRecord = { + id: 0, + name: "nas.lan", + rtype: "A", + ttl: 0, + value: "192.168.1.10", +}; + +export const sample_list_local_records: { local_records: LocalRecord[] } = { + local_records: [ + { + id: 0, + name: "nas.lan", + rtype: "A", + ttl: 0, + value: "192.168.1.10", + }, + ], +}; + +export const sample_update_local_record: LocalRecord = { + id: 0, + name: "nas.lan", + rtype: "A", + ttl: 0, + value: "192.168.1.11", +}; + +export const sample_create_forward_zone: ForwardZone = { + id: 0, + resolver: "udp://10.0.0.1:53", + zone: "lan", +}; + +export const sample_list_forward_zones: { forward_zones: ForwardZone[] } = { + forward_zones: [ + { + id: 0, + resolver: "udp://10.0.0.1:53", + zone: "lan", + }, + ], +}; + +export const sample_update_forward_zone: ForwardZone = { + id: 0, + resolver: "udp://10.0.0.2:53", + zone: "lan", +}; + +export const sample_list_clients: { clients: Client[] } = { + clients: [ + { + first_seen: 0, + group: "default", + group_id: 0, + hand_edited: false, + id: 0, + ip: "192.168.1.50", + last_seen: 0, + name: "laptop", + }, + ], +}; + +export const sample_update_client: Client = { + first_seen: 0, + group: "default", + group_id: 0, + hand_edited: true, + id: 0, + ip: "192.168.1.50", + last_seen: 0, + name: "laptop-renamed", +}; + +export const sample_put_client_prefixes: { client_prefixes: ClientPrefix[] } = { + client_prefixes: [ + { + group: "default", + group_id: 0, + id: 0, + prefix: "192.168.1.0/24", + priority: 0, + }, + ], +}; + +export const sample_list_client_prefixes: { client_prefixes: ClientPrefix[] } = { + client_prefixes: [ + { + group: "default", + group_id: 0, + id: 0, + prefix: "192.168.1.0/24", + priority: 0, + }, + ], +}; + +export const sample_list_upstreams: { upstreams: Upstream[] } = { + upstreams: [ + { + enabled: true, + id: 0, + priority: 0, + tls_name: "", + url: "https://dns.example/dns-query", + }, + ], +}; + +export const sample_create_upstream: UpstreamEcho = { + enabled: true, + id: 0, + priority: 0, + restart_required: true, + tls_name: "", + url: "https://dns2.example/dns-query", +}; + +export const sample_update_upstream: UpstreamEcho = { + enabled: true, + id: 0, + priority: 0, + restart_required: true, + tls_name: "", + url: "https://dns.example/dns-query", +}; + +export const sample_get_upstream_health: UpstreamHealth = { + available: 0, + total: 0, + upstreams: [ + { + available: true, + consecutive_failures: 0, + enabled: true, + last_error: "", + success_rate: 0, + total_failures: 0, + total_successes: 0, + url: "https://dns.example/dns-query", + }, + ], +}; + +export const sample_get_queries: QueriesPage = { + next_before: 0, + queries: [ + { + block_reason: "", + blocked: false, + cache_hit: true, + client_ip: "192.0.2.10", + domain: "d24.example", + id: 0, + qtype: 0, + response_time_us: 0, + ts: 0, + upstream: "https://dns.example/dns-query", + }, + { + block_reason: "", + blocked: false, + cache_hit: false, + client_ip: "192.0.2.10", + domain: "d23.example", + id: 0, + qtype: 0, + response_time_us: 0, + ts: 0, + upstream: "https://dns.example/dns-query", + }, + { + block_reason: "", + blocked: false, + cache_hit: true, + client_ip: "192.0.2.10", + domain: "d22.example", + id: 0, + qtype: 0, + response_time_us: 0, + ts: 0, + upstream: "https://dns.example/dns-query", + }, + { + block_reason: "", + blocked: false, + cache_hit: false, + client_ip: "192.0.2.10", + domain: "d21.example", + id: 0, + qtype: 0, + response_time_us: 0, + ts: 0, + upstream: "https://dns.example/dns-query", + }, + { + block_reason: "blocklist_domain", + blocked: true, + cache_hit: null, + client_ip: "192.0.2.10", + domain: "d20.example", + id: 0, + qtype: 0, + response_time_us: 0, + ts: 0, + upstream: "", + }, + ], +}; + +export const sample_get_stats: StatsTotals = { + avg_response_time_us: null, + blocked: 0, + cached: 0, + clients: 0, + period: "1h", + queries: 0, + since: 0, + until: 0, +}; + +export const sample_get_stats_timeseries: StatsTimeseries = { + bucket_seconds: 0, + buckets: [ + { + blocked: 0, + cached: 0, + queries: 0, + ts: 0, + }, + ], + period: "1h", + since: 0, + until: 0, +}; + +export const sample_get_pause: PauseState = { + paused: false, + until: null, +}; + +export const sample_post_pause: PauseState = { + paused: true, + until: 0, +}; + +export const sample_get_settings: SettingsEnvelope = { + restart_required: [ + "upstream.attempt_timeout_ms", + "upstream.read_timeout_ms", + "upstream.total_timeout_ms", + "dns.bind_ipv4", + "dns.bind_ipv6", + "dns.port", + "dns.rate_limit", + "dns.rate_window_seconds", + "blocking.response", + "blocking.ttl", + "cache.size", + "cache.negative_ttl_max", + "web.enabled", + "web.bind", + "web.port", + "web.session_ttl_hours", + "web.api_rate_limit_per_min", + "web.api_localhost_exempt", + "web.sse_max_connections_per_ip", + "web.trusted_proxies", + "doh_server.enabled", + "doh_server.bind", + "doh_server.port", + "doh_server.cert_path", + "doh_server.key_path", + "dot_server.enabled", + "dot_server.bind", + "dot_server.port", + "dot_server.cert_path", + "dot_server.key_path", + "edns.ecs_mode", + "logging.level", + "logging.retention_days", + "logging.query_log_buffer_max", + "logging.hide_domains", + "logging.hide_client_ips", + "logging.output", + "logging.file_path", + "logging.max_size_mb", + "logging.max_files", + "disk.min_free_mb", + "disk.warn_free_mb", + "blocklist_update.enabled", + "blocklist_update.interval_hours", + ], + settings: { + blocking: { + response: "zero", + ttl: 0, + }, + blocklist_update: { + enabled: true, + interval_hours: 0, + }, + cache: { + negative_ttl_max: 0, + size: 0, + }, + disk: { + min_free_mb: 0, + warn_free_mb: 0, + }, + dns: { + bind_ipv4: "0.0.0.0", + bind_ipv6: "::", + port: 0, + rate_limit: 0, + rate_window_seconds: 0, + }, + doh_server: { + bind: "0.0.0.0", + cert_path: "/etc/nxdns/cert.pem", + enabled: false, + key_path: "/etc/nxdns/key.pem", + port: 0, + }, + dot_server: { + bind: "0.0.0.0", + cert_path: "/etc/nxdns/cert.pem", + enabled: false, + key_path: "/etc/nxdns/key.pem", + port: 0, + }, + edns: { + ecs_mode: "strip", + }, + logging: { + file_path: "/var/log/nxdns/nxdns.log", + hide_client_ips: false, + hide_domains: false, + level: "info", + max_files: 0, + max_size_mb: 0, + output: "stderr", + query_log_buffer_max: 0, + retention_days: 0, + }, + upstream: { + attempt_timeout_ms: 0, + read_timeout_ms: 0, + total_timeout_ms: 0, + }, + web: { + api_localhost_exempt: true, + api_rate_limit_per_min: 0, + auth_enabled: false, + bind: "0.0.0.0", + enabled: true, + port: 0, + session_ttl_hours: 0, + sse_max_connections_per_ip: 0, + trusted_proxies: "", + }, + }, +}; + +export const sample_put_settings: SettingsEnvelope = { + restart_required: [ + "upstream.attempt_timeout_ms", + "upstream.read_timeout_ms", + "upstream.total_timeout_ms", + "dns.bind_ipv4", + "dns.bind_ipv6", + "dns.port", + "dns.rate_limit", + "dns.rate_window_seconds", + "blocking.response", + "blocking.ttl", + "cache.size", + "cache.negative_ttl_max", + "web.enabled", + "web.bind", + "web.port", + "web.session_ttl_hours", + "web.api_rate_limit_per_min", + "web.api_localhost_exempt", + "web.sse_max_connections_per_ip", + "web.trusted_proxies", + "doh_server.enabled", + "doh_server.bind", + "doh_server.port", + "doh_server.cert_path", + "doh_server.key_path", + "dot_server.enabled", + "dot_server.bind", + "dot_server.port", + "dot_server.cert_path", + "dot_server.key_path", + "edns.ecs_mode", + "logging.level", + "logging.retention_days", + "logging.query_log_buffer_max", + "logging.hide_domains", + "logging.hide_client_ips", + "logging.output", + "logging.file_path", + "logging.max_size_mb", + "logging.max_files", + "disk.min_free_mb", + "disk.warn_free_mb", + "blocklist_update.enabled", + "blocklist_update.interval_hours", + ], + settings: { + blocking: { + response: "zero", + ttl: 0, + }, + blocklist_update: { + enabled: true, + interval_hours: 0, + }, + cache: { + negative_ttl_max: 0, + size: 0, + }, + disk: { + min_free_mb: 0, + warn_free_mb: 0, + }, + dns: { + bind_ipv4: "0.0.0.0", + bind_ipv6: "::", + port: 0, + rate_limit: 0, + rate_window_seconds: 0, + }, + doh_server: { + bind: "0.0.0.0", + cert_path: "/etc/nxdns/cert.pem", + enabled: false, + key_path: "/etc/nxdns/key.pem", + port: 0, + }, + dot_server: { + bind: "0.0.0.0", + cert_path: "/etc/nxdns/cert.pem", + enabled: false, + key_path: "/etc/nxdns/key.pem", + port: 0, + }, + edns: { + ecs_mode: "strip", + }, + logging: { + file_path: "/var/log/nxdns/nxdns.log", + hide_client_ips: false, + hide_domains: false, + level: "info", + max_files: 0, + max_size_mb: 0, + output: "stderr", + query_log_buffer_max: 0, + retention_days: 0, + }, + upstream: { + attempt_timeout_ms: 0, + read_timeout_ms: 0, + total_timeout_ms: 0, + }, + web: { + api_localhost_exempt: true, + api_rate_limit_per_min: 0, + auth_enabled: false, + bind: "0.0.0.0", + enabled: true, + port: 0, + session_ttl_hours: 0, + sse_max_connections_per_ip: 0, + trusted_proxies: "", + }, + }, +}; + +export const sample_error_bad_request: ErrorEnvelope = { + error: "logging.level: not one of the values this setting accepts", +}; + +export const sample_error_conflict: ErrorEnvelope = { + error: "an upstream with that url already exists", +}; + +export const sample_error_not_found: ErrorEnvelope = { + error: "not found", +}; + +export const sample_error_unauthorized: ErrorEnvelope = { + error: "authentication required", +}; + +export const sample_error_rate_limited: ErrorEnvelope = { + error: "rate limited", +}; diff --git a/web/src/lib/contract_samples.zig b/web/src/lib/contract_samples.zig new file mode 100644 index 0000000..ad350f1 --- /dev/null +++ b/web/src/lib/contract_samples.zig @@ -0,0 +1,9 @@ +//! Embeds the committed contract-sample golden (milestone-17 ruling 5). Module +//! root for the `contract_samples` anonymous import (test builds only) — a +//! `.ts` file cannot root one, and @embedFile paths resolve relative to this +//! file. The `docs/docs.zig` pattern. + +pub const bytes = @embedFile("contractSamples.gen.ts"); + +/// Repo-relative path, so a failing assertion names the file to regenerate. +pub const path = "web/src/lib/contractSamples.gen.ts"; diff --git a/web/src/lib/queries.ts b/web/src/lib/queries.ts index aebcbd3..a3d6819 100644 --- a/web/src/lib/queries.ts +++ b/web/src/lib/queries.ts @@ -184,11 +184,6 @@ export const ruleCreateMutation = (qc: QueryClient) => ({ onSuccess: () => invalidateRules(qc), }); -export const ruleUpdateMutation = (qc: QueryClient) => ({ - mutationFn: ({ id, input }: { id: number; input: RuleInput }) => api.updateRule(id, input), - onSuccess: () => invalidateRules(qc), -}); - export const ruleDeleteMutation = (qc: QueryClient) => ({ mutationFn: (id: number) => api.deleteRule(id), onSuccess: () => invalidateRules(qc), diff --git a/web/src/lib/settingsDiff.test.ts b/web/src/lib/settingsDiff.test.ts index c514fd7..25361e2 100644 --- a/web/src/lib/settingsDiff.test.ts +++ b/web/src/lib/settingsDiff.test.ts @@ -3,7 +3,7 @@ import type { Settings } from "@/lib/types"; function baseSettings(): Settings { return { - upstream: { read_timeout_ms: 3000, total_timeout_ms: 5000 }, + upstream: { attempt_timeout_ms: 2500, read_timeout_ms: 3000, total_timeout_ms: 5000 }, dns: { bind_ipv4: "0.0.0.0", bind_ipv6: "::", port: 53, rate_limit: 100, rate_window_seconds: 60 }, blocking: { response: "zero", ttl: 300 }, cache: { size: 10000, negative_ttl_max: 300 }, @@ -15,6 +15,7 @@ function baseSettings(): Settings { api_rate_limit_per_min: 60, api_localhost_exempt: true, sse_max_connections_per_ip: 2, + trusted_proxies: "", auth_enabled: true, }, doh_server: { enabled: false, bind: "0.0.0.0", port: 443, cert_path: "", key_path: "" }, diff --git a/web/src/lib/types.ts b/web/src/lib/types.ts index dab3fcb..b73963d 100644 --- a/web/src/lib/types.ts +++ b/web/src/lib/types.ts @@ -3,6 +3,14 @@ export type Period = "1h" | "24h" | "7d" | "30d"; +/** + * Every non-2xx JSON response: `http_util.respondError` writes this one field + * and nothing else. `ApiError.message` in api.ts reads `error` out of it. + */ +export interface ErrorEnvelope { + error: string; +} + export interface Health { status: "ok" | "degraded"; disk: { @@ -314,6 +322,7 @@ export interface TlsListenerSettings { export interface Settings { upstream: { + attempt_timeout_ms: number; read_timeout_ms: number; total_timeout_ms: number; }; @@ -340,6 +349,8 @@ export interface Settings { api_rate_limit_per_min: number; api_localhost_exempt: boolean; sse_max_connections_per_ip: number; + /** Comma-separated IP literals; empty trusts no proxy's X-Forwarded-For. */ + trusted_proxies: string; /** Derived, read-only; true iff a password hash is stored. Never sent back. */ auth_enabled: boolean; }; diff --git a/web/src/routes.tsx b/web/src/routes.tsx index 0497287..a58a62e 100644 --- a/web/src/routes.tsx +++ b/web/src/routes.tsx @@ -25,6 +25,7 @@ import { statsQuery, timeseriesQuery, upstreamHealthQuery, + upstreamsQuery, } from "@/lib/queries"; export interface RouterContext { @@ -169,6 +170,13 @@ const localDnsRoute = createRoute({ component: lazyRouteComponent(() => import("@/features/local/LocalDnsPage")), }); +const upstreamsRoute = createRoute({ + getParentRoute: () => shellRoute, + path: "/upstreams", + loader: ({ context }) => context.queryClient.ensureQueryData(upstreamsQuery()), + component: lazyRouteComponent(() => import("@/features/upstreams/UpstreamsPage")), +}); + const lookupRoute = createRoute({ getParentRoute: () => shellRoute, path: "/lookup", @@ -194,6 +202,7 @@ const routeTree = rootRoute.addChildren([ blocklistsRoute, rulesRoute, localDnsRoute, + upstreamsRoute, lookupRoute, settingsRoute, ]), diff --git a/web/src/shell/AppShell.test.tsx b/web/src/shell/AppShell.test.tsx index 0501d3f..b6eaa0e 100644 --- a/web/src/shell/AppShell.test.tsx +++ b/web/src/shell/AppShell.test.tsx @@ -14,6 +14,7 @@ const NAV_LABELS = [ "Blocklists", "Rules", "Local DNS", + "Upstreams", "Lookup", "Settings", ]; diff --git a/web/src/shell/AppShell.tsx b/web/src/shell/AppShell.tsx index 96473df..4769d29 100644 --- a/web/src/shell/AppShell.tsx +++ b/web/src/shell/AppShell.tsx @@ -16,6 +16,7 @@ const NAV_ITEMS = [ { to: "/blocklists", label: "Blocklists" }, { to: "/rules", label: "Rules" }, { to: "/local-dns", label: "Local DNS" }, + { to: "/upstreams", label: "Upstreams" }, { to: "/lookup", label: "Lookup" }, { to: "/settings", label: "Settings" }, ] as const;