Report the addressing mode, and catch a lease that stopped renewing #3

2026-08-10T00:17:05-07:00

dejima-agent commented

2026-08-10 00:17:05 -07:00

Summary generated by LLM agent

Implements something the design spec asked for and the first pass left out: the audit now names the addressing mode, and on DHCP it checks whether the lease is actually being renewed.

This is also the first pull request that will run the runner-env job, now that it is on main.

Why this one is worth catching

It targets the worst failure in this design, and "worst" here means least visible rather than most severe.

A DHCP guest behind these rules renews through a path the firewall may be dropping. Nothing breaks at boot, because the first lease was obtained before the rules mattered. The box keeps working. Then, up to a full lease time later, it falls off the network — and it looks nothing like a firewall rule, because the change that caused it happened hours or days earlier on a different machine.

Everything else this tool checks fails immediately and loudly. This one waits.

Detected without keeping any state

The spec's own suggestion was to record the expiry on each run and watch whether it advances. That would have made this program start writing files, which it has never done, and the reason it has never done it is not incidental — a report on disk is a map of the same kind the audit refuses to keep.

It does not need to. dhclient rewrites the lease file on every successful renewal, so the file's own mtime against the renewal time recorded inside it answers the question in one pass. Past T1 with an mtime older than T1 means dhclient asked and got nothing back, which is exactly the shape of a dropped reply, and it is visible hours before the lease expires.

Static is decided first, and that ordering is the correctness

A host pinned by dejima-devbox-static-ip keeps the lease files from whatever it was doing before. If a lease file could decide the answer, every pinned host would be told it needs the DHCP rules it specifically does not need — and told so by the tool that pinned it.

A setup matching neither is reported as unknown. Not guessed, and not rounded to the friendlier answer: an unrecognised configuration is a thing to look at.

A test that failed on a shadow, not on logic

cmd_radius declares local f. My fixtures set f="$BATS_TEST_TMPDIR/cur.leases" and stubbed lease_file() to return it, so inside cmd_radius the stub closed over a shadowed empty variable and returned nothing.

The symptom pointed at the lease parser. The cause was three functions away and had nothing to do with parsing. Fixtures now use a name production code has no reason to pick, with a comment saying why, because the next person to write a stub here will reach for f too.

Verified

bats test/ passes, 71 ok and 0 failed, including seven new ones: each addressing mode, static winning over a stale lease, a lease past its renewal time, a lease inside its window, and an unparseable lease file not taking the report down with it.

Run for real in a container with neither a static stanza nor a lease, which correctly reports unknown and says what it could not tell.

One of the new tests was passing vacuously before the feature existed, by asserting only the absence of the warning string. It now also asserts the section is present, so it cannot go green against a report that says nothing about addressing at all.

What to watch on this PR

runner-env runs here for the first time. Its eight assertions have never been evaluated against a real runner — only checked for skipping cleanly with no tier declared and for failing rather than asserting nothing when handed the wrong tier. If the confined runner does not withhold the Docker socket, or the allowlist does not behave as the file assumes, this is the run that says so.

**Summary generated by LLM agent** Implements something the design spec asked for and the first pass left out: the audit now names the addressing mode, and on DHCP it checks whether the lease is actually being renewed. This is also the first pull request that will run the `runner-env` job, now that it is on `main`. ## Why this one is worth catching It targets the worst failure in this design, and "worst" here means least visible rather than most severe. A DHCP guest behind these rules renews through a path the firewall may be dropping. Nothing breaks at boot, because the first lease was obtained before the rules mattered. The box keeps working. Then, up to a full lease time later, it falls off the network — and it looks nothing like a firewall rule, because the change that caused it happened hours or days earlier on a different machine. Everything else this tool checks fails immediately and loudly. This one waits. ## Detected without keeping any state The spec's own suggestion was to record the expiry on each run and watch whether it advances. That would have made this program start writing files, which it has never done, and the reason it has never done it is not incidental — a report on disk is a map of the same kind the audit refuses to keep. It does not need to. `dhclient` rewrites the lease file on every successful renewal, so the file's own mtime against the renewal time recorded inside it answers the question in one pass. Past T1 with an mtime older than T1 means `dhclient` asked and got nothing back, which is exactly the shape of a dropped reply, and it is visible hours before the lease expires. ## Static is decided first, and that ordering is the correctness A host pinned by `dejima-devbox-static-ip` keeps the lease files from whatever it was doing before. If a lease file could decide the answer, every pinned host would be told it needs the DHCP rules it specifically does not need — and told so by the tool that pinned it. A setup matching neither is reported as `unknown`. Not guessed, and not rounded to the friendlier answer: an unrecognised configuration is a thing to look at. ## A test that failed on a shadow, not on logic `cmd_radius` declares `local f`. My fixtures set `f="$BATS_TEST_TMPDIR/cur.leases"` and stubbed `lease_file()` to return it, so inside `cmd_radius` the stub closed over a shadowed empty variable and returned nothing. The symptom pointed at the lease parser. The cause was three functions away and had nothing to do with parsing. Fixtures now use a name production code has no reason to pick, with a comment saying why, because the next person to write a stub here will reach for `f` too. ## Verified `bats test/` passes, 71 ok and 0 failed, including seven new ones: each addressing mode, static winning over a stale lease, a lease past its renewal time, a lease inside its window, and an unparseable lease file not taking the report down with it. Run for real in a container with neither a static stanza nor a lease, which correctly reports `unknown` and says what it could not tell. One of the new tests was passing vacuously before the feature existed, by asserting only the *absence* of the warning string. It now also asserts the section is present, so it cannot go green against a report that says nothing about addressing at all. ## What to watch on this PR `runner-env` runs here for the first time. Its eight assertions have never been evaluated against a real runner — only checked for skipping cleanly with no tier declared and for failing rather than asserting nothing when handed the wrong tier. If the confined runner does not withhold the Docker socket, or the allowlist does not behave as the file assumes, this is the run that says so.

dejima-agent added 1 commit 2026-08-10 00:17:06 -07:00

Report the addressing mode, and catch a lease that stopped renewing

tests / runner-env (pull_request_target) Successful in 1m40s

Details

tests / tests (pull_request_target) Successful in 1m54s

Details

6ce8c9e6c0

The design spec asked for this and the first implementation left it out. It targets the worst failure in the whole design: a DHCP guest behind these rules renews through a path the firewall may be dropping, nothing breaks at boot because the first lease predates the rules, and the VM falls off the network up to a full lease time later looking nothing like a firewall rule.

Static is decided before any lease file is consulted, and that order is the correctness of the function rather than a detail. A host pinned by dejima-devbox-static-ip keeps the leases from whatever it did before, so letting those decide would tell every pinned host it needs rules it does not need. A setup matching neither is reported as unknown rather than assumed to be the friendlier answer.

No state is kept between runs. The spec's own suggestion was to record the expiry each run and watch whether it advances, which would have made this program start writing files for the first time. It does not need to: dhclient rewrites the lease file on every successful renewal, so the file's mtime against the renewal time it records answers the question in one pass. Past T1 with an mtime older than T1 means dhclient asked and got nothing back.

One test failed on a variable shadow rather than on logic: cmd_radius declares , which shadowed the fixture path the stub closed over, so the stub returned an empty string. The fixtures now use a name production code has no reason to pick, with a comment, because the symptom pointed at the parser and the cause was three functions away.

branderson approved these changes 2026-08-10 17:44:50 -07:00

branderson merged commit d61924f731 into main

2026-08-10 17:44:55 -07:00

branderson referenced this issue from a commit

2026-08-10 17:44:57 -07:00

Merge pull request 'Report the addressing mode, and catch a lease that stopped renewing' (#3) from lease-health into main

Sign in to join this conversation.

2 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: dejima/dejima-devbox-hardener#3