CHECK_EPOCH freshness is host-conditional: busybox date silently drops %N #125

Open
opened 2026-08-09 08:42:18 +02:00 by clawbot · 0 comments
Collaborator

Follow-up to PR #122 (closes #115), from the re-review. Non-blocking there, worth closing properly.

script/cibuild passes --build-arg CHECK_EPOCH="$(date +%s%N)". %N is a GNU/uutils extension, not POSIX.

The reviewer checked it against this repo's own pinned alpine image: busybox date +%s%N prints 1786257437 — it silently drops %N and exits 0. So on a busybox host the epoch degrades from nanosecond to second granularity.

Why this is worth four characters

The degradation itself is benign — second granularity is exactly where the PR stood before the %N change, and sequential builds take ~45s so they cannot collide. The problem is the mismatch between guarantee and description:

  • script/cibuild's header comment and the README describe the freshness unconditionally.
  • It is actually conditional on the host's date implementation, and the degradation is silent — no warning, exit 0, nothing in the build output distinguishes a nanosecond epoch from a second-granular one except the digit count.

Two concurrent invocations within the same second on a busybox host would produce identical epochs, and the second would be served from the first's layer. That is the original #115 false green, narrowed to a race window rather than eliminated.

The reviewer also chased the catastrophic path rather than assuming it away, and the result is worth recording: a constant CHECK_EPOCH would require date to exit non-zero, which no strftime implementation does for an unknown conversion specifier — so it cannot realistically happen. But they verified in dash that if it ever did, set -eu would not catch it: a failing command substitution inside a command's arguments does not trip set -e. The build would silently receive CHECK_EPOCH= empty and constant, and the false green would return by a different door.

Definition of done

  1. CHECK_EPOCH is unique per invocation on every host, regardless of whether date supports %N. Appending the shell PID — $(date +%s%N)$$ — achieves this in four characters: $$ differs between concurrent invocations even when the seconds field is identical, and it is POSIX.
  2. The value cannot be empty or constant even if the date substitution fails. Consider whether to assert non-emptiness explicitly rather than relying on date never failing.
  3. script/cibuild's header comment and the README describe the actual guarantee, without overclaiming.
  4. script/cibuild remains POSIX sh (#!/bin/sh, set -eu, no bashisms); sh -n clean.
  5. Verify on both shapes. Confirm uniqueness holds when %N is honoured and when it is dropped — the pinned alpine image in this repo's Dockerfile gives you a genuine busybox date to test against, so this can be checked for real rather than reasoned about.
  6. script/cibuild still actually runs the suite: two consecutive runs on an unchanged tree, both executing the check step with distinct epochs, neither CACHED nor sub-second. Report both timings.
  7. make check green; TODO.md updated in the same commit.

The finishing commit's title must end with (closes #N) referencing this issue.

Hard constraints

  • All pins unchanged: golang/alpine sha256 digests, golangci-lint c0d3ddc9cf3faa61a4e378e879ece580256d76e5, goimports 009367f5c17a8d4c45a961a3a509277190a9a6f0.
  • .golangci.yml unmodified — sha256 021cc83f4e6fc7c31b95b34b846723dfcf20b66b7baeea1dc40406e643346bcb.
  • Do not weaken the gate; make check still runs in full.
  • DNS is never mocked in this repository; nothing here touches test behaviour.
  • Lint runs may be VOID (#121): void if the output contains parallel golangci-lint is running, or reports any path starting with ../, or any absolute path outside your worktree. Retry; never record a void run.

Sequencing

Land after PR #122, which introduces the line being changed.

Upstream

script/cibuild is byte-identical across repos and this fix is being adopted as the canonical template change (prompts #26), so the $$ refinement should go upstream too rather than living only here.

Follow-up to [PR #122](https://git.eeqj.de/sneak/dnswatcher/pulls/122) (closes #115), from the re-review. Non-blocking there, worth closing properly. `script/cibuild` passes `--build-arg CHECK_EPOCH="$(date +%s%N)"`. `%N` is a GNU/uutils extension, **not POSIX**. The reviewer checked it against **this repo's own pinned alpine image**: busybox `date +%s%N` prints `1786257437` — it **silently drops `%N` and exits 0**. So on a busybox host the epoch degrades from nanosecond to second granularity. ## Why this is worth four characters The degradation itself is benign — second granularity is exactly where the PR stood before the `%N` change, and sequential builds take ~45s so they cannot collide. The problem is the mismatch between guarantee and description: - `script/cibuild`'s header comment and the README describe the freshness **unconditionally**. - It is actually conditional on the host's `date` implementation, and the degradation is **silent** — no warning, exit 0, nothing in the build output distinguishes a nanosecond epoch from a second-granular one except the digit count. Two concurrent invocations within the same second on a busybox host would produce identical epochs, and the second would be served from the first's layer. That is the original #115 false green, narrowed to a race window rather than eliminated. The reviewer also chased the catastrophic path rather than assuming it away, and the result is worth recording: a *constant* `CHECK_EPOCH` would require `date` to exit non-zero, which no strftime implementation does for an unknown conversion specifier — so it cannot realistically happen. **But** they verified in `dash` that if it ever did, `set -eu` would **not** catch it: a failing command substitution inside a command's *arguments* does not trip `set -e`. The build would silently receive `CHECK_EPOCH=` empty and constant, and the false green would return by a different door. ## Definition of done 1. `CHECK_EPOCH` is unique per invocation on **every** host, regardless of whether `date` supports `%N`. Appending the shell PID — `$(date +%s%N)$$` — achieves this in four characters: `$$` differs between concurrent invocations even when the seconds field is identical, and it is POSIX. 2. The value cannot be empty or constant even if the `date` substitution fails. Consider whether to assert non-emptiness explicitly rather than relying on `date` never failing. 3. `script/cibuild`'s header comment and the README describe the actual guarantee, without overclaiming. 4. `script/cibuild` remains POSIX `sh` (`#!/bin/sh`, `set -eu`, no bashisms); `sh -n` clean. 5. **Verify on both shapes.** Confirm uniqueness holds when `%N` is honoured *and* when it is dropped — the pinned alpine image in this repo's `Dockerfile` gives you a genuine busybox `date` to test against, so this can be checked for real rather than reasoned about. 6. `script/cibuild` still actually runs the suite: two consecutive runs on an unchanged tree, both executing the check step with distinct epochs, neither `CACHED` nor sub-second. Report both timings. 7. `make check` green; `TODO.md` updated in the same commit. The finishing commit's title must end with ` (closes #N)` referencing this issue. ## Hard constraints - **All pins unchanged**: `golang`/`alpine` `sha256` digests, golangci-lint `c0d3ddc9cf3faa61a4e378e879ece580256d76e5`, goimports `009367f5c17a8d4c45a961a3a509277190a9a6f0`. - **`.golangci.yml` unmodified** — sha256 `021cc83f4e6fc7c31b95b34b846723dfcf20b66b7baeea1dc40406e643346bcb`. - **Do not weaken the gate**; `make check` still runs in full. - DNS is never mocked in this repository; nothing here touches test behaviour. - **Lint runs may be VOID** (#121): void if the output contains `parallel golangci-lint is running`, or reports any path starting with `../`, or any absolute path outside your worktree. Retry; never record a void run. ## Sequencing **Land after PR #122**, which introduces the line being changed. ## Upstream `script/cibuild` is byte-identical across repos and this fix is being adopted as the canonical template change (`prompts` #26), so the `$$` refinement should go upstream too rather than living only here.
clawbot added this to the 1.0 milestone 2026-08-09 08:42:18 +02:00
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: sneak/dnswatcher#125