Restore the import DNS exception and close temporary rules on every exit #159

Merged

mysticalsoap merged 1 commit from import-dns-exception into trunk

2026-08-24 19:50:50 -04:00

mysticalsoap commented

2026-08-24 17:42:54 -04:00

Owner

Problem

Upstream removed the dns_request_exception("-I") call in 2019 (e6a7818) but left the "-D" in downloaded() dangling — every import since has logged eight failed rule deletions, and with the firewall active and no tunnel up, imports couldn't resolve anything: the default ruleset's OUTPUT policy is DROP with no port-53 exception (LAN resolvers only work with block_lan=0). Surfaced while live-verifying #157 (#158).

Digging into the cleanup path also exposed a second, hidden leak: the per-IP api rules allow_ip inserts were only ever deleted in copy_certs — a failed import left them behind permanently.

Fix

The insert returns at the top of allow_ip, which every import passes through before its first resolution — custom imports included (they resolve remote hostnames from config files and were the original reason for the exception).
Cleanup becomes revoke_api_access on the thread (per-IP rules + DNS exception, idempotent), called from all three exits: copy_certs (success), fail() (handled failures), report_failure (crashes).
The dangling "-D" in the service's downloaded() is gone, as are dns_request_exception's dead dns_1/dns_2 parameters (only port was ever read).

Known accepted edge: concurrent imports share one rule copy (add_rule dedups via -C), so the first to finish can close port 53 under a sibling still resolving — the loser reports a network error and the scheduler retries next cycle. Refcounting rules across threads wasn't worth it.

Verification

pytest green (356 passed) plus the ruff/compileall gates.
New TestApiAccessLifecycle: handled failure and crash both close exactly what was opened (DNS pair + per-IP pair, in order); the success-side close is pinned against the real copy_certs since every other test stubs it.
Live smoke after merge: an import (or scheduled renewal) should now log zero iptables: failed to apply lines — and one "adding exception for DNS requests" before "removing".

Failed imports no longer leak the per-IP api ACCEPT rules — same lifecycle, same cleanup call, so it rode along rather than becoming its own PR.

Closes #158.

🤖 Generated with Claude Code

## Problem Upstream removed the `dns_request_exception("-I")` call in 2019 (e6a7818) but left the `"-D"` in `downloaded()` dangling — every import since has logged eight failed rule deletions, and with the firewall active and no tunnel up, imports couldn't resolve anything: the default ruleset's OUTPUT policy is DROP with no port-53 exception (LAN resolvers only work with `block_lan=0`). Surfaced while live-verifying #157 (#158). Digging into the cleanup path also exposed a second, hidden leak: the per-IP api rules `allow_ip` inserts were only ever deleted in `copy_certs` — a failed import left them behind permanently. ## Fix - The insert returns at the top of `allow_ip`, which every import passes through before its first resolution — custom imports included (they resolve `remote` hostnames from config files and were the original reason for the exception). - Cleanup becomes `revoke_api_access` on the thread (per-IP rules + DNS exception, idempotent), called from all three exits: `copy_certs` (success), `fail()` (handled failures), `report_failure` (crashes). - The dangling `"-D"` in the service's `downloaded()` is gone, as are `dns_request_exception`'s dead `dns_1`/`dns_2` parameters (only `port` was ever read). Known accepted edge: concurrent imports share one rule copy (`add_rule` dedups via `-C`), so the first to finish can close port 53 under a sibling still resolving — the loser reports a network error and the scheduler retries next cycle. Refcounting rules across threads wasn't worth it. ## Verification - `pytest` green (356 passed) plus the ruff/compileall gates. - New `TestApiAccessLifecycle`: handled failure and crash both close exactly what was opened (DNS pair + per-IP pair, in order); the success-side close is pinned against the real `copy_certs` since every other test stubs it. - Live smoke after merge: an import (or scheduled renewal) should now log zero `iptables: failed to apply` lines — and one "adding exception for DNS requests" before "removing". ## Related fixes (found along the way, technically out of scope) - Failed imports no longer leak the per-IP api ACCEPT rules — same lifecycle, same cleanup call, so it rode along rather than becoming its own PR. Closes #158. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

mysticalsoap added 1 commit

2026-08-24 17:42:54 -04:00

fix: open the import's DNS exception again, and close every rule on failure

ci / test (pull_request) Successful in 27s

Details

ci / test (push) Successful in 28s

Details

aa4cc02916

Upstream removed the dns_request_exception("-I") call in 2019
(e6a7818) and left the delete in downloaded() dangling: every import
since has logged eight failed rule removals, and with the firewall up
and no tunnel, nothing could resolve -- not the api hosts, not the
hostnames a custom config carries (#158).

The insert now lives at the top of allow_ip, which every import passes
through before its first resolution, custom imports included. Cleanup
moves onto the thread as revoke_api_access, called from all three
exits: copy_certs on success, fail() for handled failures and
report_failure for crashes. That also closes a second leak the one-exit
cleanup hid -- the per-IP api rules were only ever deleted on success,
so a failed import left them behind.

Concurrent imports share one rule copy (add_rule dedups via -C), so the
first to finish can close port 53 under a sibling still resolving; the
loser fails as a network error and the scheduler retries next cycle --
accepted over refcounting rules across threads.

dns_request_exception's dns_1/dns_2 parameters were dead (only port is
read) and are gone. Closes #158.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

mysticalsoap referenced this pull request

2026-08-24 18:34:29 -04:00

Cgroup teardown logs expected rule misses as warnings #161

mysticalsoap merged commit aa4cc02916 into trunk

2026-08-24 19:50:50 -04:00

mysticalsoap deleted branch import-dns-exception

2026-08-24 19:50:50 -04:00

mysticalsoap referenced this pull request

2026-08-24 19:56:07 -04:00

Quiet the log: expected states stop reporting as failures #163