Add an API regression test that excludes the four highest-count domains
and then requests `top_domains` with `count=1`, where the heap capacity is
exactly four. Before the fix the excluded domains filled the heap and were
dropped at output, so the endpoint returned nothing; now the filter runs
before the heap and the request still yields the first non-excluded domain.
References #2946
Signed-off-by: DL6ER <dl6er@dl6er.de>
Add an API regression test that requests `top_domains` with small `count`
values and asserts the result is exactly the top-K prefix of the full list,
in the right order and without dropping legitimate domains. A small count
shrinks the heap capacity to `count*4`, so this exercises the bounded top-K
eviction path guarded by the preceding fix.
References #2946
Signed-off-by: DL6ER <dl6er@dl6er.de>
The API test suite asserts exact DNS query counters. Several of them
(`TOTAL`, `DNSKEY`, `DS`, `TOP_DOMAIN`) depend on how many DNSKEY/DS
lookups dnsmasq issues while validating DNSSEC. Until now those lookups
recursed to the live ICANN root, because FTL configures the real root
trust anchors whenever `dns.dnssec` is enabled and the local PowerDNS
recursor had no root zone of its own. The number of root DNSKEY queries
therefore tracked ICANN's published root key set, so an ongoing
key-signing-key rollover silently shifted the counters (9 -> 7 DNSKEY)
and broke the suite even on unrelated PRs.
We make the whole suite hermetic:
1. Serve a locally-signed root zone from PowerDNS, forward `.` to it and
trust its key, so root DNSKEY validation resolves inside the test
environment instead of reaching the internet.
2. Mark the locally-served *unsigned* zones (`icloud.com`,
`apple-dns.net`, `in-addr.arpa`, `ip6.arpa`) as local domains, so
dnsmasq no longer proves them unsigned by walking up to the real root.
3. Give the `bogus` zone a deliberately mismatched local trust anchor so
it fails validation locally rather than by failing to find a secure
delegation at the root.
4. Drop the root-key pre-warm `dig`, an internet round-trip that no
longer serves any purpose.
With no query leaving for the real root the counters are stable and
independent of ICANN key rollovers, so they are recalibrated
accordingly. Marking the extra zones as local emits the same "negative
DS reply without NS record" warning already whitelisted for `ftl`, so
the `test_final` whitelist is broadened to match it for any zone.
Signed-off-by: DL6ER <dl6er@dl6er.de>
Cover three fixes from the security pass: session cookies carry the Secure attribute only over TLS (and never over plain HTTP), an accepted TOTP code cannot be replayed, and /api/queries clamps an unbounded length parameter. The cookie and TOTP tests live in a self-contained module so they do not disturb the order-dependent workflow in test_z_auth.py; the TOTP test clears the shared secret via the session created by the accepted login so cleanup neither trips the replay guard nor emits a 2FA warning. Account for the module's four extra pihole.toml writes in test_final.bats.
Signed-off-by: DL6ER <dl6er@dl6er.de>
CivetWeb URL-decodes local_uri_raw in place, so an encoded CR/LF (%0d%0a) in the path arrived as a literal newline. redirect_lp_handler reflected that path into the Location header of a 301 redirect verbatim, before any authentication, allowing unauthenticated HTTP response-header injection / response splitting (e.g. Set-Cookie fixation). Reject any request whose decoded URI contains control characters from a begin_request hook, which runs before auth and every handler, closing the whole class of URI-into-header injection.
The rejection is logged only at debug level and never echoes the URI: begin_request runs for every unauthenticated request, so logging each one at warning level (or logging the URI verbatim) would itself be a log-flooding / log-injection vector.
Add raw-socket API tests (TestURIControlCharRejection) verifying an encoded CR/LF path is rejected with 400 and not reflected into response headers, a bare control character is rejected, and a clean .lp request is still served.
Signed-off-by: DL6ER <dl6er@dl6er.de>
The API query-count assertions depend on resolving mask.icloud.com, whose
mask.icloud.com -> mask.apple-dns.net CNAME chain was recursed to the public
internet. dnsmasq fires extra DNSKEY validation queries depending on whether
Apple currently DNSSEC-signs icloud.com / apple-dns.net, and Apple toggles this
over time. The runtime DS-probing workaround in conftest.py could not reliably
model dnsmasq's behaviour (e.g. when Apple returns SERVFAIL on DS), so the suite
went flaky again.
Serve the icloud.com and apple-dns.net zones from the local authoritative
PowerDNS server instead, so the chain resolves hermetically and the query counts
are deterministic regardless of Apple's upstream DNSSEC posture. The DS-probing
fixture is dropped and the expected counters become fixed constants again.
Signed-off-by: DL6ER <dl6er@dl6er.de>
The pytest API tests assert exact query counts that depend on whether icloud.com and apple-dns.net are DNSSEC-signed. When Apple removed DNSSEC from those zones (April 2026), dnsmasq stopped firing two DNSKEY validation queries during the mask.icloud.com CNAME chain walk, breaking 7 tests on every CI run - including re-runs of previously green commits.
Instead of hardcoding either set of numbers, detect the current DNSSEC state at test startup by querying the local pdns_recursor (port 5555, bypassing FTL to avoid counter pollution) for DS records on both domains. Four module-level constants (TOTAL, FORWARDED, DNSKEY, TOP_DOMAIN) are set accordingly, and the 11 affected assertions now reference these constants.
The bats "Special domain: Record is returned when explicitly allowed" test is preserved unchanged - the hybrid detection makes it safe regardless of upstream DNSSEC posture.
Signed-off-by: Dominik <dl6er@dl6er.de>
Add missing NULL check after strdup() when making a writable copy of
cJSON reference strings in api_list_write(). Without this, a failed
allocation under memory pressure would dereference NULL in the
lowercasing loop. Also fix minor docstring typo in punycode test.
Signed-off-by: Dominik <dl6er@dl6er.de>
When adding or searching for exact domains, the API unconditionally
passes the input through idn2_to_ascii_lz() for IDN normalization.
This round-trips punycode domains: decode to Unicode, validate against
IDNA2008, re-encode to ASCII. Characters like emoji are disallowed by
IDNA2008 (RFC 5892), so valid punycode domains such as
xn--4ca0bs45142c.com (äöü😀.com) are rejected with "string contains
a disallowed character" even though they are perfectly valid DNS names.
Fix by checking whether the input is already pure ASCII before calling
idn2_to_ascii_lz(). If every byte is <= 0x7F, skip IDN conversion
entirely — the domain is already in a DNS-compatible form and only
needs lowercasing and valid_domain() validation. Non-ASCII input
(actual Unicode domains) still goes through the IDN conversion path.
Applied to both the list API (src/api/list.c) and the search API
(src/api/search.c).
Fixes: https://github.com/pi-hole/FTL/issues/2837
Signed-off-by: Dominik <dl6er@dl6er.de>
Add pytest tests for all previously untested GET API endpoints:
dns/blocking, domains (all type/kind combinations and single lookup),
groups, stats/summary, stats/top_domains, stats/top_clients,
stats/upstreams, stats/query_types, stats/recent_blocked,
stats/database (error handling), dhcp/leases, endpoints, info/ftl,
info/login, info/version, info/messages, info/client, info/database,
info/system, network/devices, network/interfaces, logs (dnsmasq, ftl,
webserver), and padd.
All assertions use exact expected values derived from the deterministic
BATS DNS query seeding (137 total queries, 49 blocked, 47 forwarded,
41 cached, 11 active clients, 8 gravity domains). On failure, the
full JSON response is dumped to /tmp/ftl_test_*.json for easy
inspection.
Fix double-free bug in printFTLenv() (src/config/env.c): when
printFTLenv() was called more than once (e.g. after config reload
triggered by the CLI password test), it would free item->error a
second time because neither the pointer nor the error_allocated flag
were reset after the first free. This produced "Trying to free NULL
pointer in printFTLenv()" warnings. Fix: set item->error = NULL and
item->error_allocated = false after freeing.
Files modified:
src/config/env.c — reset error/error_allocated after free
test/api/test_api.py — add 34 new endpoint tests (22 -> 56 total)
Signed-off-by: Dominik <dl6er@dl6er.de>
DL6ER
DominikandGitHub
Henry SowellandGitHub
