Skip to content

feat: standing grants (part 2 of #110) - #116

Merged
groupsky merged 16 commits into
mainfrom
feat/standing-grants-110-part2
Oct 29, 2026
Merged

groupsky merged 16 commits into
mainfrom
feat/standing-grants-110-part2

Conversation

@groupsky

@groupsky groupsky commented Oct 29, 2026 •

Copy link
Copy Markdown
Contributor

Part 2 of #110 (standing grants). Refs #110. Part 1 is #115 (merged).

The design is in the plan comment on #110 and the owner answers of 2026-10-29 (A: dropped at a broker stop, B: an alarm ends the grant, C: secrets only, D: Renew as proposed).

Part 2: missed-window alarm, warning and Renew, weekly summary, /grants, docs.

Progress

  • 1. DESIGN.md and THREAT-MODEL.md: check and fill the spec for the part 2 features
  • 2. missed-window alarm: the timer wakes at each window end; at most one alarm per window
  • 3. warning before the end (warn_days) with a Renew button; Renew makes a new grant after a full tap check; the next use of the old token gets the new grant's token; the old grant ends with cause renewed
  • 4. weekly summary (summary = "mon 09:00", broker local time)
  • 5. /grants bot command from the approver: the same list now
  • 6. docs: docs/setup.md timer example with its own system user; docs/manual-telegram-test.md new check
  • 7. security review of the whole part, Go 1.27 and 1.26, stress tests, ready

Step 1 done (3d0a2c6): DESIGN.md has the warning (once, warn_days before the end), Renew (same length from now, capped by max_days), the summary (local time, no token or hash), and a "Missed window" paragraph. THREAT-MODEL.md needed no change.
Step 2 done (a9f4f54): internal/approval/missed.go; the timer wakes at the next window end; the audit entry is written, then the alarm is sent; a failed send is logged, the grant stays. Tests: TestMissedWindowAlarm, TestNoMissedAlarmAfterAUsedWindow, TestMissedAlarmGivenBackUseStillMissed, TestMissedAlarmSendFails, TestNoMissedAlarmAfterTheGrantEnds, TestEachWindowEndSkippedHour. The stress run was not repeated after the last small fix: repeat it in step 7.

Step 5 done (082d79f): Core.ListStanding(); /grants is answered only for the approver's own message in the private approver chat, at most 3 a minute; the bot now also polls message updates. Tests: TestGrantsText, TestGrantsTextCut, TestGrantsCommand, TestSummary, TestListStanding.
Step 4 done (534cf9a): watchSummary in internal/approval/summary.go, a weekly timer; [standing] summary shown in docs/setup.md. With no standing grants it sends "No standing grants." Tests: TestNextSummary, TestWeeklySummary, TestNoSummaryWithoutStanding, TestSummaryNotRepeatedAfterClockSetBack.

Step 3 done (68d8c3d, 2295c03, 7f4d07e, 850a17e, b81cbc3): internal/approval/renew.go (Warning, Warner, OnRenewed, scanWarnings, renew, handOver), changes in grants.go (a renewal ends with its grant, same cause; handover at the old end), standinguse.go (handover on the next use of the old token), internal/telegram/renew.go (Warn, button n:), poll.go, text.go, internal/audit/event.go (answer renew, field renew_of), internal/broker/broker.go (writes the Renew entry before the new grant exists). The warning goes to the private chat; after Renew it becomes the new grant's message with Revoke. The handover also happens at the old grant's end, and old hashes move to the new grant, so an old token stays an alarm. DESIGN.md (sections 7, 8, 9) and THREAT-MODEL.md (T36) updated. Tests: TestWarningOnceBeforeTheEnd, TestWarningSendFailsNotRepeated, TestNoWarningForAShortGrant, TestNoWarningAfterTheGrantEnded, TestRenewHandsOverOnTheNextUse, TestOldTokenAfterRenewalIsAnAlarm, TestRenewHandsOverAtTheOldEnd, TestRevokeOfARenewedGrantEndsTheRenewal, TestRenewRefused, TestRenewKeepsToMaxDaysAndMaxGrants, TestRenewData, TestWarningText, TestWarnAndRenewTap, audit TestPreapproveEvents (renew cases).

Step 6 done (e3b527c, a2f9c80): docs/setup.md section 11 "A job that runs every night: standing grants" (own system user, systemd service and timer, token file rules, alarms, Renew, /grants); docs/manual-telegram-test.md check 15; DESIGN.md Telegram details now say the bot reads message updates for /grants only; THREAT-MODEL.md T5 covers /grants; the DESIGN.md example of a request now uses one window flag (the client takes --daily or --weekly, not both).

Step 7 in progress: a647e48 caps the missed-window alarms to one per grant in one scan (test TestOneMissedAlarmAfterAWallClockJump; DESIGN.md "Missed window"). Decided: keep the max_grants check on Renew (it only fails after a config change, which is the wanted meaning); no retry of a warning that fails to send (the person can make a new grant; lean); /grants@<name> stays (only the approver in the private chat is answered).

Step 7 done: f2c9e4f adds TestStandingRenew and TestStandingRenewAfterRevoke (a Renew through the broker wiring). The security review found a CPU spin: nextEnd woke at the warnAt of a renewal that waits for its handover, and scanWarnings skips it (grant length between warn_days and 2 × warn_days). Fixed in 7e381ce, with TestRenewalWaitingPastItsWarningDoesNotSpin (it times out without the fix). The fix round of the review: nothing blocking. go test -race ./... passes with CI=true on Go 1.27.1 and 1.26.0; go vet and golangci-lint run --build-tags e2e are clean; the new tests pass with -count=20 -cpu 1,2,4.

Review findings not fixed, and why:

  • Low: two windows of one grant that overlap: a use marks only the first, so the second may give a false missed_window alarm. The client sends only one --daily or --weekly, which gives no overlap; only a hand-made request can do it, and the result is a false alarm to the approver, not a wider grant.
  • Low (cosmetic): if the grant ends while its warning is being sent, the warning keeps a Renew button that does nothing; a tap gets "This request has ended."
  • /grants@<any name> is accepted; only the approver in the private chat is answered.

🤖 Generated with Claude Code

groupsky and others added 15 commits October 29, 2026 12:28
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
)

Also the test of the standing message in DESIGN.md now has one window.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@groupsky
groupsky marked this pull request as ready for review October 29, 2026 20:20
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@groupsky
groupsky merged commit 3442a2a into main Oct 29, 2026
7 checks passed
@groupsky
groupsky deleted the feat/standing-grants-110-part2 branch October 29, 2026 20:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant