Repository navigation
feat: standing grants (part 2 of #110) - #116
Merged
Merged
Conversation
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>
groupsky
marked this pull request as ready for review
October 29, 2026 20:20
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
7 of 8 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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
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 causerenewedsummary = "mon 09:00", broker local time)/grantsbot command from the approver: the same list nowdocs/setup.mdtimer example with its own system user;docs/manual-telegram-test.mdnew checkStep 1 done (3d0a2c6): DESIGN.md has the warning (once,
warn_daysbefore the end), Renew (same length from now, capped bymax_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();/grantsis answered only for the approver's own message in the private approver chat, at most 3 a minute; the bot now also pollsmessageupdates. Tests:TestGrantsText,TestGrantsTextCut,TestGrantsCommand,TestSummary,TestListStanding.Step 4 done (534cf9a):
watchSummaryininternal/approval/summary.go, a weekly timer;[standing] summaryshown indocs/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 ingrants.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, buttonn:),poll.go,text.go,internal/audit/event.go(answerrenew, fieldrenew_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, auditTestPreapproveEvents(renew cases).Step 6 done (e3b527c, a2f9c80):
docs/setup.mdsection 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.mdcheck 15; DESIGN.md Telegram details now say the bot readsmessageupdates for/grantsonly; THREAT-MODEL.md T5 covers/grants; the DESIGN.md example of a request now uses one window flag (the client takes--dailyor--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 themax_grantscheck 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
TestStandingRenewandTestStandingRenewAfterRevoke(a Renew through the broker wiring). The security review found a CPU spin:nextEndwoke at thewarnAtof a renewal that waits for its handover, andscanWarningsskips it (grant length betweenwarn_daysand 2 ×warn_days). Fixed in 7e381ce, withTestRenewalWaitingPastItsWarningDoesNotSpin(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 vetandgolangci-lint run --build-tags e2eare clean; the new tests pass with-count=20 -cpu 1,2,4.Review findings not fixed, and why:
missed_windowalarm. The client sends only one--dailyor--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./grants@<any name>is accepted; only the approver in the private chat is answered.🤖 Generated with Claude Code