Skip to content

🐛 #5483 shipped incomplete: snapshot unit test is unregistered in CI, and ProtectHome=read-only may break the push path #5504

Description

@clubanderson

PR #5499 merged the hive-snapshot.service guard for #5483. Two defects shipped with it. A second, independent implementation (#5503) got both right, which is how they surfaced.

1. The contract test is registered nowhere — it silently never runs

v2-ci.yml wires deploy tests individually. On v4 today, line 553 runs only:

run: bash src/deploy/test_hive_discord_unit_contract.sh

src/deploy/test_hive_snapshot_unit_contract.sh merged as a file and was never added. So the 184-line guard test — the thing that proves the fix works — has never executed in CI and will not execute on any future PR.

This is the exact trap #5481 documented and #5388 exists to eliminate: a guard that is present, looks complete on inspection, and asserts nothing. It is worse than an absent test, because the file’s presence reads as coverage.

Fix: add the registration step. #5503 already contains it verbatim.

2. ProtectHome=read-only is likely wrong for this unit

The merged unit sets it, with a comment claiming it "still lets git/gh read ~/.gitconfig while blocking writes outside /tmp and /var."

dashboard/publish-snapshot.sh does more than read:

line 34:  git clone --depth 1 --single-branch -b main "$DOCS_REMOTE" "$DOCS_REPO"
line 91:  git commit -s -m "chore: update hive dashboard snapshot $TIMESTAMP"
line 92:  git push origin "$SNAPSHOT_BRANCH"
line 94:  gh pr create ...
line 113: gh pr checks ...
line 135: gh pr merge ... --squash --delete-branch

gh maintains state under $HOME/.config/gh, and git may write credential/helper state during push. Mounting $HOME read-only risks failing the publish path at exactly the step that matters.

Worth being precise about what is and is not established: I verified the script calls these commands. I have not reproduced a failure on a live host with ProtectHome=read-only set — so this is a well-founded concern, not a confirmed outage. The unit is Type=oneshot on a 15-minute timer, so a failure would appear as a silently non-updating snapshot rather than a loud crash, which makes it easy to miss.

#5503’s implementation deliberately omits ProtectHome for this unit and pins the absence in its test with the reasoning attached, specifically so a later "make the units consistent" sweep cannot silently reintroduce it. That is the better call.

Why this matters beyond the two fixes

The ProtectHome divergence is concrete evidence that the hardening sweep across the remaining six units in systemd/ must be per-unit, not uniform. hive-discord.service can take ProtectHome=read-only because the bot writes nothing under $HOME; this unit cannot, because it drives gh. Copying a sibling unit’s directives without reading what the entrypoint actually does is how this defect arrived.

Suggested resolution

Take #5503’s v2-ci.yml registration and its ProtectHome omission as a follow-up to #5499. The guard script itself is correct and already merged — #5503 verified bin/hive-checkout-guard.sh is byte-identical to what #5481 landed and generalized unmodified, including keying on write bits rather than the exec bit (a 755 script is accepted, 775 refused).

Acceptance

  • test_hive_snapshot_unit_contract.sh runs in CI and is observed failing against the pre-fix unit, not merely present.
  • ProtectHome decision for this unit is settled by testing the publish path with it set, not by reasoning alone.
  • The chosen state is pinned by a test with its reasoning, so a consistency sweep cannot undo it.

Refs #5483, #5499, #5503, #5388

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    help wantedDenotes an issue that needs help from a contributor. Must meet "help wanted" guidelines.holdkind/bugCategorizes issue or PR as related to a bug.

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions