From 788726c5de790be987a57b072720a82c7b98089f Mon Sep 17 00:00:00 2001 From: Geno Roupsky <185580+groupsky@users.noreply.github.com> Date: Thu, 29 Oct 2026 12:28:53 +0200 Subject: [PATCH 01/16] chore: start standing grants part 2 (#110) Co-Authored-By: Claude Opus 5.5 From 3d0a2c616f5147414631efb74b14166abc421c69 Mon Sep 17 00:00:00 2001 From: Geno Roupsky <185580+groupsky@users.noreply.github.com> Date: Thu, 29 Oct 2026 12:29:41 +0200 Subject: [PATCH 02/16] docs: the part 2 features of standing grants (#110) Co-Authored-By: Claude Opus 5.5 --- DESIGN.md | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/DESIGN.md b/DESIGN.md index 4bb3c4c..1442206 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -1565,12 +1565,21 @@ normal way, with a tap. **On the phone.** The message of the grant has a **Revoke** button, as in a scheduled grant. Within `warn_days` of the end, a warning comes with a -**Renew** button. A tap on Renew, after the full checks of section 7, makes -a new grant (new ID, new end, the same scope). The next use of the old -token gets the token of the new grant, and the old grant ends with cause -`renewed`. Once a week (`summary`) a message lists each standing grant: the -client, the secrets, the windows, the last use and the end date. The bot -command `/grants` from the approver shows the same list. +**Renew** button. The warning is sent `warn_days` before the end (default +7), once per grant. A tap on Renew, after the full checks of section 7, +makes a new grant: a new ID, a new end (the same length as the old grant, +counted from now, and not more than `max_days`), and the same scope. The +next use of the old token gets the token of the new grant, and the old +grant ends with cause `renewed`. Once a week (`summary`, broker local +time) a message lists each standing grant: the client, the secrets, the +windows, the last use and the end date. The bot command `/grants` from the +approver shows the same list. The list never shows a token or a hash. + +**Missed window.** When a window of a live grant ends with no use in it, +the broker sends the alarm `missed_window`. The grant stays. A window that +ended before the grant started does not count. A window in the hour that a +clock change skips does not open, so it also gives this alarm. Each grant +sends at most one alarm in a window. **End.** A standing grant ends when its end date comes, on Revoke, on an alarm that ends it, on a renewal, on a cancel by its own client (`nodkey From a9f4f54da42df554135673d14f445221cdd810dd Mon Sep 17 00:00:00 2001 From: Geno Roupsky <185580+groupsky@users.noreply.github.com> Date: Thu, 29 Oct 2026 12:33:43 +0200 Subject: [PATCH 03/16] feat: the missed-window alarm of standing grants (#110) Co-Authored-By: Claude Opus 5.5 --- internal/approval/approval.go | 4 + internal/approval/decide.go | 2 +- internal/approval/grants.go | 38 ++++- internal/approval/missed.go | 144 +++++++++++++++++ internal/approval/missed_internal_test.go | 27 ++++ internal/approval/missed_test.go | 188 ++++++++++++++++++++++ internal/approval/window_test.go | 6 +- internal/broker/broker.go | 5 + internal/broker/standinguse_test.go | 5 +- internal/telegram/alarm_test.go | 6 +- internal/telegram/text.go | 2 + 11 files changed, 413 insertions(+), 14 deletions(-) create mode 100644 internal/approval/missed.go create mode 100644 internal/approval/missed_internal_test.go create mode 100644 internal/approval/missed_test.go diff --git a/internal/approval/approval.go b/internal/approval/approval.go index 6382250..9e819f8 100644 --- a/internal/approval/approval.go +++ b/internal/approval/approval.go @@ -117,6 +117,10 @@ type Core struct { // watchGrants hands them to onEnded. endedQueue []audit.GrantEnded onEnded func(audit.GrantEnded) + // alarmQueue holds the missed-window alarms, until watchGrants writes + // and sends them. + alarmQueue []missedAlarm + onAlarm func(audit.GrantAlarm) // msgQueue holds the updates of the messages of scheduled grants that // ended, until watchGrants makes them. msgQueue []msgUpdate diff --git a/internal/approval/decide.go b/internal/approval/decide.go index 8142206..c6d932a 100644 --- a/internal/approval/decide.go +++ b/internal/approval/decide.go @@ -661,7 +661,7 @@ func (c *Core) makeGrantID(p *pending) error { func (c *Core) approvePlan(p *pending, res *Result) { res.Subject.GrantID = p.plan.id g := *p.plan - g.reqID = p.id + g.reqID, g.code = p.id, p.code g.origin = res.Subject g.origin.Secrets = slices.Clone(res.Subject.Secrets) g.origin.Hosts = slices.Clone(res.Subject.Hosts) diff --git a/internal/approval/grants.go b/internal/approval/grants.go index aa43fc0..85f1dd9 100644 --- a/internal/approval/grants.go +++ b/internal/approval/grants.go @@ -74,6 +74,18 @@ type grant struct { oldHashes [][32]byte // alarmMark is the day of the last alarm outside the windows. alarmMark string + // code is the short code of the request that made the grant, for its + // alarms. + code string + // scanned is the wall time up to which the windows of a standing grant + // were checked for a missed window. It starts at the later of the start + // and the approval, and never goes back. + scanned time.Time + // used holds the openings of windows that had a use and have not been + // checked yet. + used map[windowKey]bool + // marked is the opening that the last use marked in used. + marked windowKey } // grantKind is the kind of a grant (DESIGN.md section 8, "The grant core"). @@ -166,6 +178,10 @@ func (g *grant) tryUse(now moment) useResult { return useOverUses } g.winUsed++ + if g.used == nil { + g.used = make(map[windowKey]bool) + } + g.used[key], g.marked = true, key return useOK } @@ -174,6 +190,9 @@ func (g *grant) tryUse(now moment) useResult { func (g *grant) giveBackUse() { if g.standing() { g.winUsed = max(g.winUsed-1, 0) + if g.winUsed == 0 { + delete(g.used, g.marked) + } return } g.uses++ @@ -403,10 +422,13 @@ func (c *Core) deliverEnded() { func (c *Core) watchGrants(ctx context.Context) { for { c.mu.Lock() - c.sweepGrants(c.now()) + now := c.now() + c.sweepGrants(now) + c.scanMissed(now) next, ok := c.nextEnd() c.mu.Unlock() c.deliverEnded() + c.deliverAlarms() c.deliverMessages(c.stopCtx) var timer <-chan time.Time var t Timer @@ -431,9 +453,9 @@ func (c *Core) watchGrants(ctx context.Context) { // nextEnd returns the earliest time that watchGrants must wake for, on // the monotonic clock: the end of an audited grant, or the end of the -// current window of a standing grant (the missed-window alarm of part 2 -// reads it). The end of a window is the wall clock time left in it, added -// to the monotonic clock; if the clocks change inside the window, the wake +// next window of a standing grant (scanMissed looks for a missed +// window then). The end of a window is the wall clock time left until it, +// added to the monotonic clock; if the clocks change inside the window, the wake // is early or late, and the one who reads it looks again. Core.mu must be // held. func (c *Core) nextEnd() (time.Time, bool) { @@ -451,7 +473,7 @@ func (c *Core) nextEnd() (time.Time, bool) { } take(g.end.mono) if g.standing() && now.before(g.end) { - if _, left, in := windowAt(g.windows, now.wall); in { + if left, ok := nextWindowEnd(g, now.wall); ok { take(now.mono.Add(left)) } } @@ -661,6 +683,12 @@ func (c *Core) startScheduled(r *Result, now moment) { } } } + if g.standing() { + g.scanned = now.wall + if g.start.After(g.scanned) { + g.scanned = g.start + } + } c.grants = append(c.grants, g) r.live = true if r.msg != "" { diff --git a/internal/approval/missed.go b/internal/approval/missed.go new file mode 100644 index 0000000..4c9b7e7 --- /dev/null +++ b/internal/approval/missed.go @@ -0,0 +1,144 @@ +package approval + +import ( + "log/slog" + "slices" + "time" + + "github.com/code6x/nodkey/internal/audit" +) + +// missedAlarm is the alarm of a window that ended with no use (DESIGN.md +// section 8): the entry for the audit log and the message for the approver. +type missedAlarm struct { + ev audit.GrantAlarm + al Alarm +} + +// maxScanDays is the most days that scanMissed looks back. A grant lasts +// at most 366 days. +const maxScanDays = 400 + +// windowEnd returns the end of the opening of window w on the date of +// day. A window in the hour that a clock change skips has the end that +// time.Date gives; it never opened, so it counts as missed. +func windowEnd(w window, d time.Time) time.Time { + y, m, dd := d.Date() + mins := int(w.to / time.Minute) + return time.Date(y, m, dd, mins/60, mins%60, 0, 0, d.Location()) +} + +// eachWindowEnd calls f for each opening of a window of ws whose end is +// after from and not after to, in the order of the days. f gets the +// key of the opening and its end. +func eachWindowEnd(ws []window, from, to time.Time, f func(windowKey, time.Time)) { + if !to.After(from) { + return + } + y, m, d := from.Date() + day0 := time.Date(y, m, d, 0, 0, 0, 0, from.Location()) + for i := 0; i < maxScanDays; i++ { + day := day0.AddDate(0, 0, i) + if day.After(to) { + return + } + for idx, w := range ws { + if !w.valid() || !w.days.has(day.Weekday()) { + continue + } + if end := windowEnd(w, day); end.After(from) && !end.After(to) { + f(windowKey{day: dayOf(day), index: idx}, end) + } + } + } +} + +// nextWindowEnd returns the time from wall to the end of the next opening +// of a window of g that ends after wall and before the end of the grant. +func nextWindowEnd(g *grant, wall time.Time) (time.Duration, bool) { + var best time.Time + y, m, d := wall.Date() + day0 := time.Date(y, m, d, 0, 0, 0, 0, wall.Location()) + for i := 0; i < 8 && best.IsZero(); i++ { // a window comes at least every 7 days + day := day0.AddDate(0, 0, i) + for _, w := range g.windows { + if !w.valid() || !w.days.has(day.Weekday()) { + continue + } + if end := windowEnd(w, day); end.After(wall) && (best.IsZero() || end.Before(best)) { + best = end + } + } + } + if best.IsZero() || best.After(g.end.wall) { + return 0, false + } + return best.Sub(wall), true +} + +// scanMissed finds the windows of standing grants that ended since the +// last scan with no use in them, and queues one alarm for each. The grant +// stays. Each opening is looked at once, because scanned only moves +// forward. Core.mu must be held. +func (c *Core) scanMissed(now moment) { + for _, g := range c.grants { + if !g.standing() || !now.wall.After(g.scanned) { + continue + } + from := g.scanned + g.scanned = now.wall + eachWindowEnd(g.windows, from, now.wall, func(k windowKey, end time.Time) { + if g.used[k] { + delete(g.used, k) + return + } + if !end.Before(g.end.wall) { + return // the grant ended before this window did + } + sub := g.origin + sub.Secrets = slices.Clone(g.origin.Secrets) + sub.Hosts = slices.Clone(g.origin.Hosts) + sub.Windows = slices.Clone(g.origin.Windows) + c.alarmQueue = append(c.alarmQueue, missedAlarm{ + ev: audit.GrantAlarm{Subject: sub, Cause: audit.AlarmMissedWindow}, + al: Alarm{RequestID: g.reqID, ShortCode: g.code, Client: g.client, GrantID: g.id, + Secrets: slices.Clone(sub.Secrets), Cause: audit.AlarmMissedWindow}, + }) + }) + } +} + +// OnGrantAlarm sets the function that gets the audit entry of each +// missed-window alarm. The broker writes the entry. f is called from the +// core's own goroutine, not under its lock. Call it before Run. +func (c *Core) OnGrantAlarm(f func(audit.GrantAlarm)) { + c.mu.Lock() + defer c.mu.Unlock() + c.onAlarm = f +} + +// deliverAlarms writes the entries of the queued alarms, then sends the +// messages. A send that fails is logged: the entry is written, and the +// grant stays. The alarm of that window is not tried again. +func (c *Core) deliverAlarms() { + c.mu.Lock() + q, f := c.alarmQueue, c.onAlarm + c.alarmQueue = nil + c.mu.Unlock() + for _, a := range q { + if f != nil { + f(a.ev) + } + al, ok := c.approver.(Alarmer) + if !ok { + continue + } + ctx, cancel := c.callContext(c.stopCtx, endTimeout) + err := al.Alarm(ctx, a.al) + cancel() + if err != nil { + c.logger.LogAttrs(ctx, slog.LevelWarn, "approval: cannot send the missed-window alarm of a standing grant", + slog.String("grant_id", a.al.GrantID), slog.Any("error", err)) + } + } +} diff --git a/internal/approval/missed_internal_test.go b/internal/approval/missed_internal_test.go new file mode 100644 index 0000000..b829ceb --- /dev/null +++ b/internal/approval/missed_internal_test.go @@ -0,0 +1,27 @@ +package approval + +import ( + "testing" + "time" +) + +// TestEachWindowEndSkippedHour: on 2026-03-29 the clocks in Berlin go from +// 02:00 to 03:00. A window in 02:00-03:00 never opens, but its end is +// found once, so the missed-window alarm comes (DESIGN.md section 8). +func TestEachWindowEndSkippedHour(t *testing.T) { + berlin := mustZone(t, "Europe/Berlin") + ws := []window{{days: anyDay, from: hm(2, 0), to: hm(3, 0)}} + from := time.Date(2026, 3, 28, 12, 0, 0, 0, berlin) + to := time.Date(2026, 3, 30, 12, 0, 0, 0, berlin) + var n int + eachWindowEnd(ws, from, to, func(k windowKey, end time.Time) { n++ }) + if n != 2 { // 03-29 and 03-30 + t.Errorf("%d window ends, want 2", n) + } + n = 0 + eachWindowEnd(ws, to, from, func(windowKey, time.Time) { n++ }) // time goes back: nothing + eachWindowEnd(ws, from, from, func(windowKey, time.Time) { n++ }) + if n != 0 { + t.Errorf("%d window ends for an empty span", n) + } +} diff --git a/internal/approval/missed_test.go b/internal/approval/missed_test.go new file mode 100644 index 0000000..d276662 --- /dev/null +++ b/internal/approval/missed_test.go @@ -0,0 +1,188 @@ +package approval_test + +// Tests of the missed-window alarm of standing grants (DESIGN.md section +// 8). The fake clock starts at noon on 2026-10-14; the window is 03:00-05:00 +// each day, and the grant lasts 30 days. + +import ( + "context" + "errors" + "sync" + "testing" + "testing/synctest" + "time" + + "github.com/code6x/nodkey/internal/approval" + "github.com/code6x/nodkey/internal/audit" + "github.com/code6x/nodkey/internal/grantkey" +) + +// missedEnv starts a core with one standing grant and collects the entries +// of the alarms. +func missedEnv(t *testing.T) (*env, grantkey.Token, func() []audit.GrantAlarm) { + t.Helper() + e := standEnv(t) + var mu sync.Mutex + var got []audit.GrantAlarm + e.core.OnGrantAlarm(func(ev audit.GrantAlarm) { + mu.Lock() + defer mu.Unlock() + got = append(got, ev) + }) + tok := newToken(t) + e.grant(standReq("laptop", tok, "github-token"), 1) + return e, tok, func() []audit.GrantAlarm { + mu.Lock() + defer mu.Unlock() + return append([]audit.GrantAlarm(nil), got...) + } +} + +func (e *env) pass(d time.Duration) { + e.t.Helper() + e.clock.Advance(d) + synctest.Wait() +} + +// useGrant uses the grant with tok and returns the next token. +func (e *env) useGrant(tok grantkey.Token) grantkey.Token { + e.t.Helper() + h := grantkey.Hash(tok) + res := e.core.UseStanding(approval.StandingRequest{Client: "laptop", Secrets: []string{"github-token"}, Token: &h}) + checkAllowed(e.t, res) + next, ok := res.NewToken() + if !ok { + e.t.Fatal("no new token") + } + if err := res.SendNotice(context.Background()); err != nil { + e.t.Fatal(err) + } + return next +} + +func TestMissedWindowAlarm(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + e, _, entries := missedEnv(t) + defer e.stop() + // The window of the day before the grant started, and the time up + // to 03:00, give nothing. + e.pass(14*time.Hour + 59*time.Minute) + if n := len(e.fake.Alarms()); n != 0 { + t.Fatalf("%d alarms before the first window ended", n) + } + e.pass(2*time.Hour + time.Minute) // 05:00 on 2026-10-15 passed with no use + al := e.fake.Alarms() + if len(al) != 1 { + t.Fatalf("%d alarms, want 1", len(al)) + } + a := al[0] + if a.Cause != audit.AlarmMissedWindow || a.Ended || a.Client != "laptop" || a.GrantID == "" || + len(a.Secrets) != 1 || a.Secrets[0] != "github-token" { + t.Errorf("alarm %+v", a) + } + ev := entries() + if len(ev) != 1 || ev[0].Cause != audit.AlarmMissedWindow || ev[0].GrantID != a.GrantID || ev[0].Client != "laptop" { + t.Errorf("entries %+v", ev) + } + if h, _, _, _ := e.core.StandingGrants("laptop"); len(h) != 1 { + t.Error("the grant must stay after the alarm") + } + // At most one for the window, however often the core looks again. + e.pass(time.Hour) + e.pass(10 * time.Hour) + if n := len(e.fake.Alarms()); n != 1 { + t.Errorf("%d alarms for one window", n) + } + // The next window gives its own. + e.pass(24 * time.Hour) + if n := len(e.fake.Alarms()); n != 2 { + t.Errorf("%d alarms after two windows, want 2", n) + } + }) +} + +func TestNoMissedAlarmAfterAUsedWindow(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + e, tok, entries := missedEnv(t) + defer e.stop() + e.pass(15*time.Hour + 30*time.Minute) // 03:30 on 2026-10-15 + tok = e.useGrant(tok) + e.pass(2 * time.Hour) // 05:30: the window ended with a use + if n := len(e.fake.Alarms()); n != 0 { + t.Errorf("%d alarms after a used window", n) + } + // The next day is not used, and does alarm: the use of one day + // does not cover the next. + e.pass(24 * time.Hour) + if n := len(e.fake.Alarms()); n != 1 { + t.Errorf("%d alarms for the unused next window, want 1", n) + } + _ = tok + if n := len(entries()); n != 1 { + t.Errorf("%d entries", n) + } + }) +} + +func TestMissedAlarmGivenBackUseStillMissed(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + e, tok, _ := missedEnv(t) + defer e.stop() + e.pass(15*time.Hour + 30*time.Minute) + h := grantkey.Hash(tok) + res := e.core.UseStanding(approval.StandingRequest{Client: "laptop", Secrets: []string{"github-token"}, Token: &h}) + checkAllowed(t, res) + res.Undo() // the use did not happen + e.pass(2 * time.Hour) + if n := len(e.fake.Alarms()); n != 1 { + t.Errorf("%d alarms, want 1: an undone use is no use", n) + } + }) +} + +func TestMissedAlarmSendFails(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + e, _, entries := missedEnv(t) + defer e.stop() + e.fake.FailAlarm(errors.New("telegram is down")) + e.pass(17*time.Hour + time.Minute) + if n := len(e.fake.Alarms()); n != 0 { + t.Errorf("%d alarms sent although the send fails", n) + } + if n := len(entries()); n != 1 { + t.Errorf("%d entries, want 1: the audit entry is written first", n) + } + if h, _, _, _ := e.core.StandingGrants("laptop"); len(h) != 1 { + t.Error("the grant must stay after a failed send") + } + // No second try for that window; the next window works. + e.fake.FailAlarm(nil) + e.pass(time.Hour) + if n := len(e.fake.Alarms()); n != 0 { + t.Errorf("%d alarms: the window must not be tried again", n) + } + e.pass(24 * time.Hour) + if n := len(e.fake.Alarms()); n != 1 { + t.Errorf("%d alarms for the next window, want 1", n) + } + }) +} + +func TestNoMissedAlarmAfterTheGrantEnds(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + e, _, _ := missedEnv(t) + defer e.stop() + for range 31 { + e.pass(24 * time.Hour) + } + // The windows up to the end date alarm; none after it. + n := len(e.fake.Alarms()) + if n < 29 || n > 30 { + t.Errorf("%d alarms over 30 days", n) + } + e.pass(5 * 24 * time.Hour) + if m := len(e.fake.Alarms()); m != n { + t.Errorf("%d alarms after the end, was %d", m, n) + } + }) +} diff --git a/internal/approval/window_test.go b/internal/approval/window_test.go index 055f886..455eb44 100644 --- a/internal/approval/window_test.go +++ b/internal/approval/window_test.go @@ -291,10 +291,10 @@ func TestNextEndWakesAtTheEndOfTheWindow(t *testing.T) { if !ok || !next.Equal(now.Add(time.Hour)) { t.Errorf("nextEnd = %v, %v; want the window end %v", next, ok, now.Add(time.Hour)) } - clock.now = time.Date(2026, 11, 2, 6, 0, 0, 0, berlin) // outside: the end date + clock.now = time.Date(2026, 11, 2, 6, 0, 0, 0, berlin) // outside: the end of the next window next, ok = c.nextEnd() - if !ok || !next.Equal(end) { - t.Errorf("nextEnd outside = %v, %v; want %v", next, ok, end) + if want := time.Date(2026, 11, 3, 5, 0, 0, 0, berlin); !ok || !next.Equal(want) { + t.Errorf("nextEnd outside = %v, %v; want %v", next, ok, want) } c.grants = c.grants[:1] if _, ok := c.nextEnd(); ok { diff --git a/internal/broker/broker.go b/internal/broker/broker.go index 8dca644..04c7cd9 100644 --- a/internal/broker/broker.go +++ b/internal/broker/broker.go @@ -405,6 +405,11 @@ func (s *Service) Run(ctx context.Context, core *approval.Core, alert AlertFunc, } dropped.note(ev) }) + core.OnGrantAlarm(func(ev audit.GrantAlarm) { + if _, err := s.append(ev); err != nil { + s.log.Warn("broker: cannot write the grant_alarm entry of a missed window") + } + }) err := core.Run(ctx) s.coreStopped.Store(true) // first: deny every request from now on // The grants are dropped by now. Their messages are updated, and one diff --git a/internal/broker/standinguse_test.go b/internal/broker/standinguse_test.go index c86eab4..f809173 100644 --- a/internal/broker/standinguse_test.go +++ b/internal/broker/standinguse_test.go @@ -192,11 +192,12 @@ func TestStandingOutsideWindow(t *testing.T) { res, _ := e.useToken("laptop", tok0) mustStatus(t, res, http.StatusForbidden) } + // The window of 11:00-13:00 ended with no use: that is the first alarm. al := e.alarmEntries() - if len(al) != 1 || al[0].Cause != audit.AlarmOutsideWindow || al[0].GrantID != id { + if len(al) != 2 || al[0].Cause != audit.AlarmMissedWindow || al[1].Cause != audit.AlarmOutsideWindow || al[1].GrantID != id { t.Fatalf("alarms: %+v", al) } - if as := e.approver.Alarms(); len(as) != 1 || as[0].Ended { + if as := e.approver.Alarms(); len(as) != 2 || as[0].Ended || as[1].Ended { t.Errorf("the alarms: %+v", as) } e.clock.Advance(20 * time.Hour) // 11:00 the next day diff --git a/internal/telegram/alarm_test.go b/internal/telegram/alarm_test.go index 5e6c214..392d207 100644 --- a/internal/telegram/alarm_test.go +++ b/internal/telegram/alarm_test.go @@ -51,13 +51,13 @@ func alarmOf(cause audit.AlarmCause) approval.Alarm { // counts toward the notices per minute. func TestAlarm(t *testing.T) { e := newEnv(t) - for _, cause := range []audit.AlarmCause{audit.AlarmOldToken, audit.AlarmOverUses, audit.AlarmOutsideWindow, audit.AlarmUnknownToken} { + for _, cause := range []audit.AlarmCause{audit.AlarmOldToken, audit.AlarmOverUses, audit.AlarmOutsideWindow, audit.AlarmUnknownToken, audit.AlarmMissedWindow} { if err := e.a.Alarm(context.Background(), alarmOf(cause)); err != nil { t.Fatalf("%s: %v", cause, err) } } calls := e.f.callsOf("sendMessage") - if len(calls) != 4 { + if len(calls) != 5 { t.Fatalf("%d messages", len(calls)) } for _, c := range calls { @@ -70,7 +70,7 @@ func TestAlarm(t *testing.T) { t.Errorf("texts: %q, %q", calls[0].str("text"), calls[3].str("text")) } // The same budget as Notify: 20 in all. - for i := 4; i < NoticesPerMinute; i++ { + for i := 5; i < NoticesPerMinute; i++ { if err := e.a.Alarm(context.Background(), alarmOf(audit.AlarmOldToken)); err != nil { t.Fatal(err) } diff --git a/internal/telegram/text.go b/internal/telegram/text.go index 3e5cb0a..8869378 100644 --- a/internal/telegram/text.go +++ b/internal/telegram/text.go @@ -680,6 +680,8 @@ func alarmText(a approval.Alarm) (*text, error) { what = "a valid token was used outside the windows. The request was denied; the grant stays." case audit.AlarmUnknownToken: what = "a token that no grant of this machine has. The request was denied." + case audit.AlarmMissedWindow: + what = "a window ended and the grant was not used in it. The grant stays." default: return nil, bad("the cause") } From 082d79f5d79c160ff8b8c59ae35fbe181ec98ece Mon Sep 17 00:00:00 2001 From: Geno Roupsky <185580+groupsky@users.noreply.github.com> Date: Thu, 29 Oct 2026 12:40:49 +0200 Subject: [PATCH 04/16] feat: /grants lists the standing grants (#110) Co-Authored-By: Claude Opus 5.5 --- DESIGN.md | 4 + internal/approval/approval.go | 3 + internal/approval/grants.go | 5 ++ internal/approval/list.go | 63 ++++++++++++++ internal/approval/list_test.go | 41 +++++++++ internal/telegram/api.go | 12 ++- internal/telegram/grants.go | 129 ++++++++++++++++++++++++++++ internal/telegram/grants_test.go | 143 +++++++++++++++++++++++++++++++ internal/telegram/poll.go | 5 +- internal/telegram/poll_test.go | 2 +- internal/telegram/telegram.go | 4 + 11 files changed, 406 insertions(+), 5 deletions(-) create mode 100644 internal/approval/list.go create mode 100644 internal/approval/list_test.go create mode 100644 internal/telegram/grants.go create mode 100644 internal/telegram/grants_test.go diff --git a/DESIGN.md b/DESIGN.md index 1442206..0923fc4 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -1574,6 +1574,10 @@ grant ends with cause `renewed`. Once a week (`summary`, broker local time) a message lists each standing grant: the client, the secrets, the windows, the last use and the end date. The bot command `/grants` from the approver shows the same list. The list never shows a token or a hash. +For `/grants` the bot also asks Telegram for `message` updates. Only the +exact text `/grants` from the approver in the private chat with the approver +gets an answer, at most 3 in a minute. A message from anyone or anywhere +else is ignored without an answer. **Missed window.** When a window of a live grant ends with no use in it, the broker sends the alarm `missed_window`. The grant stays. A window that diff --git a/internal/approval/approval.go b/internal/approval/approval.go index 9e819f8..102cdee 100644 --- a/internal/approval/approval.go +++ b/internal/approval/approval.go @@ -184,6 +184,9 @@ func (c *Core) Run(ctx context.Context) error { if !c.start() { return errors.New("approval: Run can run only once") } + if gl, ok := c.approver.(GrantLister); ok { + gl.SetGrantSource(c.ListStanding) + } watched := make(chan struct{}) go func() { defer close(watched) diff --git a/internal/approval/grants.go b/internal/approval/grants.go index 85f1dd9..2cdba8a 100644 --- a/internal/approval/grants.go +++ b/internal/approval/grants.go @@ -86,6 +86,9 @@ type grant struct { used map[windowKey]bool // marked is the opening that the last use marked in used. marked windowKey + // lastUse is the wall time of the last use of a standing grant, and + // prevUse the one before it (giveBackUse restores it). Zero: never. + lastUse, prevUse time.Time } // grantKind is the kind of a grant (DESIGN.md section 8, "The grant core"). @@ -178,6 +181,7 @@ func (g *grant) tryUse(now moment) useResult { return useOverUses } g.winUsed++ + g.prevUse, g.lastUse = g.lastUse, now.wall if g.used == nil { g.used = make(map[windowKey]bool) } @@ -190,6 +194,7 @@ func (g *grant) tryUse(now moment) useResult { func (g *grant) giveBackUse() { if g.standing() { g.winUsed = max(g.winUsed-1, 0) + g.lastUse = g.prevUse if g.winUsed == 0 { delete(g.used, g.marked) } diff --git a/internal/approval/list.go b/internal/approval/list.go new file mode 100644 index 0000000..7af20e6 --- /dev/null +++ b/internal/approval/list.go @@ -0,0 +1,63 @@ +package approval + +import ( + "cmp" + "context" + "slices" + "strings" + "time" +) + +// GrantInfo is one live standing grant, as the list of DESIGN.md section 8 +// shows it. It holds no token and no hash. +type GrantInfo struct { + Client string + Secrets []string + // Windows are the windows as text, such as "daily 03:00-05:00". + Windows []string + // LastUse is the wall time of the last use, or the zero time for a + // grant that was never used. + LastUse time.Time + // End is the wall time at which the grant ends. + End time.Time +} + +// ListStanding returns the live standing grants, ordered by client and +// then by end. A grant that has not started yet is live. +func (c *Core) ListStanding() []GrantInfo { + c.mu.Lock() + defer c.mu.Unlock() + now := c.now() + var out []GrantInfo + for _, g := range c.grants { + if !g.standing() || !now.before(g.end) { + continue + } + gi := GrantInfo{Client: g.client, LastUse: g.lastUse, End: g.end.wall} + for _, r := range g.refs { + gi.Secrets = append(gi.Secrets, r.name) + } + for _, w := range g.windows { + gi.Windows = append(gi.Windows, windowText(w)) + } + out = append(out, gi) + } + slices.SortFunc(out, func(a, b GrantInfo) int { + return cmp.Or(strings.Compare(a.Client, b.Client), a.End.Compare(b.End), + strings.Compare(strings.Join(a.Secrets, ","), strings.Join(b.Secrets, ","))) + }) + return out +} + +// GrantLister is an Approver that can answer the bot command /grants. The +// core gives it the function that lists the standing grants before it +// calls Run. +type GrantLister interface { + SetGrantSource(f func() []GrantInfo) +} + +// Summarizer is an Approver that can send the weekly summary of the +// standing grants (DESIGN.md section 8). +type Summarizer interface { + Summary(ctx context.Context, grants []GrantInfo) error +} diff --git a/internal/approval/list_test.go b/internal/approval/list_test.go new file mode 100644 index 0000000..afb0944 --- /dev/null +++ b/internal/approval/list_test.go @@ -0,0 +1,41 @@ +package approval_test + +import ( + "testing" + "testing/synctest" + "time" +) + +// TestListStanding: a live standing grant is listed with its windows, +// "never" before a use, the time of the use after it, and the end. A grant +// that ended is not listed. +func TestListStanding(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + e := standEnv(t) + defer e.stop() + if l := e.core.ListStanding(); len(l) != 0 { + t.Fatalf("%d grants before any was made", len(l)) + } + tok := newToken(t) + e.grant(standReq("laptop", tok, "github-token", "npm-token"), 1) + l := e.core.ListStanding() + if len(l) != 1 { + t.Fatalf("%d grants, want 1", len(l)) + } + g := l[0] + if g.Client != "laptop" || len(g.Secrets) != 2 || len(g.Windows) != 1 || g.Windows[0] != "daily 03:00-05:00" || + !g.LastUse.IsZero() || !g.End.Equal(noon.Add(30*24*time.Hour)) { + t.Errorf("the entry is %+v", g) + } + e.pass(15*time.Hour + time.Minute) // 03:01 on 2026-10-15 + e.useGrant(tok) + l = e.core.ListStanding() + if len(l) != 1 || !l[0].LastUse.Equal(noon.Add(15*time.Hour+time.Minute)) { + t.Errorf("after a use: %+v", l) + } + e.pass(31 * 24 * time.Hour) + if l := e.core.ListStanding(); len(l) != 0 { + t.Errorf("an ended grant is listed: %+v", l) + } + }) +} diff --git a/internal/telegram/api.go b/internal/telegram/api.go index 2bd8137..983ac62 100644 --- a/internal/telegram/api.go +++ b/internal/telegram/api.go @@ -496,11 +496,14 @@ func emptyKeyboard() inlineKeyboard { // The parts of updates that this package reads. Other fields are ignored. type ( - // update is one update from getUpdates. Only button presses - // (callback_query) are asked for. + // update is one update from getUpdates. Button presses + // (callback_query) and messages (for /grants) are asked for. update struct { UpdateID int64 `json:"update_id"` CallbackQuery *callbackQuery `json:"callback_query"` + // Message is a message to the bot. Only the command /grants is + // read from it. + Message *message `json:"message"` // malformed: the update does not have the expected form. Only its // ID is kept. malformed bool @@ -525,6 +528,9 @@ type ( message struct { MessageID int64 `json:"message_id"` Chat chat `json:"chat"` + // From and Text are read only for the command /grants. + From user `json:"from"` + Text string `json:"text"` // EditDate is set when the message was edited. The broker edits a // message only after its request ended. EditDate int64 `json:"edit_date"` @@ -548,7 +554,7 @@ func (c *client) getUpdates(ctx context.Context, offset int64) ([]update, *apiEr Offset: offset, Limit: pollLimit, Timeout: int(pollTimeout / time.Second), - AllowedUpdates: []string{"callback_query"}, + AllowedUpdates: []string{"callback_query", "message"}, } var raws []json.RawMessage if e := c.callOnce(ctx, "getUpdates", params, &raws, c.pollTimeout); e != nil { diff --git a/internal/telegram/grants.go b/internal/telegram/grants.go new file mode 100644 index 0000000..98ed3f2 --- /dev/null +++ b/internal/telegram/grants.go @@ -0,0 +1,129 @@ +package telegram + +import ( + "context" + "errors" + "log/slog" + "strconv" + "strings" + + "github.com/code6x/nodkey/internal/approval" +) + +// GrantsPerMinute is the most answers to /grants that the approver sends +// in one minute. More commands in that minute get no answer. +const GrantsPerMinute = 3 + +// dateLayout is how the list shows the end date and the last use. +const dateLayout = "2006-01-02 15:04" + +// SetGrantSource sets the function that lists the standing grants, for +// /grants and the weekly summary. The core calls it before Run. +func (a *Approver) SetGrantSource(f func() []approval.GrantInfo) { + a.mu.Lock() + defer a.mu.Unlock() + a.source = f +} + +// isGrantsCommand reports whether s is the command /grants, also as +// /grants@botname, with nothing after it. +func isGrantsCommand(s string) bool { + s = strings.TrimSpace(s) + return s == "/grants" || strings.HasPrefix(s, "/grants@") && !strings.ContainsAny(s, " \t\n") +} + +// command handles a message to the bot. Only /grants from the approver in +// the private chat with the approver gets an answer; every other message +// is ignored, and so is a command over the limit. Nothing is said to +// others. +func (a *Approver) command(ctx context.Context, m *message) { + if m.From.ID != a.userID || m.Chat.Type != "private" || m.Chat.ID != a.userID || + !isGrantsCommand(m.Text) { + return + } + a.mu.Lock() + src := a.source + a.mu.Unlock() + if src == nil || !a.take(&a.listed, GrantsPerMinute) { + return + } + t, err := grantsText(src(), false) + if err != nil { + a.log.LogAttrs(ctx, slog.LevelWarn, "telegram: cannot show the standing grants", slog.Any("error", err)) + return + } + tctx, cancel := context.WithTimeout(ctx, tapTimeout) + defer cancel() + err = a.api.call(tctx, "sendMessage", sendMessageParams{ + ChatID: a.userID, + Text: t.html.String(), + ParseMode: "HTML", + LinkPreviewOptions: noPreview, + }, nil) + if err != nil { + a.log.LogAttrs(ctx, slog.LevelWarn, "telegram: cannot send the standing grants", slog.Any("error", err)) + } +} + +// Summary sends the weekly summary of the standing grants to the notice +// chat. A week has one, so it does not count toward the notices per +// minute. +func (a *Approver) Summary(ctx context.Context, grants []approval.GrantInfo) error { + if err := ctx.Err(); err != nil { + return err + } + t, err := grantsText(grants, true) + if err != nil { + return err + } + return a.api.call(ctx, "sendMessage", sendMessageParams{ + ChatID: a.noticeChat, + Text: t.html.String(), + ParseMode: "HTML", + LinkPreviewOptions: noPreview, + DisableNotification: true, + }, nil) +} + +// grantsText returns the list of the standing grants: the client, the +// secrets, the windows, the last use and the end date of each. It has no +// token and no hash. weekly adds the name of the weekly summary to the +// title. A list that is too long is cut, with the count of the grants +// left out. +func grantsText(list []approval.GrantInfo, weekly bool) (*text, error) { + title := "Standing grants" + if weekly { + title = "Weekly summary ยท standing grants" + } + t := &text{} + if len(list) == 0 { + t.write("๐Ÿ“‹ " + title + "\nNo standing grants.") + return t, nil + } + for _, g := range list { + if !namePattern.MatchString(g.Client) || !allNames(g.Secrets) || len(g.Secrets) == 0 || + g.End.IsZero() { + return nil, errors.New("telegram: a standing grant in the list is not valid") + } + } + t.write("๐Ÿ“‹ " + title + " ยท " + strconv.Itoa(len(list))) + for i, g := range list { + var e text + e.write("\n\nโœ… machine: " + g.Client) + e.write("\nsecrets: " + strings.Join(g.Secrets, ", ")) + e.write("\nwindows: " + strings.Join(g.Windows, "; ")) + last := "never" + if !g.LastUse.IsZero() { + last = g.LastUse.Format(dateLayout) + } + e.write("\nlast use: " + last) + e.write("\nends: " + g.End.Format(dateLayout)) + if t.units()+e.units() > maxAlertUnits { + t.write("\n\nโ€ฆ and " + strconv.Itoa(len(list)-i) + " more") + break + } + t.html.WriteString(e.html.String()) + t.plain.WriteString(e.plain.String()) + } + return t, nil +} diff --git a/internal/telegram/grants_test.go b/internal/telegram/grants_test.go new file mode 100644 index 0000000..659b855 --- /dev/null +++ b/internal/telegram/grants_test.go @@ -0,0 +1,143 @@ +package telegram + +import ( + "context" + "strings" + "testing" + "time" + + "github.com/code6x/nodkey/internal/approval" +) + +func testGrants() []approval.GrantInfo { + return []approval.GrantInfo{ + {Client: "laptop", Secrets: []string{"backup-token", "db-pass"}, Windows: []string{"daily 03:00-05:00"}, + End: time.Date(2026, 12, 1, 3, 0, 0, 0, time.UTC)}, + {Client: "web-1", Secrets: []string{"deploy-key"}, Windows: []string{"mon,thu 01:00-02:00"}, + LastUse: time.Date(2026, 11, 2, 1, 5, 0, 0, time.UTC), End: time.Date(2027, 1, 5, 1, 0, 0, 0, time.UTC)}, + } +} + +// TestGrantsText: the list shows client, secrets, windows, last use (or +// never) and end; an empty list has its own text; a bad name is refused. +func TestGrantsText(t *testing.T) { + tx, err := grantsText(testGrants(), false) + if err != nil { + t.Fatal(err) + } + got := tx.plain.String() + for _, want := range []string{"machine: laptop", "backup-token, db-pass", "daily 03:00-05:00", + "last use: never", "ends: 2026-12-01 03:00", "last use: 2026-11-02 01:05", "mon,thu 01:00-02:00"} { + if !strings.Contains(got, want) { + t.Errorf("missing %q in %q", want, got) + } + } + w, _ := grantsText(testGrants(), true) + if !strings.Contains(w.plain.String(), "Weekly summary") { + t.Error("the weekly text has no title") + } + e, err := grantsText(nil, true) + if err != nil || !strings.Contains(e.plain.String(), "No standing grants.") { + t.Errorf("empty list: %v %q", err, e.plain.String()) + } + bad := testGrants() + bad[0].Client = "x" + if _, err := grantsText(bad, false); err == nil { + t.Error("a bad client name was accepted") + } + bad = testGrants() + bad[0].Secrets = nil + if _, err := grantsText(bad, false); err == nil { + t.Error("a grant without secrets was accepted") + } +} + +// TestGrantsTextCut: a long list is cut with a count. +func TestGrantsTextCut(t *testing.T) { + var l []approval.GrantInfo + for range 100 { + l = append(l, testGrants()[0]) + } + tx, err := grantsText(l, false) + if err != nil { + t.Fatal(err) + } + if tx.units() > maxAlertUnits+50 || !strings.Contains(tx.plain.String(), "more") { + t.Errorf("not cut: %d units", tx.units()) + } +} + +func (f *fakeTelegram) message(from, chatID int64, chatType, txt string) { + f.mu.Lock() + defer f.mu.Unlock() + f.addUpdateLocked(map[string]any{"message": map[string]any{ + "message_id": 5, "text": txt, + "from": map[string]any{"id": from}, + "chat": map[string]any{"id": chatID, "type": chatType}, + }}) +} + +// TestGrantsCommand: /grants from the approver in the private chat gets the +// list, and stays private. Strangers, other chats, other text and no source +// get no answer, and the command is limited per minute. +func TestGrantsCommand(t *testing.T) { + e := newEnv(t) + core := &stubCore{} + // No source yet: no answer. + e.a.SetGrantSource(nil) + stop := e.run(t, core.answer) + e.f.message(approverID, approverID, "private", "/grants") + e.f.tap(goodTap(1, "x")) + e.f.waitFor("handled", func() bool { return len(e.f.callsOfLocked("answerCallbackQuery")) >= 1 }) + e.a.SetGrantSource(testGrants) + + e.f.message(approverID+1, approverID+1, "private", "/grants") // a stranger + e.f.message(approverID+1, approverID, "private", "/grants") // a stranger in the chat + e.f.message(approverID, noticeChatID, "group", "/grants") // another chat + e.f.message(approverID, approverID, "private", "/grants now") // other text + e.f.message(approverID, approverID, "private", "grants") + e.f.message(approverID, approverID, "private", "/grants@somebot") + e.f.waitCalls("sendMessage", 1) + // A sentinel: updates go in order, so all before it were handled. + e.f.tap(goodTap(1, "y")) + e.f.waitFor("sentinel", func() bool { return len(e.f.callsOfLocked("answerCallbackQuery")) >= 2 }) + calls := e.f.callsOf("sendMessage") + if len(calls) != 1 { + t.Fatalf("%d answers, want 1", len(calls)) + } + if calls[0].num("chat_id") != approverID || !strings.Contains(calls[0].str("text"), "laptop") { + t.Errorf("the answer: %s", calls[0].Raw) + } + for _, bad := range []string{"deadbeef", "hash"} { + if strings.Contains(calls[0].str("text"), bad) { + t.Error("the answer has " + bad) + } + } + + // The limit: more commands than GrantsPerMinute in a minute. + for range 5 { + e.f.message(approverID, approverID, "private", "/grants") + } + e.f.tap(goodTap(1, "z")) + e.f.waitFor("sentinel 2", func() bool { return len(e.f.callsOfLocked("answerCallbackQuery")) >= 3 }) + if n := len(e.f.callsOf("sendMessage")); n != GrantsPerMinute { + t.Errorf("%d answers, want %d", n, GrantsPerMinute) + } + _ = stop() +} + +// TestSummary: the summary goes to the notice chat; an empty list says so. +func TestSummary(t *testing.T) { + e := newEnv(t) + if err := e.a.Summary(context.Background(), testGrants()); err != nil { + t.Fatal(err) + } + if err := e.a.Summary(context.Background(), nil); err != nil { + t.Fatal(err) + } + calls := e.f.callsOf("sendMessage") + if len(calls) != 2 || calls[0].num("chat_id") != noticeChatID || + !strings.Contains(calls[1].str("text"), "No standing grants.") { + t.Errorf("calls: %d", len(calls)) + } +} diff --git a/internal/telegram/poll.go b/internal/telegram/poll.go index 2fbdfcb..d438ffc 100644 --- a/internal/telegram/poll.go +++ b/internal/telegram/poll.go @@ -180,7 +180,10 @@ func (a *Approver) handle(ctx context.Context, u update, answer func(approval.An } q := u.CallbackQuery if q == nil { - return // not a button press: allowed_updates asks for no other kind + if u.Message != nil { + a.command(ctx, u.Message) + } + return } tctx, cancel := context.WithTimeout(ctx, tapTimeout) defer cancel() diff --git a/internal/telegram/poll_test.go b/internal/telegram/poll_test.go index de588e8..99e040e 100644 --- a/internal/telegram/poll_test.go +++ b/internal/telegram/poll_test.go @@ -273,7 +273,7 @@ func TestPolling(t *testing.T) { for i, want := range []int64{0, 7, 8} { c := calls[i] if c.num("offset") != want || c.num("timeout") != 50 || c.num("limit") != pollLimit || - c.field("allowed_updates") != `["callback_query"]` { + c.field("allowed_updates") != `["callback_query","message"]` { t.Errorf("getUpdates %d: %s, want offset %d", i, c.Raw, want) } } diff --git a/internal/telegram/telegram.go b/internal/telegram/telegram.go index fa44e16..4fc8311 100644 --- a/internal/telegram/telegram.go +++ b/internal/telegram/telegram.go @@ -161,6 +161,10 @@ type Approver struct { // notices holds when the newest notices of Notify were sent, and // informs those of Inform. notices, informs []time.Time + // listed holds when the newest answers to /grants were sent, and + // source lists the standing grants (SetGrantSource). + listed []time.Time + source func() []approval.GrantInfo // reports counts the reports of refused taps of other users in the // current minute, and ownReports those of the approver's own taps. // Each is a budget of its own, so the taps of others cannot crowd out From 534cf9ac73f330ba979cfb4b1aded30b9309469f Mon Sep 17 00:00:00 2001 From: Geno Roupsky <185580+groupsky@users.noreply.github.com> Date: Thu, 29 Oct 2026 12:42:35 +0200 Subject: [PATCH 05/16] feat: the weekly summary of standing grants (#110) Co-Authored-By: Claude Opus 5.5 --- DESIGN.md | 1 + docs/setup.md | 3 + internal/approval/approval.go | 6 ++ internal/approval/approvaltest/approver.go | 20 +++++++ internal/approval/summary.go | 61 ++++++++++++++++++++ internal/approval/summary_ext_test.go | 65 ++++++++++++++++++++++ internal/approval/summary_test.go | 38 +++++++++++++ 7 files changed, 194 insertions(+) create mode 100644 internal/approval/summary.go create mode 100644 internal/approval/summary_ext_test.go create mode 100644 internal/approval/summary_test.go diff --git a/DESIGN.md b/DESIGN.md index 0923fc4..1921d51 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -1574,6 +1574,7 @@ grant ends with cause `renewed`. Once a week (`summary`, broker local time) a message lists each standing grant: the client, the secrets, the windows, the last use and the end date. The bot command `/grants` from the approver shows the same list. The list never shows a token or a hash. +With no standing grant, the weekly message says so, and `/grants` does too. For `/grants` the bot also asks Telegram for `message` updates. Only the exact text `/grants` from the approver in the private chat with the approver gets an answer, at most 3 in a minute. A message from anyone or anywhere diff --git a/docs/setup.md b/docs/setup.md index adfa1da..2cb05ae 100644 --- a/docs/setup.md +++ b/docs/setup.md @@ -779,6 +779,9 @@ max_open = "8h" # the longest time from start to end max_uses = 10 # the most uses of one grant max_grants = 5 # the most per client: approved, not ended, plus waiting +[standing] # optional; the limits of standing grants (DESIGN.md section 8) +summary = "mon 09:00" # the weekly list of standing grants, in broker local time + [client.laptop] pin = "sha256:Yf3c...base64..." # printed by `nodkey keygen` diff --git a/internal/approval/approval.go b/internal/approval/approval.go index 102cdee..4c3b7a1 100644 --- a/internal/approval/approval.go +++ b/internal/approval/approval.go @@ -192,9 +192,15 @@ func (c *Core) Run(ctx context.Context) error { defer close(watched) c.watchGrants(ctx) }() + summed := make(chan struct{}) + go func() { + defer close(summed) + c.watchSummary(ctx) + }() defer func() { c.stopAll() <-watched + <-summed c.dropScheduled() }() if err := c.approver.Run(ctx, c.Answer); err != nil { diff --git a/internal/approval/approvaltest/approver.go b/internal/approval/approvaltest/approver.go index 9150539..e16ee15 100644 --- a/internal/approval/approvaltest/approver.go +++ b/internal/approval/approvaltest/approver.go @@ -50,6 +50,7 @@ type Approver struct { ended []Ended notices []approval.Prompt alarms []approval.Alarm + summaries [][]approval.GrantInfo alarmErr error askErr error notifyErr error @@ -369,3 +370,22 @@ func Text(p approval.Prompt) string { b.WriteString(strings.Join(buttons, " ")) return b.String() } + +// Summary keeps the list of a weekly summary. +func (a *Approver) Summary(ctx context.Context, grants []approval.GrantInfo) error { + if err := ctx.Err(); err != nil { + return err + } + a.mu.Lock() + defer a.mu.Unlock() + a.summaries = append(a.summaries, append([]approval.GrantInfo(nil), grants...)) + a.changedLocked() + return nil +} + +// Summaries returns the lists of the weekly summaries, oldest first. +func (a *Approver) Summaries() [][]approval.GrantInfo { + a.mu.Lock() + defer a.mu.Unlock() + return append([][]approval.GrantInfo(nil), a.summaries...) +} diff --git a/internal/approval/summary.go b/internal/approval/summary.go new file mode 100644 index 0000000..3776c98 --- /dev/null +++ b/internal/approval/summary.go @@ -0,0 +1,61 @@ +package approval + +import ( + "context" + "log/slog" + "time" +) + +// summaryTimeout bounds the send of the weekly summary. +const summaryTimeout = 30 * time.Second + +// nextSummary returns the first time after wall that is on weekday day at +// minute of the day, in the location of wall. +func nextSummary(wall time.Time, day time.Weekday, minute int) time.Time { + y, m, d := wall.Date() + for i := 0; i < 9; i++ { // a match comes at least every 7 days + t := time.Date(y, m, d+i, minute/60, minute%60, 0, 0, wall.Location()) + if t.Weekday() == day && t.After(wall) { + return t + } + } + return wall.AddDate(0, 0, 7) // not reached +} + +// watchSummary sends the weekly summary of the standing grants (DESIGN.md +// section 8) once a week, at the time of [standing] summary in broker local +// time, until ctx or the core ends. It does nothing without [standing] or +// with an approver that cannot send it. A send that fails is logged and +// not tried again until next week. +func (c *Core) watchSummary(ctx context.Context) { + s, ok := c.approver.(Summarizer) + if !ok || !c.cfg.Standing.Enabled { + return + } + var sent time.Time // the target of the last summary + for { + now := c.now() + target := nextSummary(now.wall, c.cfg.Standing.SummaryDay, c.cfg.Standing.SummaryMinute) + t := c.clock.TimerAt(now.mono.Add(target.Sub(now.wall))) + select { + case <-t.C(): + case <-ctx.Done(): + case <-c.stopCtx.Done(): + } + t.Stop() + if ctx.Err() != nil || c.stopCtx.Err() != nil { + return + } + if c.now().wall.Before(target) || !target.After(sent) { + continue // the timer was early, or this time had its summary: the wall clock was set back + } + sent = target + cctx, cancel := c.callContext(c.stopCtx, summaryTimeout) + err := s.Summary(cctx, c.ListStanding()) + cancel() + if err != nil { + c.logger.LogAttrs(ctx, slog.LevelWarn, "approval: cannot send the weekly summary of standing grants", + slog.Any("error", err)) + } + } +} diff --git a/internal/approval/summary_ext_test.go b/internal/approval/summary_ext_test.go new file mode 100644 index 0000000..de2bef8 --- /dev/null +++ b/internal/approval/summary_ext_test.go @@ -0,0 +1,65 @@ +package approval_test + +import ( + "testing" + "testing/synctest" + "time" +) + +// TestWeeklySummary: with a fake clock, the summary comes once a week on +// Monday at 09:00 (the default), lists the live grants, and says nothing +// else on the way. An empty list is sent too. +func TestWeeklySummary(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + e := standEnv(t) // starts Wednesday 2026-10-14 12:00 + defer e.stop() + e.pass(4*24*time.Hour + 20*time.Hour + 59*time.Minute) // Monday 08:59 + if n := len(e.fake.Summaries()); n != 0 { + t.Fatalf("%d summaries before the time", n) + } + e.pass(time.Minute) // Monday 2026-10-19 09:00 + if s := e.fake.Summaries(); len(s) != 1 || len(s[0]) != 0 { + t.Fatalf("summaries: %+v", s) + } + e.pass(time.Hour) + if n := len(e.fake.Summaries()); n != 1 { + t.Fatalf("%d summaries one hour later, want 1", n) + } + e.grant(standReq("laptop", newToken(t), "github-token"), 1) + e.pass(7*24*time.Hour - time.Hour) // Monday 2026-10-26 09:00 + s := e.fake.Summaries() + if len(s) != 2 || len(s[1]) != 1 || s[1][0].Client != "laptop" { + t.Fatalf("summaries: %+v", s) + } + }) +} + +// TestNoSummaryWithoutStanding: a config without [standing] sends none. +func TestNoSummaryWithoutStanding(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + e := newEnv(t, nil) + defer e.stop() + e.pass(15 * 24 * time.Hour) + if n := len(e.fake.Summaries()); n != 0 { + t.Errorf("%d summaries without [standing]", n) + } + }) +} + +// TestSummaryNotRepeatedAfterClockSetBack: a wall clock set back past the +// time does not send the same summary again. +func TestSummaryNotRepeatedAfterClockSetBack(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + e := standEnv(t) + defer e.stop() + e.pass(4*24*time.Hour + 21*time.Hour) // Monday 09:00 + if n := len(e.fake.Summaries()); n != 1 { + t.Fatalf("%d summaries, want 1", n) + } + e.clock.JumpWall(-2 * time.Hour) + e.pass(3 * time.Hour) + if n := len(e.fake.Summaries()); n != 1 { + t.Errorf("%d summaries after the clock was set back, want 1", n) + } + }) +} diff --git a/internal/approval/summary_test.go b/internal/approval/summary_test.go new file mode 100644 index 0000000..1e4b436 --- /dev/null +++ b/internal/approval/summary_test.go @@ -0,0 +1,38 @@ +package approval + +import ( + "testing" + "time" +) + +// TestNextSummary: the next time is after wall, on the day, at the minute +// of the day, in the location of wall. +func TestNextSummary(t *testing.T) { + utc := time.UTC + tests := []struct { + name string + wall time.Time + day time.Weekday + min int + want time.Time + }{ + {"same day, later", time.Date(2026, 10, 19, 8, 0, 0, 0, utc), time.Monday, 9 * 60, time.Date(2026, 10, 19, 9, 0, 0, 0, utc)}, + {"at the minute: next week", time.Date(2026, 10, 19, 9, 0, 0, 0, utc), time.Monday, 9 * 60, time.Date(2026, 10, 26, 9, 0, 0, 0, utc)}, + {"same day, later than the time", time.Date(2026, 10, 19, 9, 1, 0, 0, utc), time.Monday, 9 * 60, time.Date(2026, 10, 26, 9, 0, 0, 0, utc)}, + {"midweek", time.Date(2026, 10, 14, 12, 0, 0, 0, utc), time.Sunday, 23*60 + 59, time.Date(2026, 10, 18, 23, 59, 0, 0, utc)}, + {"year end", time.Date(2026, 12, 30, 12, 0, 0, 0, utc), time.Friday, 0, time.Date(2027, 1, 1, 0, 0, 0, 0, utc)}, + } + for _, tt := range tests { + if got := nextSummary(tt.wall, tt.day, tt.min); !got.Equal(tt.want) { + t.Errorf("%s: %v, want %v", tt.name, got, tt.want) + } + } + if loc, err := time.LoadLocation("Europe/Sofia"); err == nil { + // The clocks go back on 2026-10-25: the day is 25 hours long. + wall := time.Date(2026, 10, 24, 12, 0, 0, 0, loc) + got := nextSummary(wall, time.Monday, 9*60) + if want := time.Date(2026, 10, 26, 9, 0, 0, 0, loc); !got.Equal(want) { + t.Errorf("clock change: %v, want %v", got, want) + } + } +} From 68d8c3da27e25beeb934292e51220a191a770f87 Mon Sep 17 00:00:00 2001 From: Geno Roupsky <185580+groupsky@users.noreply.github.com> Date: Thu, 29 Oct 2026 17:09:03 +0200 Subject: [PATCH 06/16] feat: the audit entry of a renewal of a standing grant (#110) Co-Authored-By: Claude Opus 5.5 --- internal/audit/event.go | 15 +++++++++++++++ internal/audit/event_test.go | 8 ++++++++ 2 files changed, 23 insertions(+) diff --git a/internal/audit/event.go b/internal/audit/event.go index a40bff0..018a2da 100644 --- a/internal/audit/event.go +++ b/internal/audit/event.go @@ -55,6 +55,10 @@ const ( // has a decision entry: its result is a grant_ended entry with cause // revoked. AnswerRevoke Answer = "revoke" + // AnswerRenew is the Renew button of the warning before the end of a + // standing grant (DESIGN.md section 8). Its decision entry has the + // Subject of the new grant, and RenewOf names the old one. + AnswerRenew Answer = "renew" ) // Via says what allowed a request without a tap. @@ -221,6 +225,9 @@ type Decision struct { // WindowMinutes is the length of the grant, for AnswerApproveWindow // only. WindowMinutes int `json:"window_minutes,omitempty"` + // RenewOf is the ID of the standing grant that a renewal replaces, for + // AnswerRenew only. + RenewOf string `json:"renew_of,omitempty"` } // GrantUsed records a request that a grant or a "never ask" rule allowed @@ -488,7 +495,15 @@ func (e Decision) validate() error { if e.UserID <= 0 { return errors.New("user_id must be a Telegram user ID") } + if (e.Answer == AnswerRenew) != (e.RenewOf != "") { + return errors.New("renew_of is for a renew answer, and a renew answer needs it") + } switch e.Answer { + case AnswerRenew: + if e.Kind != KindStanding || e.WindowMinutes != 0 || !requestIDPattern.MatchString(e.GrantID) || + !requestIDPattern.MatchString(e.RenewOf) || e.RenewOf == e.GrantID { + return errors.New("a renew answer needs a standing request, its new grant_id and the renew_of grant") + } case AnswerApproveOnce, AnswerDeny: if e.WindowMinutes != 0 { return errors.New("window_minutes is only for approve_window") diff --git a/internal/audit/event_test.go b/internal/audit/event_test.go index 3bf6324..d63c746 100644 --- a/internal/audit/event_test.go +++ b/internal/audit/event_test.go @@ -224,6 +224,14 @@ func TestStandingEvents(t *testing.T) { good("an unknown token alarm", GrantAlarm{Subject: noID, Cause: AlarmUnknownToken}) bad("an unknown token alarm with a grant", GrantAlarm{Subject: ok, Cause: AlarmUnknownToken}) bad("an alarm with a cause that is not known", GrantAlarm{Subject: ok, Cause: "x"}) + old := testRequestID(9) + good("a renew decision", Decision{Subject: ok, UserID: 1, Answer: AnswerRenew, RenewOf: old}) + bad("a renew decision without renew_of", Decision{Subject: ok, UserID: 1, Answer: AnswerRenew}) + bad("renew_of on an approval", Decision{Subject: ok, UserID: 1, Answer: AnswerApproveOnce, RenewOf: old}) + bad("a renew decision of the same grant", Decision{Subject: ok, UserID: 1, Answer: AnswerRenew, RenewOf: ok.GrantID}) + bad("a renew decision with renew_of that is not an ID", Decision{Subject: ok, UserID: 1, Answer: AnswerRenew, RenewOf: "x"}) + bad("a renew decision without a grant", Decision{Subject: noID, UserID: 1, Answer: AnswerRenew, RenewOf: old}) + bad("a renew decision of a scheduled grant", Decision{Subject: preapproveSubject(), UserID: 1, Answer: AnswerRenew, RenewOf: old}) for _, c := range []EndCause{EndAlarm, EndRenewed, EndRevoked, EndStopped, EndEnded} { good("grant_ended "+string(c), GrantEnded{Subject: ok, Cause: c}) } From 2295c03ee5902b9dfb96f6dd91e6d84c7db7e750 Mon Sep 17 00:00:00 2001 From: Geno Roupsky <185580+groupsky@users.noreply.github.com> Date: Thu, 29 Oct 2026 17:12:39 +0200 Subject: [PATCH 07/16] feat: the warning before the end of a standing grant, and Renew (#110) Co-Authored-By: Claude Opus 5.5 --- internal/approval/approval.go | 4 + internal/approval/approvaltest/approver.go | 39 ++ internal/approval/decide.go | 3 + internal/approval/grants.go | 59 ++- internal/approval/list.go | 2 +- internal/approval/missed.go | 2 +- internal/approval/renew.go | 251 +++++++++++++ internal/approval/renew_test.go | 400 +++++++++++++++++++++ internal/approval/standinguse.go | 9 +- internal/broker/broker.go | 8 + 10 files changed, 766 insertions(+), 11 deletions(-) create mode 100644 internal/approval/renew.go create mode 100644 internal/approval/renew_test.go diff --git a/internal/approval/approval.go b/internal/approval/approval.go index 4c3b7a1..810de2b 100644 --- a/internal/approval/approval.go +++ b/internal/approval/approval.go @@ -121,6 +121,10 @@ type Core struct { // and sends them. alarmQueue []missedAlarm onAlarm func(audit.GrantAlarm) + // warnQueue holds the warnings before the end of standing grants, + // until watchGrants sends them. onRenewed writes the entry of a Renew. + warnQueue []warnItem + onRenewed func(audit.Decision) error // msgQueue holds the updates of the messages of scheduled grants that // ended, until watchGrants makes them. msgQueue []msgUpdate diff --git a/internal/approval/approvaltest/approver.go b/internal/approval/approvaltest/approver.go index e16ee15..5f2e9e6 100644 --- a/internal/approval/approvaltest/approver.go +++ b/internal/approval/approvaltest/approver.go @@ -51,6 +51,8 @@ type Approver struct { notices []approval.Prompt alarms []approval.Alarm summaries [][]approval.GrantInfo + warnings []Warned + warnErr error alarmErr error askErr error notifyErr error @@ -389,3 +391,40 @@ func (a *Approver) Summaries() [][]approval.GrantInfo { defer a.mu.Unlock() return append([][]approval.GrantInfo(nil), a.summaries...) } + +// Warned is a warning before the end of a standing grant, and its message. +type Warned struct { + Warning approval.Warning + Message approval.MessageID +} + +// Warn keeps the warning, unless FailWarn set an error. +func (a *Approver) Warn(ctx context.Context, w approval.Warning) (approval.MessageID, error) { + if err := ctx.Err(); err != nil { + return "", err + } + a.mu.Lock() + defer a.mu.Unlock() + if a.warnErr != nil { + return "", a.warnErr + } + a.next++ + m := approval.MessageID("message-" + strconv.Itoa(a.next)) + a.warnings = append(a.warnings, Warned{Warning: w, Message: m}) + a.changedLocked() + return m, nil +} + +// FailWarn makes Warn fail with err. nil makes it work again. +func (a *Approver) FailWarn(err error) { + a.mu.Lock() + defer a.mu.Unlock() + a.warnErr = err +} + +// Warnings returns the warnings, oldest first. +func (a *Approver) Warnings() []Warned { + a.mu.Lock() + defer a.mu.Unlock() + return append([]Warned(nil), a.warnings...) +} diff --git a/internal/approval/decide.go b/internal/approval/decide.go index c6d932a..7d8154d 100644 --- a/internal/approval/decide.go +++ b/internal/approval/decide.go @@ -531,6 +531,9 @@ func (c *Core) waitLook(ctx context.Context, p *pending) *Result { // - ErrNotApprover, ErrWrongMessage, ErrExpired, ErrNotAllowed or // ErrStopped. func (c *Core) Answer(a Answer) error { + if a.Action == audit.AnswerRenew { + return c.renew(a) // it writes an entry, so it takes the lock itself + } c.mu.Lock() defer c.mu.Unlock() if c.stopped { diff --git a/internal/approval/grants.go b/internal/approval/grants.go index 2cdba8a..6715987 100644 --- a/internal/approval/grants.go +++ b/internal/approval/grants.go @@ -89,6 +89,21 @@ type grant struct { // lastUse is the wall time of the last use of a standing grant, and // prevUse the one before it (giveBackUse restores it). Zero: never. lastUse, prevUse time.Time + + // warnAt is when the warning before the end of a standing grant comes + // (zero: none, the grant is too short), warned says that it was + // queued, and warnMsg is its message, which has the Renew button. + warnAt time.Time + warned bool + warnMsg MessageID + // next is the renewal of a standing grant, which waits for the next + // use of this grant's token, and renewOf is the grant that a waiting + // renewal renews. renewing is true while the entry of a Renew is + // being written. + next, renewOf *grant + renewing bool + // endedAs is the cause with which the grant ended. + endedAs audit.EndCause } // grantKind is the kind of a grant (DESIGN.md section 8, "The grant core"). @@ -222,17 +237,34 @@ func (g *grant) touches(items []item) bool { // endWhere is the one place where grants end. It removes each grant for // which cause returns a cause, and returns the audit entries of the -// scheduled ones among them. Core.mu must be held. +// scheduled ones among them. A renewal that waits for its handover ends +// with its grant, with the same cause, because it has no token of its own. +// Core.mu must be held. func (c *Core) endWhere(cause func(*grant) audit.EndCause) []audit.GrantEnded { + why := make(map[*grant]audit.EndCause) + for _, g := range c.grants { + if w := cause(g); w != "" { + why[g] = w + } + } + for _, g := range c.grants { + if w := why[g]; w != "" && w != audit.EndRenewed && g.next != nil && why[g.next] == "" { + why[g.next] = w + } + } var ended []audit.GrantEnded c.grants = slices.DeleteFunc(c.grants, func(g *grant) bool { - why := cause(g) - if why == "" { + w := why[g] + if w == "" { return false } + g.endedAs = w + if g.renewOf != nil { + g.renewOf.next = nil + } if g.audited() { - ended = append(ended, audit.GrantEnded{Subject: g.origin, Cause: why, UsesLeft: max(g.uses, 0)}) - c.queueMessage(g, why) + ended = append(ended, audit.GrantEnded{Subject: g.origin, Cause: w, UsesLeft: max(g.uses, 0)}) + c.queueMessage(g, w) } return true }) @@ -264,6 +296,13 @@ func (c *Core) queueMessage(g *grant, why audit.EndCause) { // scheduled ones wait for the goroutine of watchGrants. Core.mu must be // held. func (c *Core) sweepGrants(now moment) { + // A grant with a renewal hands its token over at its end: its client + // gets the renewal's token on its next use. + for _, g := range slices.Clone(c.grants) { + if g.next != nil && !now.before(g.end) && now.before(g.next.end) { + c.handOver(g) + } + } c.queueEnded(c.endWhere(func(g *grant) audit.EndCause { if !now.before(g.end) { return audit.EndEnded @@ -430,10 +469,12 @@ func (c *Core) watchGrants(ctx context.Context) { now := c.now() c.sweepGrants(now) c.scanMissed(now) + c.scanWarnings(now) next, ok := c.nextEnd() c.mu.Unlock() c.deliverEnded() c.deliverAlarms() + c.deliverWarnings() c.deliverMessages(c.stopCtx) var timer <-chan time.Time var t Timer @@ -477,7 +518,10 @@ func (c *Core) nextEnd() (time.Time, bool) { continue } take(g.end.mono) - if g.standing() && now.before(g.end) { + if g.standing() && !g.warned && !g.warnAt.IsZero() { + take(now.mono.Add(max(g.warnAt.Sub(now.wall), 0))) + } + if g.standing() && !g.pendingRenewal() && now.before(g.end) { if left, ok := nextWindowEnd(g, now.wall); ok { take(now.mono.Add(left)) } @@ -603,7 +647,7 @@ func (c *Core) scheduledCount(client string) int { func (c *Core) standingCount(client string) int { n := 0 for _, g := range c.grants { - if g.standing() && g.client == client { + if g.standing() && !g.pendingRenewal() && g.client == client { n++ } } @@ -689,6 +733,7 @@ func (c *Core) startScheduled(r *Result, now moment) { } } if g.standing() { + g.warnAt = c.warnTime(g, now.wall) g.scanned = now.wall if g.start.After(g.scanned) { g.scanned = g.start diff --git a/internal/approval/list.go b/internal/approval/list.go index 7af20e6..83a5684 100644 --- a/internal/approval/list.go +++ b/internal/approval/list.go @@ -30,7 +30,7 @@ func (c *Core) ListStanding() []GrantInfo { now := c.now() var out []GrantInfo for _, g := range c.grants { - if !g.standing() || !now.before(g.end) { + if !g.standing() || g.pendingRenewal() || !now.before(g.end) { continue } gi := GrantInfo{Client: g.client, LastUse: g.lastUse, End: g.end.wall} diff --git a/internal/approval/missed.go b/internal/approval/missed.go index 4c9b7e7..ee657a0 100644 --- a/internal/approval/missed.go +++ b/internal/approval/missed.go @@ -82,7 +82,7 @@ func nextWindowEnd(g *grant, wall time.Time) (time.Duration, bool) { // forward. Core.mu must be held. func (c *Core) scanMissed(now moment) { for _, g := range c.grants { - if !g.standing() || !now.wall.After(g.scanned) { + if !g.standing() || g.pendingRenewal() || !now.wall.After(g.scanned) { continue } from := g.scanned diff --git a/internal/approval/renew.go b/internal/approval/renew.go new file mode 100644 index 0000000..3bbc00e --- /dev/null +++ b/internal/approval/renew.go @@ -0,0 +1,251 @@ +package approval + +import ( + "context" + "log/slog" + "slices" + "time" + + "github.com/code6x/nodkey/internal/audit" +) + +// Warning is what the approver shows warnDays before the end of a standing +// grant (DESIGN.md section 8): a message with a Renew button. It holds no +// token. +type Warning struct { + // RequestID is the ID of the request that made the grant. The Renew + // button carries it. + RequestID, ShortCode string + Client string + Secrets []string + // Windows are the windows as text, such as "daily 03:00-05:00", and + // Uses the uses in each. + Windows []string + Uses int + GrantID string + // End is when the grant ends, in the broker's local time. + End time.Time +} + +// Warner is an Approver that can send the warning before the end of a +// standing grant. It returns the message, which must be in the chat where +// the approver's taps count; a Renew counts only on it. The core sends a +// warning only to an approver that has this method. +type Warner interface { + Warn(ctx context.Context, w Warning) (MessageID, error) +} + +// warnItem is a queued warning and its grant. +type warnItem struct { + g *grant + w Warning +} + +// OnRenewed sets the function that writes the decision entry of a Renew. +// The new grant exists only after f returned nil; with no f, or when f +// fails, a Renew does not count. f is called from the approver's goroutine, +// not under the core's lock. Call it before Run. +func (c *Core) OnRenewed(f func(audit.Decision) error) { + c.mu.Lock() + defer c.mu.Unlock() + c.onRenewed = f +} + +// warnTime returns when the warning of the standing grant g comes, if g +// was approved at now: warnDays before its end. A grant that is not longer +// than that gets no warning (zero). +func (c *Core) warnTime(g *grant, now time.Time) time.Time { + at := g.end.wall.Add(-time.Duration(c.cfg.Standing.WarnDays) * 24 * time.Hour) + if !at.After(now) { + return time.Time{} + } + return at +} + +// pendingRenewal reports whether g is a renewal that waits for the next +// use of the token of the grant it renews. It has no token yet; it is not +// listed, has no alarms of its own and does not count toward max_grants. +func (g *grant) pendingRenewal() bool { return g.renewOf != nil } + +// scanWarnings queues the warning of each standing grant whose warning +// time came. Each grant gets one warning, also when the send fails. +// Core.mu must be held. +func (c *Core) scanWarnings(now moment) { + for _, g := range c.grants { + if !g.standing() || g.warned || g.warnAt.IsZero() || g.pendingRenewal() || g.next != nil || + now.wall.Before(g.warnAt) || !now.before(g.end) { + continue + } + g.warned = true + loc := c.clock.Wall().Location() + w := Warning{RequestID: g.reqID, ShortCode: g.code, Client: g.client, Uses: g.perWindow, + GrantID: g.id, End: g.end.wall.In(loc)} + for _, r := range g.refs { + w.Secrets = append(w.Secrets, r.name) + } + for _, x := range g.windows { + w.Windows = append(w.Windows, windowText(x)) + } + c.warnQueue = append(c.warnQueue, warnItem{g: g, w: w}) + } +} + +// deliverWarnings sends the queued warnings. A send that fails is logged; +// that grant gets no Renew button, and the person can make a new grant. +func (c *Core) deliverWarnings() { + c.mu.Lock() + q := c.warnQueue + c.warnQueue = nil + c.mu.Unlock() + wr, ok := c.approver.(Warner) + if !ok { + return + } + for _, it := range q { + ctx, cancel := c.callContext(c.stopCtx, endTimeout) + m, err := wr.Warn(ctx, it.w) + cancel() + if err != nil || m == "" { + c.logger.LogAttrs(ctx, slog.LevelWarn, "approval: cannot send the warning before the end of a standing grant", + slog.String("grant_id", it.w.GrantID), slog.Any("error", err)) + continue + } + c.mu.Lock() + if slices.Contains(c.grants, it.g) { + it.g.warnMsg = m + } + c.mu.Unlock() + } +} + +// renew makes a new standing grant for the Renew button on the warning of +// the grant of a.RequestID (DESIGN.md section 8): a new ID, the same scope, +// and the same length counted from now, at most max_days. It passes the +// same checks as an approval: the approver, the grant and its own warning +// message. The decision entry is written first; the new grant exists only +// after that. The new grant has no token: the next use of the old grant's +// token hands it over (handOver). +func (c *Core) renew(a Answer) error { + c.mu.Lock() + unlock := true + defer func() { + if unlock { + c.mu.Unlock() + } + }() + if c.stopped { + return ErrStopped + } + if a.UserID <= 0 || a.UserID != c.cfg.Telegram.UserID { + return ErrNotApprover + } + now := c.now() + c.sweepGrants(now) + i := slices.IndexFunc(c.grants, func(g *grant) bool { + return g.standing() && !g.pendingRenewal() && g.reqID == a.RequestID + }) + if i < 0 { + return ErrUnknownRequest + } + g := c.grants[i] + switch { + case g.next != nil || g.renewing: + return ErrUnknownRequest // renewed already + case g.warned && g.warnMsg == "": + return ErrTooEarly + case g.warnMsg == "" || a.Message != g.warnMsg: + return ErrWrongMessage + case c.onRenewed == nil: + return ErrNotAllowed + } + lim := c.cfg.Standing + // The new grant takes the place of g: g is counted, n is not. + if c.standingCount(g.client) > lim.MaxGrants { + return ErrNotAllowed + } + begin := g.start + if begin.IsZero() || g.approved.After(begin) { + begin = g.approved + } + length := min(g.end.wall.Sub(begin), time.Duration(lim.MaxDays)*24*time.Hour) + end := now.wall.Add(length).Truncate(time.Second) + if length <= 0 || !now.wall.Before(end) { + return ErrNotAllowed + } + id, err := newID(c.random) + if err != nil { + return ErrNotAllowed + } + reqID, err := newID(c.random) + if err != nil { + return ErrNotAllowed + } + n := &grant{ + kind: kindStanding, id: id, client: g.client, refs: slices.Clone(g.refs), + end: moment{wall: end, mono: now.mono.Add(end.Sub(now.wall))}, uses: noLimit, + reqID: reqID, code: g.code, approved: now.wall, + windows: slices.Clone(g.windows), perWindow: g.perWindow, scanned: now.wall, + } + n.warnAt = c.warnTime(n, now.wall) + n.origin = g.origin + n.origin.RequestID, n.origin.GrantID = reqID, id + n.origin.Secrets = slices.Clone(g.origin.Secrets) + n.origin.Windows = slices.Clone(g.origin.Windows) + n.origin.Hosts = nil + n.origin.NotBefore = now.wall.UTC().Truncate(time.Second).Format(time.RFC3339) + n.origin.NotAfter = end.UTC().Format(time.RFC3339) + loc := c.clock.Wall().Location() + n.prompt = Prompt{RequestID: reqID, ShortCode: g.code, Kind: audit.KindStanding, Client: g.client, + Secrets: slices.Clone(n.origin.Secrets), Uses: g.perWindow, From: now.wall.In(loc), Until: end.In(loc), + Windows: slices.Clone(g.origin.Windows)} + ev := audit.Decision{Subject: n.origin, UserID: a.UserID, Answer: audit.AnswerRenew, RenewOf: g.id} + ev.Subject.Secrets = slices.Clone(n.origin.Secrets) + ev.Subject.Windows = slices.Clone(n.origin.Windows) + f := c.onRenewed + g.renewing = true // a second tap waits for nothing: it is refused + c.mu.Unlock() + werr := f(ev) + c.mu.Lock() + g.renewing = false + if werr != nil { + return ErrNotAllowed + } + if !slices.Contains(c.grants, g) { + // g ended while the entry was written: n ends as g did, and was + // never usable, because only g's token could reach it. + c.queueEnded([]audit.GrantEnded{{Subject: n.origin, Cause: g.endedAs}}) + return ErrUnknownRequest + } + n.renewOf, g.next = g, n + c.grants = append(c.grants, n) + // The warning becomes the message of the new grant, with its Revoke + // button. + n.msg = g.warnMsg + g.warnMsg = "" + c.msgQueue = append(c.msgQueue, msgUpdate{msg: n.msg, prompt: n.prompt, + ending: Ending{Answer: audit.AnswerApproveOnce, At: now.wall, Revoke: true}}) + c.wakeGrants() + return nil +} + +// handOver ends the standing grant g, which has a renewal, with cause +// renewed, and gives the renewal g's token, its old tokens and its counts: +// the token that g's client holds is now the renewal's, and an old token +// is still an alarm. It runs on the next use of g's token, or at g's end. +// Core.mu must be held. +func (c *Core) handOver(g *grant) *grant { + n := g.next + n.tokenHash, n.oldHashes = g.tokenHash, g.oldHashes + n.winKey, n.winUsed, n.marked, n.used = g.winKey, g.winUsed, g.marked, g.used + n.lastUse, n.prevUse, n.alarmMark = g.lastUse, g.prevUse, g.alarmMark + n.scanned = g.scanned + g.next, n.renewOf = nil, nil + g.oldHashes, g.used = nil, nil + c.queueEnded(c.endWhere(func(x *grant) audit.EndCause { + if x == g { + return audit.EndRenewed + } + return "" + })) + return n +} diff --git a/internal/approval/renew_test.go b/internal/approval/renew_test.go new file mode 100644 index 0000000..3c33043 --- /dev/null +++ b/internal/approval/renew_test.go @@ -0,0 +1,400 @@ +package approval_test + +// Tests of the warning before the end of a standing grant and of Renew +// (DESIGN.md section 8). The fake clock starts at noon on 2026-10-14; the +// window is 03:00-05:00 each day, the grant lasts 30 days, and warn_days +// is 7, so the warning comes at noon on 2026-11-06. + +import ( + "errors" + "sync" + "testing" + "testing/synctest" + "time" + + "github.com/code6x/nodkey/internal/approval" + "github.com/code6x/nodkey/internal/approval/approvaltest" + "github.com/code6x/nodkey/internal/audit" + "github.com/code6x/nodkey/internal/config" + "github.com/code6x/nodkey/internal/grantkey" +) + +const day = 24 * time.Hour + +// renewRec records the decision entries of Renew taps. +type renewRec struct { + mu sync.Mutex + evs []audit.Decision + fail error +} + +func (r *renewRec) add(ev audit.Decision) error { + r.mu.Lock() + defer r.mu.Unlock() + if r.fail != nil { + return r.fail + } + r.evs = append(r.evs, ev) + return nil +} + +func (r *renewRec) all() []audit.Decision { + r.mu.Lock() + defer r.mu.Unlock() + return append([]audit.Decision(nil), r.evs...) +} + +// renewEnv starts a core with one standing grant of laptop. It returns the +// token and the result of the grant. +func renewEnv(t *testing.T, edit ...func(*config.Broker)) (*env, grantkey.Token, *approval.Result, *renewRec, *endedRecorder) { + t.Helper() + e := standEnv(t, edit...) + rec, ended := &renewRec{}, &endedRecorder{} + e.core.OnRenewed(rec.add) + e.core.OnGrantEnded(ended.add) + tok := newToken(t) + res := e.grant(standReq("laptop", tok, "github-token"), 1) + return e, tok, res, rec, ended +} + +// warning waits for the one warning and returns it. +func (e *env) warning() approvaltest.Warned { + e.t.Helper() + w := e.fake.Warnings() + if len(w) != 1 { + e.t.Fatalf("%d warnings, want 1", len(w)) + } + return w[0] +} + +// renewTap presses Renew on the warning w, as the approver. +func (e *env) renewTap(w approvaltest.Warned) error { + return e.fake.Answer(approval.Answer{RequestID: w.Warning.RequestID, Action: audit.AnswerRenew, + Message: w.Message, UserID: approverID}) +} + +// use uses a standing grant of laptop with tok and returns the result. +func (e *env) use(tok grantkey.Token) *approval.Result { + h := grantkey.Hash(tok) + return e.core.UseStanding(approval.StandingRequest{Client: "laptop", Secrets: []string{"github-token"}, Token: &h}) +} + +func alarmOf(res *approval.Result) audit.AlarmCause { + for _, ev := range res.Events() { + if a, ok := ev.(audit.GrantAlarm); ok { + return a.Cause + } + } + return "" +} + +func TestWarningOnceBeforeTheEnd(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + e, _, res, _, _ := renewEnv(t) + defer e.stop() + e.pass(23*day - time.Minute) + if n := len(e.fake.Warnings()); n != 0 { + t.Fatalf("%d warnings before warn_days", n) + } + e.pass(2 * time.Minute) + w := e.warning().Warning + if w.RequestID != res.RequestID || w.GrantID != res.GrantID || w.Client != "laptop" || w.Uses != 2 || + len(w.Secrets) != 1 || w.Secrets[0] != "github-token" || len(w.Windows) != 1 || + w.Windows[0] != "daily 03:00-05:00" || !w.End.Equal(noon.Add(30*day)) || w.ShortCode == "" { + t.Errorf("warning %+v", w) + } + e.pass(day) + e.pass(5 * day) + if n := len(e.fake.Warnings()); n != 1 { + t.Errorf("%d warnings, want one", n) + } + }) +} + +func TestWarningSendFailsNotRepeated(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + e, _, _, _, _ := renewEnv(t) + defer e.stop() + e.fake.FailWarn(errors.New("telegram is down")) + e.pass(23*day + time.Minute) + e.fake.FailWarn(nil) + e.pass(day) + if n := len(e.fake.Warnings()); n != 0 { + t.Errorf("%d warnings: a grant gets one try", n) + } + }) +} + +func TestNoWarningForAShortGrant(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + e := standEnv(t) + defer e.stop() + r := standReq("laptop", newToken(t), "github-token") + r.NotAfter = noon.Add(7 * day) // not longer than warn_days + e.grant(r, 1) + e.pass(8 * day) + if n := len(e.fake.Warnings()); n != 0 { + t.Errorf("%d warnings for a grant of 7 days", n) + } + }) +} + +func TestNoWarningAfterTheGrantEnded(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + e, _, res, _, _ := renewEnv(t) + defer e.stop() + e.pass(day) + if _, ok := e.core.CancelGrant("laptop", res.GrantID); !ok { + t.Fatal("cannot cancel the grant") + } + e.pass(30 * day) + if n := len(e.fake.Warnings()); n != 0 { + t.Errorf("%d warnings for an ended grant", n) + } + }) +} + +// TestRenewHandsOverOnTheNextUse: Renew writes its entry and makes a new +// grant with no token. The next use of the old token gets a token of the +// new grant, and the old grant ends with cause renewed. The old token used +// again is an old_token alarm, which ends the new grant. +func TestRenewHandsOverOnTheNextUse(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + e, tok, old, rec, ended := renewEnv(t) + defer e.stop() + e.pass(23*day + time.Minute) // noon on 2026-11-06 + w := e.warning() + at := e.clock.Wall() + if err := e.renewTap(w); err != nil { + t.Fatalf("Renew: %v", err) + } + synctest.Wait() + evs := rec.all() + if len(evs) != 1 { + t.Fatalf("%d renew entries", len(evs)) + } + d := evs[0] + end := at.Add(30 * day).Truncate(time.Second) + if d.Answer != audit.AnswerRenew || d.RenewOf != old.GrantID || d.GrantID == "" || d.GrantID == old.GrantID || + d.RequestID == old.RequestID || d.Kind != audit.KindStanding || d.UserID != approverID || + d.NotAfter != end.UTC().Format(time.RFC3339) || len(d.Secrets) != 1 || d.Uses != 2 || len(d.Windows) != 1 { + t.Errorf("renew entry %+v", d) + } + // The warning now carries the Revoke button of the new grant. + var hold *approvaltest.Ended + for _, u := range e.fake.Ended() { + if u.Message == w.Message { + hold = &u + } + } + if hold == nil || !hold.Ending.Revoke || hold.Prompt.RequestID != d.RequestID || !hold.Prompt.Until.Equal(end) { + t.Errorf("the warning message was not given the new grant: %+v", hold) + } + if err := e.renewTap(w); !errors.Is(err, approval.ErrUnknownRequest) { + t.Errorf("a second Renew: %v, want ErrUnknownRequest", err) + } + if n := len(ended.all()); n != 0 { + t.Fatalf("%d ended entries before the handover", n) + } + + e.pass(15*time.Hour + 30*time.Minute) // 03:30 on 2026-11-07 + res := e.use(tok) + checkAllowed(t, res) + next, ok := res.NewToken() + if !ok { + t.Fatal("no new token") + } + var used audit.GrantUsed + for _, ev := range res.Events() { + if u, ok := ev.(audit.GrantUsed); ok { + used = u + } + } + if used.GrantID != d.GrantID || used.Via != audit.ViaStanding { + t.Errorf("the use names %+v, want the new grant", used) + } + synctest.Wait() + got := ended.all() + if len(got) != 1 || got[0].Cause != audit.EndRenewed || got[0].GrantID != old.GrantID { + t.Fatalf("ended entries %+v; want the old grant, renewed", got) + } + if h, _, _, _ := e.core.StandingGrants("laptop"); len(h) != 1 || h[0] != grantkey.Hash(next) { + t.Error("the new grant must hold the new token") + } + // The second use in the same window still counts toward the window. + next = e.useGrant(next) + if r := e.use(next); r.Allowed || alarmOf(r) != audit.AlarmOverUses { + t.Errorf("a third use in the window: allowed %v, alarm %q", r.Allowed, alarmOf(r)) + } + }) +} + +func TestOldTokenAfterRenewalIsAnAlarm(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + e, tok, _, rec, ended := renewEnv(t) + defer e.stop() + e.pass(23*day + time.Minute) + if err := e.renewTap(e.warning()); err != nil { + t.Fatal(err) + } + e.pass(15*time.Hour + 30*time.Minute) + _ = e.useGrant(tok) + res := e.use(tok) + if res.Allowed || alarmOf(res) != audit.AlarmOldToken { + t.Fatalf("the old token again: allowed %v, alarm %q", res.Allowed, alarmOf(res)) + } + var alarmEnd []audit.GrantEnded + for _, ev := range res.Events() { + if g, ok := ev.(audit.GrantEnded); ok { + alarmEnd = append(alarmEnd, g) + } + } + if len(alarmEnd) != 1 || alarmEnd[0].Cause != audit.EndAlarm || alarmEnd[0].GrantID != rec.all()[0].GrantID { + t.Errorf("the alarm ended %+v; want the new grant", alarmEnd) + } + if h, _, _, _ := e.core.StandingGrants("laptop"); len(h) != 0 { + t.Error("a grant is left after the alarm") + } + _ = ended + }) +} + +// TestRenewHandsOverAtTheOldEnd: with no use before the old grant's end, +// the old token passes to the new grant at that end. +func TestRenewHandsOverAtTheOldEnd(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + e, tok, old, rec, ended := renewEnv(t) + defer e.stop() + e.pass(23*day + time.Minute) + if err := e.renewTap(e.warning()); err != nil { + t.Fatal(err) + } + e.pass(7 * day) // past the old end + got := ended.all() + if len(got) != 1 || got[0].Cause != audit.EndRenewed || got[0].GrantID != old.GrantID { + t.Fatalf("ended entries %+v", got) + } + e.pass(15*time.Hour + 30*time.Minute) // 03:30 + res := e.use(tok) + checkAllowed(t, res) + var used audit.GrantUsed + for _, ev := range res.Events() { + if u, ok := ev.(audit.GrantUsed); ok { + used = u + } + } + if used.GrantID != rec.all()[0].GrantID { + t.Errorf("the use names grant %q, want the new grant", used.GrantID) + } + }) +} + +// TestRevokeOfARenewedGrantEndsTheRenewal: the renewal has no token of its +// own, so it ends with its grant. +func TestRevokeOfARenewedGrantEndsTheRenewal(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + e, tok, _, _, ended := renewEnv(t) + defer e.stop() + e.pass(23*day + time.Minute) + if err := e.renewTap(e.warning()); err != nil { + t.Fatal(err) + } + e.tap(1, audit.AnswerRevoke) + synctest.Wait() + got := ended.all() + if len(got) != 2 || got[0].Cause != audit.EndRevoked || got[1].Cause != audit.EndRevoked { + t.Fatalf("ended entries %+v; want both revoked", got) + } + e.pass(15*time.Hour + 30*time.Minute) + if res := e.use(tok); res.Allowed { + t.Error("the token works after Revoke") + } + }) +} + +func TestRenewRefused(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + e, _, old, rec, _ := renewEnv(t) + defer e.stop() + // No warning yet: no Renew. + ans := approval.Answer{RequestID: old.RequestID, Action: audit.AnswerRenew, Message: e.fake.Asked()[0].Message, UserID: approverID} + if err := e.fake.Answer(ans); !errors.Is(err, approval.ErrWrongMessage) { + t.Errorf("Renew before the warning: %v", err) + } + e.pass(23*day + time.Minute) + w := e.warning() + tests := []struct { + name string + ans approval.Answer + want error + }{ + {"another user", approval.Answer{RequestID: w.Warning.RequestID, Action: audit.AnswerRenew, Message: w.Message, UserID: approverID + 1}, approval.ErrNotApprover}, + {"no user", approval.Answer{RequestID: w.Warning.RequestID, Action: audit.AnswerRenew, Message: w.Message}, approval.ErrNotApprover}, + {"another message", approval.Answer{RequestID: w.Warning.RequestID, Action: audit.AnswerRenew, Message: e.fake.Asked()[0].Message, UserID: approverID}, approval.ErrWrongMessage}, + {"another chat", approval.Answer{RequestID: w.Warning.RequestID, Action: audit.AnswerRenew, Message: "other:" + w.Message, UserID: approverID}, approval.ErrWrongMessage}, + {"an unknown request", approval.Answer{RequestID: w.Warning.GrantID, Action: audit.AnswerRenew, Message: w.Message, UserID: approverID}, approval.ErrUnknownRequest}, + } + for _, tt := range tests { + if err := e.fake.Answer(tt.ans); !errors.Is(err, tt.want) { + t.Errorf("%s: %v, want %v", tt.name, err, tt.want) + } + } + // The entry cannot be written: no new grant. + rec.mu.Lock() + rec.fail = errors.New("disk full") + rec.mu.Unlock() + if err := e.renewTap(w); !errors.Is(err, approval.ErrNotAllowed) { + t.Errorf("Renew without its entry: %v", err) + } + if n := len(e.core.ListStanding()); n != 1 { + t.Errorf("%d grants listed", n) + } + if h, _, _, _ := e.core.StandingGrants("laptop"); len(h) != 1 { + t.Errorf("%d grants after a failed Renew, want 1", len(h)) + } + if n := len(rec.all()); n != 0 { + t.Errorf("%d entries", n) + } + // An ended grant cannot be renewed. + if _, ok := e.core.CancelGrant("laptop", old.GrantID); !ok { + t.Fatal("cannot cancel") + } + rec.mu.Lock() + rec.fail = nil + rec.mu.Unlock() + if err := e.renewTap(w); !errors.Is(err, approval.ErrUnknownRequest) { + t.Errorf("Renew of an ended grant: %v", err) + } + if n := len(rec.all()); n != 0 { + t.Errorf("%d entries", n) + } + }) +} + +func TestRenewKeepsToMaxDaysAndMaxGrants(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + e, _, _, rec, _ := renewEnv(t) + defer e.stop() + e.pass(23*day + time.Minute) + w := e.warning() + e.cfg.Standing.MaxGrants = 0 // as if the client had too many + if err := e.renewTap(w); !errors.Is(err, approval.ErrNotAllowed) { + t.Errorf("Renew over max_grants: %v", err) + } + e.cfg.Standing.MaxGrants = 1 + e.cfg.Standing.MaxDays = 10 + at := e.clock.Wall() + if err := e.renewTap(w); err != nil { + t.Fatal(err) + } + evs := rec.all() + if len(evs) != 1 || evs[0].NotAfter != at.Add(10*day).Truncate(time.Second).UTC().Format(time.RFC3339) { + t.Errorf("renew entries %+v; want an end after max_days", evs) + } + // The renewal takes the place of its grant: a client at max_grants + // stays there. + if n := len(e.core.ListStanding()); n != 1 { + t.Errorf("%d grants listed, want 1", n) + } + }) +} diff --git a/internal/approval/standinguse.go b/internal/approval/standinguse.go index 6aa6951..bf2a628 100644 --- a/internal/approval/standinguse.go +++ b/internal/approval/standinguse.go @@ -104,8 +104,8 @@ func (c *Core) UseStanding(r StandingRequest) *Result { var cur, old, foreign *grant if r.Token != nil { for _, g := range c.grants { - if !g.standing() { - continue + if !g.standing() || g.pendingRenewal() { + continue // a renewal has no token before its handover } mine := g.client == p.client if grantkey.Equal(g.tokenHash, *r.Token) { @@ -160,6 +160,11 @@ func (c *Core) UseStanding(r StandingRequest) *Result { return deny(old, audit.AlarmOldToken, true) } g := cur + if g.next != nil { + // g was renewed: its token now belongs to the renewal, which this + // use rotates (DESIGN.md section 8). + g = c.handOver(g) + } for _, it := range p.items { if !g.covers(p.client, it.ref, "", "") { return c.denied(p, audit.CauseNotAllowed) // a client's mistake, not an alarm diff --git a/internal/broker/broker.go b/internal/broker/broker.go index 04c7cd9..df6b05e 100644 --- a/internal/broker/broker.go +++ b/internal/broker/broker.go @@ -410,6 +410,14 @@ func (s *Service) Run(ctx context.Context, core *approval.Core, alert AlertFunc, s.log.Warn("broker: cannot write the grant_alarm entry of a missed window") } }) + // A Renew counts only when its decision entry is on disk. + core.OnRenewed(func(ev audit.Decision) error { + if _, err := s.append(ev); err != nil { + s.log.Warn("broker: cannot write the decision entry of a Renew; the Renew does not count") + return err + } + return nil + }) err := core.Run(ctx) s.coreStopped.Store(true) // first: deny every request from now on // The grants are dropped by now. Their messages are updated, and one From 7f4d07e89e89526cdf295c71e91fa8c35132b1f3 Mon Sep 17 00:00:00 2001 From: Geno Roupsky <185580+groupsky@users.noreply.github.com> Date: Thu, 29 Oct 2026 17:14:21 +0200 Subject: [PATCH 08/16] feat: the Telegram warning with a Renew button (#110) Co-Authored-By: Claude Opus 5.5 --- internal/approval/grants.go | 6 ++ internal/approval/renew_test.go | 10 +++ internal/telegram/poll.go | 10 +++ internal/telegram/renew.go | 84 ++++++++++++++++++++++ internal/telegram/renew_test.go | 122 ++++++++++++++++++++++++++++++++ internal/telegram/text.go | 7 +- 6 files changed, 238 insertions(+), 1 deletion(-) create mode 100644 internal/telegram/renew.go create mode 100644 internal/telegram/renew_test.go diff --git a/internal/approval/grants.go b/internal/approval/grants.go index 6715987..b664be3 100644 --- a/internal/approval/grants.go +++ b/internal/approval/grants.go @@ -265,6 +265,12 @@ func (c *Core) endWhere(cause func(*grant) audit.EndCause) []audit.GrantEnded { if g.audited() { ended = append(ended, audit.GrantEnded{Subject: g.origin, Cause: w, UsesLeft: max(g.uses, 0)}) c.queueMessage(g, w) + if g.warnMsg != "" { + // The warning loses its Renew button. + x := *g + x.msg = g.warnMsg + c.queueMessage(&x, w) + } } return true }) diff --git a/internal/approval/renew_test.go b/internal/approval/renew_test.go index 3c33043..5ac07f9 100644 --- a/internal/approval/renew_test.go +++ b/internal/approval/renew_test.go @@ -362,6 +362,16 @@ func TestRenewRefused(t *testing.T) { rec.mu.Lock() rec.fail = nil rec.mu.Unlock() + synctest.Wait() + var last approval.Ending + for _, u := range e.fake.Ended() { + if u.Message == w.Message { + last = u.Ending + } + } + if last.GrantEnd != audit.EndCancelled || last.Revoke { + t.Errorf("the warning was not updated at the end of its grant: %+v", last) + } if err := e.renewTap(w); !errors.Is(err, approval.ErrUnknownRequest) { t.Errorf("Renew of an ended grant: %v", err) } diff --git a/internal/telegram/poll.go b/internal/telegram/poll.go index d438ffc..aafa35e 100644 --- a/internal/telegram/poll.go +++ b/internal/telegram/poll.go @@ -42,6 +42,7 @@ const ( tapCountedApprove = "Approved." tapCountedDeny = "Denied." tapCountedRevoke = "Revoked." + tapCountedRenew = "Renewed." tapTooEarly = "Not ready yet. Tap again in a moment." tapEnded = "This request has ended." tapExpired = "This request has expired." @@ -245,6 +246,8 @@ func tapResult(action audit.Answer, err error) (string, audit.TapCause) { return tapCountedDeny, "" case err == nil && action == audit.AnswerRevoke: return tapCountedRevoke, "" + case err == nil && action == audit.AnswerRenew: + return tapCountedRenew, "" case err == nil: return tapCountedApprove, "" case errors.Is(err, approval.ErrTooEarly): @@ -319,6 +322,13 @@ func (a *Approver) changed(ctx context.Context, q *callbackQuery, ans approval.A } return false, false } + if ans.Action == audit.AnswerRenew && known && sent.held { + // A Renew made this message the message of the new grant, with its + // Revoke button: a second Renew is a late tap. + a.answerTap(ctx, q, tapEnded) + a.report(ctx, q, audit.TapUnknownRequest) + return true, false + } if known && sent.final == "" && sent.held { // The request of this message is decided; End gave it a Revoke // button. Only a Revoke can count on it. A button of its request is diff --git a/internal/telegram/renew.go b/internal/telegram/renew.go new file mode 100644 index 0000000..a7371d9 --- /dev/null +++ b/internal/telegram/renew.go @@ -0,0 +1,84 @@ +package telegram + +import ( + "context" + "errors" + "strconv" + "strings" + + "github.com/code6x/nodkey/internal/approval" +) + +// renewKeyboard returns the one Renew button of the warning before the end +// of the standing grant of the request id. +func renewKeyboard(id string) inlineKeyboard { + return inlineKeyboard{InlineKeyboard: [][]inlineButton{{{Text: "Renew", CallbackData: dataRenew + id}}}} +} + +// warningText returns the text of the warning before the end of a +// standing grant (DESIGN.md section 8). It holds names from the config, +// the windows, the grant ID and the end; no token and no claimed text. +func warningText(w approval.Warning) (*text, error) { + bad := func(what string) error { return errors.New("telegram: the warning is not valid: " + what) } + switch { + case !validRequestID(w.RequestID): + return nil, bad("the request ID") + case !shortCodePattern.MatchString(w.ShortCode): + return nil, bad("the short code") + case !namePattern.MatchString(w.Client): + return nil, bad("the client name") + case len(w.Secrets) == 0 || !allNames(w.Secrets): + return nil, bad("the secrets") + case len(w.Windows) == 0 || len(w.Windows) > 7: + return nil, bad("the windows") + case !validRequestID(w.GrantID): + return nil, bad("the grant ID") + case w.Uses < 1 || w.End.IsZero(): + return nil, bad("the uses or the end") + } + for _, x := range w.Windows { + if !windowPattern.MatchString(x) { + return nil, bad("the windows") + } + } + t := &text{} + t.write("โณ Standing grant ends soon ยท request " + w.ShortCode) + t.write("\nโœ… machine: " + w.Client) + t.write("\nโœ… secrets: " + strings.Join(w.Secrets, ", ")) + t.write("\nโœ… windows: " + strings.Join(w.Windows, ", ") + ", " + strconv.Itoa(w.Uses) + " use(s) in each") + t.write("\nโœ… grant: " + w.GrantID) + t.write("\nโœ… ends: " + w.End.Format(dateTimeFormat)) + t.write("\nRenew makes a new grant of the same length from now. The next use of the job's token gets a token of the new grant.") + return t, nil +} + +// Warn sends the warning before the end of a standing grant to the +// approver's private chat, with a sound and one Renew button, and returns +// its message. A Renew counts only on this message, as Ask sent it. +func (a *Approver) Warn(ctx context.Context, w approval.Warning) (approval.MessageID, error) { + if err := ctx.Err(); err != nil { + return "", err + } + t, err := warningText(w) + if err != nil { + return "", err + } + kb := renewKeyboard(w.RequestID) + var sent message + err = a.api.call(ctx, "sendMessage", sendMessageParams{ + ChatID: a.userID, + Text: t.html.String(), + ParseMode: "HTML", + LinkPreviewOptions: noPreview, + ReplyMarkup: &kb, + }, &sent) + if err != nil { + return "", err + } + if sent.MessageID <= 0 || sent.Chat.ID != a.userID { + return "", errors.New("telegram: sendMessage: the answer names another chat, or no message") + } + m := messageID(sent.Chat.ID, sent.MessageID) + a.remember(m, kb.InlineKeyboard) + return m, nil +} diff --git a/internal/telegram/renew_test.go b/internal/telegram/renew_test.go new file mode 100644 index 0000000..72a6be3 --- /dev/null +++ b/internal/telegram/renew_test.go @@ -0,0 +1,122 @@ +package telegram + +import ( + "context" + "strings" + "testing" + "time" + + "github.com/code6x/nodkey/internal/approval" + "github.com/code6x/nodkey/internal/audit" +) + +// Tests of the warning before the end of a standing grant and of its +// Renew button (DESIGN.md section 8). + +func warningOf() approval.Warning { + return approval.Warning{RequestID: testID(1), ShortCode: "m4k2", Client: "laptop", Secrets: []string{"backup-token"}, + Windows: []string{"daily 03:00-05:00"}, Uses: 1, GrantID: testID(5), + End: time.Date(2026, 11, 13, 12, 0, 0, 0, time.UTC)} +} + +func TestRenewData(t *testing.T) { + kb := renewKeyboard(testID(1)) + if len(kb.InlineKeyboard) != 1 || len(kb.InlineKeyboard[0]) != 1 || kb.InlineKeyboard[0][0].Text != "Renew" { + t.Fatalf("keyboard %+v", kb) + } + action, id, ok := parseData(kb.InlineKeyboard[0][0].CallbackData) + if !ok || action != audit.AnswerRenew || id != testID(1) { + t.Errorf("parseData = %q, %q, %v", action, id, ok) + } + for _, bad := range []string{"n:", "n:" + testID(1) + "x", "N:" + testID(1)} { + if _, _, ok := parseData(bad); ok { + t.Errorf("parseData(%q) was accepted", bad) + } + } +} + +func TestWarningText(t *testing.T) { + tx, err := warningText(warningOf()) + if err != nil { + t.Fatal(err) + } + got := tx.plain.String() + for _, want := range []string{"โณ Standing grant ends soon ยท request m4k2", "โœ… machine: laptop", "โœ… secrets: backup-token", + "โœ… windows: daily 03:00-05:00, 1 use(s) in each", "โœ… grant: " + testID(5), "โœ… ends: 2026-11-13 12:00:00 UTC", "Renew makes"} { + if !strings.Contains(got, want) { + t.Errorf("warning\n%s\nwant %q", got, want) + } + } + for name, change := range map[string]func(*approval.Warning){ + "a bad request ID": func(w *approval.Warning) { w.RequestID = "x" }, + "a bad short code": func(w *approval.Warning) { w.ShortCode = "" }, + "a bad client": func(w *approval.Warning) { w.Client = "a b" }, + "no secrets": func(w *approval.Warning) { w.Secrets = nil }, + "a bad secret": func(w *approval.Warning) { w.Secrets = []string{""} }, + "no windows": func(w *approval.Warning) { w.Windows = nil }, + "a bad window": func(w *approval.Warning) { w.Windows = []string{"daily"} }, + "a bad grant ID": func(w *approval.Warning) { w.GrantID = "" }, + "no uses": func(w *approval.Warning) { w.Uses = 0 }, + "no end": func(w *approval.Warning) { w.End = time.Time{} }, + } { + w := warningOf() + change(&w) + if _, err := warningText(w); err == nil { + t.Errorf("%s: accepted", name) + } + } +} + +// TestWarnAndRenewTap: the warning goes to the private chat, with a sound +// and one Renew button. A Renew on it reaches the core; a Renew from +// another user or chat, or on a changed warning, does not. +func TestWarnAndRenewTap(t *testing.T) { + e := newEnv(t) + core := &stubCore{} + e.run(t, core.answer) + m, err := e.a.Warn(context.Background(), warningOf()) + if err != nil { + t.Fatal(err) + } + calls := e.f.callsOf("sendMessage") + if len(calls) != 1 { + t.Fatalf("%d messages", len(calls)) + } + c := calls[0] + if c.num("chat_id") != approverID || c.field("disable_notification") == "true" || + len(c.buttonData()) != 1 || c.buttonData()[0] != dataRenew+testID(1) { + t.Errorf("warning %s", c.Raw) + } + _, msg, ok := parseMessageID(m) + if !ok { + t.Fatalf("message ID %q", m) + } + data := dataRenew + testID(1) + for name, tc := range map[string]struct { + spec tapSpec + cause audit.TapCause + }{ + "other user": {tapSpec{from: otherUserID, chatID: approverID, chatType: "private", msgID: msg, data: data}, audit.TapOtherUser}, + "other chat": {tapSpec{from: approverID, chatID: noticeChatID, chatType: "supergroup", msgID: msg, data: data}, audit.TapOtherChat}, + "edited": {func() tapSpec { s := goodTap(msg, data); s.edited = true; return s }(), audit.TapEditedMessage}, + } { + before := len(e.taps.all()) + e.f.waitHandled(e.f.tap(tc.spec)) + taps := e.taps.all() + if len(taps) != before+1 || taps[before].Cause != tc.cause { + t.Errorf("%s: reports %+v, want cause %s", name, taps[before:], tc.cause) + } + } + if n := len(core.all()); n != 0 { + t.Fatalf("the core got %d answers from refused taps", n) + } + cb := e.f.tap(goodTap(msg, data)) + if got := e.f.waitAnswer(cb).str("text"); got != tapCountedRenew { + t.Errorf("the phone shows %q", got) + } + got := core.all() + want := approval.Answer{RequestID: testID(1), Action: audit.AnswerRenew, Message: m, UserID: approverID} + if len(got) != 1 || got[0] != want { + t.Errorf("the core got %+v, want %+v", got, want) + } +} diff --git a/internal/telegram/text.go b/internal/telegram/text.go index 8869378..83609e5 100644 --- a/internal/telegram/text.go +++ b/internal/telegram/text.go @@ -90,6 +90,9 @@ const ( // dataRevoke is the Revoke button of a scheduled grant. It carries the // ID of the request that made the grant. dataRevoke = "r:" + // dataRenew is the Renew button of the warning before the end of a + // standing grant. It carries the ID of the request that made the grant. + dataRenew = "n:" dataLen = 2 + requestIDLen ) @@ -113,7 +116,7 @@ func isBase64URL(c byte) bool { return 'A' <= c && c <= 'Z' || 'a' <= c && c <= 'z' || '0' <= c && c <= '9' || c == '-' || c == '_' } -// parseData reads button data strictly: "o:", "w:", "d:" or "r:", and a +// parseData reads button data strictly: "o:", "w:", "d:", "r:" or "n:", and a // request ID. Anything else is not valid. func parseData(data string) (audit.Answer, string, bool) { if len(data) != dataLen { @@ -129,6 +132,8 @@ func parseData(data string) (audit.Answer, string, bool) { action = audit.AnswerDeny case dataRevoke: action = audit.AnswerRevoke + case dataRenew: + action = audit.AnswerRenew default: return "", "", false } From 850a17e8bf8ac55da39b957a7628522280ff7f1e Mon Sep 17 00:00:00 2001 From: Geno Roupsky <185580+groupsky@users.noreply.github.com> Date: Thu, 29 Oct 2026 17:14:35 +0200 Subject: [PATCH 09/16] chore: gofmt and lint of the Renew code (#110) Co-Authored-By: Claude Opus 5.5 --- internal/approval/renew.go | 4 ++-- internal/telegram/text.go | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/approval/renew.go b/internal/approval/renew.go index 3bbc00e..c63fa9c 100644 --- a/internal/approval/renew.go +++ b/internal/approval/renew.go @@ -199,8 +199,8 @@ func (c *Core) renew(a Answer) error { Secrets: slices.Clone(n.origin.Secrets), Uses: g.perWindow, From: now.wall.In(loc), Until: end.In(loc), Windows: slices.Clone(g.origin.Windows)} ev := audit.Decision{Subject: n.origin, UserID: a.UserID, Answer: audit.AnswerRenew, RenewOf: g.id} - ev.Subject.Secrets = slices.Clone(n.origin.Secrets) - ev.Subject.Windows = slices.Clone(n.origin.Windows) + ev.Secrets = slices.Clone(n.origin.Secrets) + ev.Windows = slices.Clone(n.origin.Windows) f := c.onRenewed g.renewing = true // a second tap waits for nothing: it is refused c.mu.Unlock() diff --git a/internal/telegram/text.go b/internal/telegram/text.go index 83609e5..bb3dd37 100644 --- a/internal/telegram/text.go +++ b/internal/telegram/text.go @@ -93,7 +93,7 @@ const ( // dataRenew is the Renew button of the warning before the end of a // standing grant. It carries the ID of the request that made the grant. dataRenew = "n:" - dataLen = 2 + requestIDLen + dataLen = 2 + requestIDLen ) // validRequestID reports whether id is a request ID: 22 characters of From b81cbc32a566f5484404fb117a9074e2a0ea9a9a Mon Sep 17 00:00:00 2001 From: Geno Roupsky <185580+groupsky@users.noreply.github.com> Date: Thu, 29 Oct 2026 17:15:08 +0200 Subject: [PATCH 10/16] docs: the warning and Renew of standing grants as built (#110) Co-Authored-By: Claude Opus 5.5 --- DESIGN.md | 34 ++++++++++++++++++++++++++++------ THREAT-MODEL.md | 9 +++++++++ 2 files changed, 37 insertions(+), 6 deletions(-) diff --git a/DESIGN.md b/DESIGN.md index 1921d51..3c2b1be 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -565,6 +565,12 @@ except on a message whose request may still wait: there it is refused as a changed message, with the alert. The alert still comes from the other taps. +The warning before the end of a standing grant (section 8) has one +**Renew** button. Its data is `n:` and the ID of the request that made +the grant. A Renew gives a right, so it passes every check of an Approve, +also the check of a changed message. A Renew on a warning that is already +the message of the new grant is refused as a late tap. + A notice of a "never ask" use (section 12) has no buttons and no expiry. A notice of a use of a scheduled grant is the same, with the second line `allowed by a scheduled grant, without a tap`. It goes to the notice chat @@ -1566,11 +1572,27 @@ normal way, with a tap. **On the phone.** The message of the grant has a **Revoke** button, as in a scheduled grant. Within `warn_days` of the end, a warning comes with a **Renew** button. The warning is sent `warn_days` before the end (default -7), once per grant. A tap on Renew, after the full checks of section 7, -makes a new grant: a new ID, a new end (the same length as the old grant, -counted from now, and not more than `max_days`), and the same scope. The -next use of the old token gets the token of the new grant, and the old -grant ends with cause `renewed`. Once a week (`summary`, broker local +7), once per grant, to the private chat with the approver, with sound. A +grant that is not longer than `warn_days` gets no warning. A warning that +cannot be sent is logged and not sent again. A tap on Renew, after the +full checks of section 7 (as for an Approve: the approver, the private +chat, the button data, the warning message as the broker sent it, and in +the core the grant of the request ID with this very warning), makes a new +grant: a new ID, a new request ID, a new end (the same length as the old +grant, counted from now, and not more than `max_days`), and the same +scope. The new grant takes the place of the old one: the two count as one +for `max_grants`, and a grant can be renewed once. Its `decision` entry, +with answer `renew` and `renew_of` (the old grant's ID), is on disk +before the new grant exists; if it cannot be written, the Renew does not +count. The warning message then becomes the message of the new grant, with +its Revoke button. The new grant has no token. The next use of the old +token gets the token of the new grant, and the old grant ends with cause +`renewed`. If the old grant reaches its end first, it ends with cause +`renewed` then, and its token passes to the new grant. The old tokens pass +too, so an old token is still an alarm. If the old grant ends in another +way before that (Revoke, an alarm, a cancel, a stop), the new grant ends +with it, with the same cause: it has no token of its own. Until the +handover, the list and `/grants` show the old grant only. Once a week (`summary`, broker local time) a message lists each standing grant: the client, the secrets, the windows, the last use and the end date. The bot command `/grants` from the approver shows the same list. The list never shows a token or a hash. @@ -1663,7 +1685,7 @@ The fields of each event, after `seq`, `time`, `event` and `prev`: | `broker_started` | `version` | | `broker_stopped` | none | | `request` | the request fields; `message`: the exact text the approver saw, if a message was sent | -| `decision` | the request fields; `user_id`: the Telegram user that answered; `answer`: `approve_once`, `approve_window` or `deny`; `window_minutes` for `approve_window` | +| `decision` | the request fields; `user_id`: the Telegram user that answered; `answer`: `approve_once`, `approve_window`, `deny` or `renew`; `window_minutes` for `approve_window`; for `renew`, the fields of the new standing grant (a new `request_id` and `grant_id`, `not_before` the time of the tap) and `renew_of`, the ID of the grant it renews | | `grant_used` | the request fields; `via`: `grant`, `never_rule`, `scheduled` or `standing` (then `grant_id` is in the request fields); for `standing` also `new_token_hash` | | `grant_ended` | the request fields of the `preapprove` request that made the grant, with its `grant_id`; `cause`: `used_up`, `ended`, `cancelled`, `denied`, `revoked`, `stopped`, `alarm` or `renewed`; `uses_left` | | `grant_alarm` | the request fields; `grant_id` (none for an unknown token); `cause`: `old_token`, `over_uses`, `outside_window`, `unknown_token` or `missed_window` | diff --git a/THREAT-MODEL.md b/THREAT-MODEL.md index 5b6dbcd..77a1ea4 100644 --- a/THREAT-MODEL.md +++ b/THREAT-MODEL.md @@ -1061,6 +1061,15 @@ each use. The token is in a file on the client. list of all standing grants on the phone. - A Deny of another request does not end a standing grant, so a Deny cannot be used to break the job. The person ends it with **Revoke**. +- **Renew** gives a right, so a Renew tap has every check of an Approve + (T6): the approver's user ID, the private chat, strict button data, a + warning message that nobody changed, and in the core the grant of the + request ID with that very warning. Its `decision` entry is on disk before + the new grant exists. The new grant has no token: only the next use of + the old grant's token, which the job holds, gets one. So a forged Renew + cannot give a new token to anyone who does not hold the old one. The old + tokens pass to the new grant, so a stolen old token is still an alarm, + and an alarm or Revoke of the old grant also ends a renewal that waits. - Standing grants live in memory and are dropped at a broker stop. The stop notice lists them (T35). From e3b527c36208b03811d510e2fe0f122afae9fa7f Mon Sep 17 00:00:00 2001 From: Geno Roupsky <185580+groupsky@users.noreply.github.com> Date: Thu, 29 Oct 2026 22:11:09 +0200 Subject: [PATCH 11/16] docs: the setup guide and the manual check of standing grants (#110) Co-Authored-By: Claude Opus 5.5 --- DESIGN.md | 11 ++- THREAT-MODEL.md | 12 ++- docs/manual-telegram-test.md | 64 +++++++++++++++ docs/setup.md | 150 ++++++++++++++++++++++++++++++++++- 4 files changed, 229 insertions(+), 8 deletions(-) diff --git a/DESIGN.md b/DESIGN.md index 3c2b1be..aedc9d2 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -996,7 +996,12 @@ Not masked, for example: internet. The next call confirms the updates of the call before, so Telegram forgets them. After a crash, Telegram sends the updates that were not confirmed again; their taps find no waiting request. -- Only `callback_query` updates are read. Text sent to the bot is ignored. +- The broker asks only for `callback_query` and `message` updates. A + `message` is read only for the command `/grants` (section 8): the text + `/grants`, or `/grants@` and a name, with no space inside, from the + approver in the private chat with the approver, at most 3 answers in + any 60 seconds. Every other text sent to the bot is ignored, without an + answer. - If `getUpdates` fails with a conflict (another program polls with the same token, or a webhook was set), the broker logs an alert and sends a message: the bot token may be stolen. Then the Telegram code stops @@ -1598,8 +1603,8 @@ windows, the last use and the end date. The bot command `/grants` from the approver shows the same list. The list never shows a token or a hash. With no standing grant, the weekly message says so, and `/grants` does too. For `/grants` the bot also asks Telegram for `message` updates. Only the -exact text `/grants` from the approver in the private chat with the approver -gets an answer, at most 3 in a minute. A message from anyone or anywhere +text `/grants` (or `/grants@` and a name) from the approver in the +private chat with the approver gets an answer, at most 3 in a minute. A message from anyone or anywhere else is ignored without an answer. **Missed window.** When a window of a live grant ends with no use in it, diff --git a/THREAT-MODEL.md b/THREAT-MODEL.md index 77a1ea4..66fe7b5 100644 --- a/THREAT-MODEL.md +++ b/THREAT-MODEL.md @@ -181,7 +181,17 @@ Risk that remains: none known. **T5. Someone else presses the button.** The broker accepts only the configured Telegram user ID, in the private chat with that user. -Risk that remains: see T7. +The bot also reads text messages, for the command `/grants` only +(DESIGN.md section 8). It answers only the text `/grants` (or `/grants@` +and a name) from the approver, in the private chat with the +approver, at most 3 times in any 60 seconds, and only in that chat. The +answer lists the standing grants: the machine, the secrets, the windows, +the last use and the end. It never has a token or a hash. Any other text, +and any text from another user or chat, is ignored without an answer, so +nobody else learns of the grants, and nobody can make the bot send many +messages. A command can approve, renew or end nothing. +Risk that remains: see T7. Someone who can use the approver's account +can read the list. **T6. The bot token is stolen.** With the token, an attacker can read updates before the broker does, send diff --git a/docs/manual-telegram-test.md b/docs/manual-telegram-test.md index d6dfc14..4078958 100644 --- a/docs/manual-telegram-test.md +++ b/docs/manual-telegram-test.md @@ -577,6 +577,65 @@ Expected: scheduled grant(s) ...`, with the grant ID, `test`, `demo-always` and the uses left. The broker still stops within a few seconds. +### Check 15: Standing grants (optional) + +This checks a standing grant: a use with the token file, the alarm of an +old token, `/grants` and Revoke. In `$T/broker.toml`, add `standing = +true` to the section of `demo-always`, add an empty `[standing]` +section, and start the broker again. Then, terminal 2, with `START` and +`END` as a window that starts in 2 minutes and ends 10 minutes later +(for example `14:05` and `14:15`, in the local time of the broker), and +`UNTIL` as tomorrow (for example `2026-10-30T00:00`): + +```sh +HOME="$T/home" "$T/nodkey" preapprove --secret demo-always \ + --daily START-END --until UNTIL \ + --grant-token "$T/home/token" --reason "check 15" +``` + +1. Tap **Approve**. Then `ls -l "$T/home/token"`. +2. Send `/grants` to the bot, in your private chat with it. +3. Save a copy of the token, and wait until `START`: + `cp -p "$T/home/token" "$T/home/token.old"`. Then use the grant: + + ```sh + HOME="$T/home" "$T/nodkey" run --grant-token "$T/home/token" \ + --secret DEMO_TOKEN=demo-always --reason "check 15, use" -- \ + sh -c 'echo "the command got ${#DEMO_TOKEN} characters"' + echo "exit code: $?" + cmp "$T/home/token" "$T/home/token.old" + ``` + +4. Use the old copy: the same `nodkey run` command, with + `--grant-token "$T/home/token.old"`. +5. Do step 1 again, with `--grant-token "$T/home/token2"`. Tap + **Revoke**. Send `/grants` again. +6. Remove the token files: `rm -f "$T/home/token" "$T/home/token.old" + "$T/home/token2"`. + +Expected: + +- Step 1: the message `๐Ÿ—“ Standing grant ยท request XXXX` has the lines + `โœ… window: daily START-END`, `โœ… uses in each window: 1` and `A Deny of + other requests does not end this grant. Use Revoke to end it.`. After + the tap it has `๐Ÿ‘ approved by you at ...` and one **Revoke** button. + Terminal 2 prints the ID of the grant, and no token. The token file + has mode `-rw-------`. +- Step 2: a message `๐Ÿ“‹ Standing grants ยท 1` comes, with `machine: test`, + `secrets: demo-always`, `windows: daily START-END`, `last use: never` + and the end. It has no token. +- Step 3: no tap. Terminal 2 prints `the command got 15 characters` and + `exit code: 0`. A silent notice comes: `๐Ÿ”” Standing grant ยท request + ...` with `allowed by a standing grant, without a tap`. `cmp` says + that the files differ: the token was changed. +- Step 4: exit code 77, and no tap. An alarm comes, with sound: `๐Ÿšจ + Standing grant alarm ยท request ...` with `an old token was used again; + it may be stolen. The grant is ended.` and the grant ID. The message + of the grant has no button, and a line `๐Ÿšจ ended by an alarm at ...`. +- Step 5: the phone shows `Revoked.`. The message has no button, and a + line `โ›” revoked by you at ...`. `/grants` answers `No standing + grants.`. + ## Clean up 1. Terminal 1: stop the broker with Ctrl-C, if it still runs. @@ -621,3 +680,8 @@ raw answer of the Bot API, and the lines of the log with `user_id` or | 14 | Revoke on a scheduled grant | `Revoked.`; button gone; `revoked by you` | | | | 14 | grant ends with uses left | `ended at ... with 2 of 2 use(s) left`; silent notice `not used` | | | | 14 | stop with a scheduled grant | `dropped: the broker stopped`; one notice that lists the grant | | | +| 15 | a standing grant | Approve; the ID, no token; token file `-rw-------`; one **Revoke** button | | | +| 15 | `/grants` | the list with `test`, `demo-always`, the window; no token; after Revoke `No standing grants.` | | | +| 15 | a use with the token file | no tap; exit 0; silent notice; the token file changed | | | +| 15 | the old token again | exit 77; alarm with sound; `ended by an alarm` | | | +| 15 | Revoke on a standing grant | `Revoked.`; button gone; `revoked by you` | | | diff --git a/docs/setup.md b/docs/setup.md index 2cb05ae..7e02c52 100644 --- a/docs/setup.md +++ b/docs/setup.md @@ -839,6 +839,13 @@ It is off by default, and it is an error with `ask = "never"`. The four limits are optional. Their defaults are in the example above. They are safe, so leave them unless you need more. +**`[standing]` and `standing`.** A secret with `standing = true` lets a +client ask for a standing grant: daily or weekly windows, for weeks +([section 11](#a-job-that-runs-every-night-standing-grants)). It is a +separate flag from `preapprove`, off by default, and an error with +`ask = "never"`. Without a `[standing]` section, no standing grant is +allowed. Its keys and defaults are in section 11. + **`[updates]`.** `check` is optional, `true` by default. Then the broker asks GitHub 10 minutes after its start, and then once a day, whether a newer release of nodkey is out, and sends you a silent notice when one @@ -1377,10 +1384,6 @@ Things to know: - Anything on that machine can use the grant inside its time, not only the job. Keep the span short and the uses few. If something else uses the grant up, the real job fails, and that shows it. -- If you use a token file (`--grant-token`), keep it in a private folder, - such as the home folder of the job user. Do not put it in a shared - folder like `/tmp`: another user there could delete the file or block - its update. - For a key, use `--key`, `--user` and `--host` as for `nodkey ssh-grant`. **Each SSH connection takes one use.** A tool that opens many connections, such as Ansible, needs more uses, or `ControlPersist` in @@ -1399,6 +1402,145 @@ Things to know: same secret or key ends the grant, also before its start; the message of the grant says so. +### A job that runs every night: standing grants + +A scheduled grant covers one time span. A job that runs every night for +weeks would need a new grant, and a tap, each day. A standing grant lets +you approve it once: this client may use these secrets in these daily or +weekly windows, N times in each window, until a date. It is for secrets +only, not for SSH keys. + +Turn it on for the secret, in `broker.toml`. It is off by default, and +it is a separate flag from `preapprove`. The rule must have `ask = +"always"` or `ask = "window"`. The `[standing]` section must be there, +also when it is empty: without it, no standing grant is allowed. These +are its keys and their defaults: + +```toml +[secret.backup-token] +# ... +clients = ["backup"] +ask = "always" +standing = true + +[standing] +max_days = 90 # from now to the end date; at most 366 +max_window = "8h" # one window; at most 8h +max_uses = 1 # uses in each window; at most 10 +max_grants = 3 # standing grants of one client; at most 20 +warn_days = 7 # the warning before the end; less than max_days +summary = "mon 09:00" # the weekly list, in broker local time +``` + +**Run the job as its own system user.** Anything that runs as the same +user can read the token file and use the grant. Give the job its own +user, with its home folder in `/var/lib`, and its own client key and +config ([section 11](#the-key-and-the-config-of-the-client)), with its +own `[client.backup]` section on the broker. Then the agent's user +cannot read the token or the key, if it cannot use `sudo` or become root. On the agent machine, as root: + +```sh +useradd --system --home-dir /var/lib/nightly-backup --shell /usr/sbin/nologin nightly-backup +install -d -m 0700 -o nightly-backup -g nightly-backup /var/lib/nightly-backup +sudo -H -u nightly-backup sh -c 'install -d -m 0700 ~/.config ~/.config/nodkey && + nodkey keygen --out ~/.config/nodkey/client.key' +``` + +Write `~nightly-backup/.config/nodkey/client.toml` as before, owned by +`nightly-backup`, with mode 0600. + +**Ask for the grant** as that user. `--until` is a must. `--uses` is +the uses in each window (default 1). `--from` is optional (default: +now). Give `--daily HH:MM-HH:MM`, or `--weekly DAYS HH:MM-HH:MM` (such as +`--weekly mon,thu 03:00-05:00`), not both. A window is in broker local +time, and does not pass midnight. `--grant-token FILE` is a must: + +```sh +sudo -H -u nightly-backup nodkey preapprove --secret backup-token \ + --daily 03:00-05:00 --until 2026-12-31T00:00 \ + --grant-token /var/lib/nightly-backup/token --reason "job nightly-backup" +``` + +The message shows the windows, the uses in each window and the end, with +**Approve** and **Deny**. After **Approve**, the command writes the token +to the file and prints the ID of the grant. It never prints the token. +The file must be a new file, or a file that the user owns with mode 0600. +The client refuses a token file that is not its own, or that other users +can read or write. Keep it in a private folder, not in `/tmp`. + +**The job.** A systemd service and a timer. The timer starts the job +inside the window: + +```ini +# /etc/systemd/system/nightly-backup.service +[Unit] +Description=Nightly backup + +[Service] +Type=oneshot +User=nightly-backup +Group=nightly-backup +StateDirectory=nightly-backup +StateDirectoryMode=0700 +Environment=HOME=/var/lib/nightly-backup +ExecStart=/usr/local/bin/nodkey run --grant-token /var/lib/nightly-backup/token \ + --secret BACKUP_TOKEN=backup-token --reason "job nightly-backup" -- /usr/local/bin/backup +``` + +```ini +# /etc/systemd/system/nightly-backup.timer +[Unit] +Description=Nightly backup + +[Timer] +OnCalendar=*-*-* 03:30 + +[Install] +WantedBy=timers.target +``` + +```sh +systemctl daemon-reload +systemctl enable --now nightly-backup.timer +``` + +`OnCalendar` is in the local time of the agent machine, and the window +is in the local time of the broker. If the two differ, add the zone of +the broker, such as `OnCalendar=*-*-* 03:30 UTC`. Do not add +`Persistent=true` to the timer: a missed run would start +later, outside the window, and that gives an alarm. Each use needs no +tap and sends a silent notice. `nodkey run` writes a new token to the +file before it runs the command. If it cannot save the file, the command +does not run (exit 77). + +Things to know: + +- **The client never retries a request with a token.** An old token + ends the grant with an alarm, because it looks like a stolen token. So + if a run failed in the middle (for example the network broke after + the broker sent the answer), the next run has an old token. Then make + a new grant: one tap. +- Alarms come with sound. These end the grant: an old token, more uses + in a window than allowed, and the token of another machine's grant. + These only alarm, and the grant stays: a valid token outside the + windows, an unknown token, and a window that ended with no use. +- `warn_days` (default 7) before the end, a warning with a **Renew** + button comes, with sound. Renew makes a new grant of the same length, + from now, with the same secrets and windows. The job keeps its token + file: its next use gets the token of the new grant. A grant can be + renewed once, and a grant that is not longer than `warn_days` gets no + warning. +- The message of the grant has a **Revoke** button: it ends the grant at + once. `nodkey preapprove --cancel ID`, as the job user, ends it too. A + **Deny** of another request does not end a standing grant. +- Send `/grants` to the bot, in your private chat with it: it lists each + standing grant, with the machine, the secrets, the windows, the last + use and the end. Once a week, at `summary`, the same list comes as a + silent message. The list never shows a token. +- A broker restart drops all standing grants, as it drops scheduled + grants. The stop notice lists them. Then make new grants. Keeping + them over a restart is issue #111. + ## 12. Check the audit log The broker writes each request and each decision to the audit log, From a2f9c8032531b4f01b9576d9085f829105a4835c Mon Sep 17 00:00:00 2001 From: Geno Roupsky <185580+groupsky@users.noreply.github.com> Date: Thu, 29 Oct 2026 22:11:37 +0200 Subject: [PATCH 12/16] docs: the standing grant example takes one window flag (#110) Co-Authored-By: Claude Opus 5.5 --- DESIGN.md | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/DESIGN.md b/DESIGN.md index aedc9d2..0cfc045 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -1498,8 +1498,7 @@ A config without `[standing]` allows no standing grant. The message is that of a scheduled grant, with the windows, the uses per window and the end date, and the buttons **Approve** and **Deny**. It says that a Deny of other requests does not end the grant. The reason is the claimed job name. -For `nodkey preapprove --secret backup-token --daily 03:00-05:00 --weekly -mon,thu 10:30-11:00 --until 2026-12-31 --reason "job nightly-backup"` it +For `nodkey preapprove --secret backup-token --daily 03:00-05:00 --until 2026-12-31 --reason "job nightly-backup"` it is: ``` @@ -1507,7 +1506,6 @@ is: โœ… machine: laptop โœ… secrets: backup-token โœ… window: daily 03:00-05:00 -โœ… window: mon,thu 10:30-11:00 โœ… uses in each window: 1 โœ… from: 2026-10-28 03:00:00 UTC โœ… until: 2026-12-31 00:00:00 UTC From a647e484dadad47050479f77fbde9c34ce7108a2 Mon Sep 17 00:00:00 2001 From: Geno Roupsky <185580+groupsky@users.noreply.github.com> Date: Thu, 29 Oct 2026 22:12:29 +0200 Subject: [PATCH 13/16] fix: one missed-window alarm per grant after a clock jump (#110) Co-Authored-By: Claude Opus 5.5 --- DESIGN.md | 3 ++- internal/approval/missed.go | 13 ++++++++----- internal/approval/missed_test.go | 22 ++++++++++++++++++++++ 3 files changed, 32 insertions(+), 6 deletions(-) diff --git a/DESIGN.md b/DESIGN.md index 0cfc045..74cb069 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -1609,7 +1609,8 @@ else is ignored without an answer. the broker sends the alarm `missed_window`. The grant stays. A window that ended before the grant started does not count. A window in the hour that a clock change skips does not open, so it also gives this alarm. Each grant -sends at most one alarm in a window. +sends at most one alarm in a window. If the wall clock jumps ahead over +many windows, the grant sends one alarm for all of them. **End.** A standing grant ends when its end date comes, on Revoke, on an alarm that ends it, on a renewal, on a cancel by its own client (`nodkey diff --git a/internal/approval/missed.go b/internal/approval/missed.go index ee657a0..995eb88 100644 --- a/internal/approval/missed.go +++ b/internal/approval/missed.go @@ -77,9 +77,10 @@ func nextWindowEnd(g *grant, wall time.Time) (time.Duration, bool) { } // scanMissed finds the windows of standing grants that ended since the -// last scan with no use in them, and queues one alarm for each. The grant -// stays. Each opening is looked at once, because scanned only moves -// forward. Core.mu must be held. +// last scan with no use in them, and queues an alarm for them: at most +// one for each grant in one scan, so that a long jump of the clock does +// not queue many. The grant stays. Each opening is looked at once, +// because scanned only moves forward. Core.mu must be held. func (c *Core) scanMissed(now moment) { for _, g := range c.grants { if !g.standing() || g.pendingRenewal() || !now.wall.After(g.scanned) { @@ -87,14 +88,16 @@ func (c *Core) scanMissed(now moment) { } from := g.scanned g.scanned = now.wall + queued := false eachWindowEnd(g.windows, from, now.wall, func(k windowKey, end time.Time) { if g.used[k] { delete(g.used, k) return } - if !end.Before(g.end.wall) { - return // the grant ended before this window did + if queued || !end.Before(g.end.wall) { + return // one alarm in this scan, or the grant ended before this window did } + queued = true sub := g.origin sub.Secrets = slices.Clone(g.origin.Secrets) sub.Hosts = slices.Clone(g.origin.Hosts) diff --git a/internal/approval/missed_test.go b/internal/approval/missed_test.go index d276662..3986ba3 100644 --- a/internal/approval/missed_test.go +++ b/internal/approval/missed_test.go @@ -186,3 +186,25 @@ func TestNoMissedAlarmAfterTheGrantEnds(t *testing.T) { } }) } + +// TestOneMissedAlarmAfterAWallClockJump: a wall clock that jumps days +// ahead queues one alarm for the grant, not one for each skipped window. +func TestOneMissedAlarmAfterAWallClockJump(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + e, _, entries := missedEnv(t) + defer e.stop() + e.clock.JumpWall(72 * time.Hour) + e.pass(17 * time.Hour) // the timer wakes; three or more windows passed with no use + if n := len(e.fake.Alarms()); n != 1 { + t.Errorf("%d alarms after the jump, want 1", n) + } + if n := len(entries()); n != 1 { + t.Errorf("%d entries after the jump, want 1", n) + } + // The next window still gives its own. + e.pass(24 * time.Hour) + if n := len(e.fake.Alarms()); n != 2 { + t.Errorf("%d alarms after the next window, want 2", n) + } + }) +} From f2c9e4f4a3fd42a1a3692c244f0ee47f52c7e3c7 Mon Sep 17 00:00:00 2001 From: Geno Roupsky <185580+groupsky@users.noreply.github.com> Date: Thu, 29 Oct 2026 22:14:04 +0200 Subject: [PATCH 14/16] test: a Renew of a standing grant through the broker (#110) Co-Authored-By: Claude Opus 5.5 --- internal/broker/standingrenew_test.go | 130 ++++++++++++++++++++++++++ 1 file changed, 130 insertions(+) create mode 100644 internal/broker/standingrenew_test.go diff --git a/internal/broker/standingrenew_test.go b/internal/broker/standingrenew_test.go new file mode 100644 index 0000000..2e7d23d --- /dev/null +++ b/internal/broker/standingrenew_test.go @@ -0,0 +1,130 @@ +package broker + +import ( + "errors" + "net/http" + "testing" + "time" + + "github.com/code6x/nodkey/internal/approval" + "github.com/code6x/nodkey/internal/approval/approvaltest" + "github.com/code6x/nodkey/internal/audit" +) + +// warned moves the clock to the day after the warning of the one standing +// grant of useEnv (it ends at 2026-11-14, warn_days is 7; the clock then +// reads 12:00, inside the daily window) and waits for the warning. +func (e *env) warned() approvaltest.Warned { + e.t.Helper() + e.clock.Advance(24 * 24 * time.Hour) + deadline := time.Now().Add(waitLimit) + for { + if w := e.approver.Warnings(); len(w) > 0 { + return w[0] + } + if time.Now().After(deadline) { + e.t.Fatal("no warning before the end of the standing grant") + } + time.Sleep(time.Millisecond) + } +} + +// renewTap presses Renew on the warning w and returns what the core said. +func (e *env) renewTap(w approvaltest.Warned) error { + return e.approver.Answer(approval.Answer{RequestID: w.Warning.RequestID, Action: audit.AnswerRenew, + Message: w.Message, UserID: approverID}) +} + +// TestStandingRenew: a Renew tap goes through the broker's OnRenewed hook. +// Its decision entry is written first, the next use of the old token gets +// a token of the new grant, and the old grant ends as renewed. +func TestStandingRenew(t *testing.T) { + e, tok0, oldID := useEnv(t) + w := e.warned() + if w.Warning.GrantID != oldID { + t.Fatalf("the warning is for grant %q, want %q", w.Warning.GrantID, oldID) + } + if err := e.renewTap(w); err != nil { + t.Fatalf("the Renew tap: %v", err) + } + // A missed window may add an alarm entry; look for the Renew entry. + var at int + var d audit.Decision + for at = 0; ; at++ { + events := e.audit.waitFor(t, at+1) + if x, ok := events[at].(audit.Decision); ok && x.Answer == audit.AnswerRenew { + d = x + break + } + } + if d.RenewOf != oldID || d.Kind != audit.KindStanding || d.GrantID == "" || d.GrantID == oldID { + t.Fatalf("the Renew entry is %+v, renew_of %q", d.Subject, d.RenewOf) + } + for _, ev := range e.audit.all()[:at] { + if g, ok := ev.(audit.GrantEnded); ok && g.GrantID == oldID { + t.Fatalf("the old grant ended before the Renew entry: %s", g.Cause) + } + } + newID, before := d.GrantID, at+1 + res, ans := e.useToken("laptop", tok0) + mustStatus(t, res, http.StatusOK) + tok1 := ans.GrantToken + if tok1 == "" || tok1 == tok0 { + t.Fatal("the use did not return a new token") + } + var ended, used = -1, -1 + all := e.audit.all() + for i, ev := range all[before:] { + switch x := ev.(type) { + case audit.GrantEnded: + if x.GrantID == oldID && x.Cause == audit.EndRenewed { + ended = before + i + } + case audit.GrantUsed: + if x.GrantID == newID && x.NewTokenHash == hashHex(tok1) { + used = before + i + } + } + } + if ended < 0 || used < 0 { + t.Fatalf("after the use: grant_ended:renewed at %d, grant_used of the new grant at %d: %s", + ended, used, eventNames(all)) + } + // The new token works for the new grant. + res, _ = e.useToken("laptop", tok1) + mustStatus(t, res, http.StatusOK) + // The old token is not valid any more. + res, _ = e.useToken("laptop", tok0) + mustStatus(t, res, http.StatusForbidden) + e.noToken(tok0, tok1) +} + +// TestStandingRenewAfterRevoke: a Renew tap after the old grant was +// revoked is refused, and no new grant or entry comes of it. +func TestStandingRenewAfterRevoke(t *testing.T) { + e, tok0, oldID := useEnv(t) + w := e.warned() + e.tap(w.Warning.RequestID, audit.AnswerRevoke) + for at := 0; ; at++ { + if g, ok := e.audit.waitFor(t, at+1)[at].(audit.GrantEnded); ok && g.GrantID == oldID && g.Cause == audit.EndRevoked { + break + } + } + before := len(e.audit.all()) + err := e.renewTap(w) + if !errors.Is(err, approval.ErrUnknownRequest) { + t.Errorf("the Renew tap after the revoke: %v, want ErrUnknownRequest", err) + } + for _, ev := range e.audit.all() { + if d, ok := ev.(audit.Decision); ok && d.Answer == audit.AnswerRenew { + t.Errorf("a Renew entry was written: %+v", d.Subject) + } + } + res, _ := e.useToken("laptop", tok0) + mustStatus(t, res, http.StatusForbidden) + for _, ev := range e.audit.all()[before:] { + if _, ok := ev.(audit.GrantUsed); ok { + t.Error("the token of a revoked grant was used") + } + } +} From 7e381ce7083657bec230d41060e3a9a8563fe7b2 Mon Sep 17 00:00:00 2001 From: Geno Roupsky <185580+groupsky@users.noreply.github.com> Date: Thu, 29 Oct 2026 22:18:38 +0200 Subject: [PATCH 15/16] fix: a renewal that waits for its handover does not spin the timer (#110) Also the test of the standing message in DESIGN.md now has one window. Co-Authored-By: Claude Opus 5.5 --- internal/approval/grants.go | 2 +- internal/approval/renew_test.go | 19 +++++++++++++++++++ internal/telegram/preapprove_text_test.go | 5 ++--- 3 files changed, 22 insertions(+), 4 deletions(-) diff --git a/internal/approval/grants.go b/internal/approval/grants.go index b664be3..a12d3cd 100644 --- a/internal/approval/grants.go +++ b/internal/approval/grants.go @@ -524,7 +524,7 @@ func (c *Core) nextEnd() (time.Time, bool) { continue } take(g.end.mono) - if g.standing() && !g.warned && !g.warnAt.IsZero() { + if g.standing() && !g.warned && !g.warnAt.IsZero() && !g.pendingRenewal() && now.before(g.end) { take(now.mono.Add(max(g.warnAt.Sub(now.wall), 0))) } if g.standing() && !g.pendingRenewal() && now.before(g.end) { diff --git a/internal/approval/renew_test.go b/internal/approval/renew_test.go index 5ac07f9..e7b9665 100644 --- a/internal/approval/renew_test.go +++ b/internal/approval/renew_test.go @@ -408,3 +408,22 @@ func TestRenewKeepsToMaxDaysAndMaxGrants(t *testing.T) { } }) } + +// TestRenewalWaitingPastItsWarningDoesNotSpin: a renewal that still waits +// for its handover when its own warning time comes must not wake the +// timer again and again. With a loop, synctest.Wait in pass never returns. +func TestRenewalWaitingPastItsWarningDoesNotSpin(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + e, _, _, _, _ := renewEnv(t, func(c *config.Broker) { c.Standing.WarnDays = 20 }) + defer e.stop() + e.pass(10*day + time.Hour) // the warning of the 30-day grant came + if err := e.renewTap(e.warning()); err != nil { + t.Fatalf("Renew: %v", err) + } + synctest.Wait() + e.pass(11 * day) // past the warning time of the renewal, before the old end + if n := len(e.fake.Warnings()); n != 1 { + t.Errorf("%d warnings, want 1", n) + } + }) +} diff --git a/internal/telegram/preapprove_text_test.go b/internal/telegram/preapprove_text_test.go index a3f6380..64b34d5 100644 --- a/internal/telegram/preapprove_text_test.go +++ b/internal/telegram/preapprove_text_test.go @@ -157,13 +157,12 @@ func TestScheduledNotice(t *testing.T) { // designStanding is the example of a standing grant in DESIGN.md section // 8, as the person sees it. It is the message of // -// nodkey preapprove --secret backup-token --daily 03:00-05:00 --weekly mon,thu 10:30-11:00 \ +// nodkey preapprove --secret backup-token --daily 03:00-05:00 \ // --until 2026-12-31 --uses 1 --reason "job nightly-backup" const designStanding = `๐Ÿ—“ Standing grant ยท request m4k2 โœ… machine: laptop โœ… secrets: backup-token โœ… window: daily 03:00-05:00 -โœ… window: mon,thu 10:30-11:00 โœ… uses in each window: 1 โœ… from: 2026-10-28 03:00:00 UTC โœ… until: 2026-12-31 00:00:00 UTC @@ -176,7 +175,7 @@ expires 14:07:00 (120 s) func standingPrompt() approval.Prompt { p := preapprovePrompt() p.Kind = audit.KindStanding - p.Windows = []string{"daily 03:00-05:00", "mon,thu 10:30-11:00"} + p.Windows = []string{"daily 03:00-05:00"} p.Until = time.Date(2026, 12, 31, 0, 0, 0, 0, time.UTC) return p } From 953b905d06e62cad5ccf84bd84f5459077780dba Mon Sep 17 00:00:00 2001 From: Geno Roupsky <185580+groupsky@users.noreply.github.com> Date: Thu, 29 Oct 2026 22:21:27 +0200 Subject: [PATCH 16/16] test: wait for the end entry of a renewed grant (#110) Co-Authored-By: Claude Opus 5.5 --- internal/broker/standingrenew_test.go | 37 +++++++++++++++++++-------- 1 file changed, 26 insertions(+), 11 deletions(-) diff --git a/internal/broker/standingrenew_test.go b/internal/broker/standingrenew_test.go index 2e7d23d..8e831e6 100644 --- a/internal/broker/standingrenew_test.go +++ b/internal/broker/standingrenew_test.go @@ -72,18 +72,14 @@ func TestStandingRenew(t *testing.T) { if tok1 == "" || tok1 == tok0 { t.Fatal("the use did not return a new token") } + // The end entry of the old grant is written by the core's own + // goroutine, so it may come a little after the answer. var ended, used = -1, -1 - all := e.audit.all() - for i, ev := range all[before:] { - switch x := ev.(type) { - case audit.GrantEnded: - if x.GrantID == oldID && x.Cause == audit.EndRenewed { - ended = before + i - } - case audit.GrantUsed: - if x.GrantID == newID && x.NewTokenHash == hashHex(tok1) { - used = before + i - } + var all []audit.Event + for wait := time.Now().Add(waitLimit); ; time.Sleep(10 * time.Millisecond) { + ended, used, all = renewedEntries(e.audit.all(), before, oldID, newID, hashHex(tok1)) + if (ended >= 0 && used >= 0) || time.Now().After(wait) { + break } } if ended < 0 || used < 0 { @@ -128,3 +124,22 @@ func TestStandingRenewAfterRevoke(t *testing.T) { } } } + +// renewedEntries finds, from index before on, the end entry of the old +// grant with cause renewed and the use entry of the new grant. +func renewedEntries(all []audit.Event, before int, oldID, newID, newHash string) (ended, used int, _ []audit.Event) { + ended, used = -1, -1 + for i, ev := range all[before:] { + switch x := ev.(type) { + case audit.GrantEnded: + if x.GrantID == oldID && x.Cause == audit.EndRenewed { + ended = before + i + } + case audit.GrantUsed: + if x.GrantID == newID && x.NewTokenHash == newHash { + used = before + i + } + } + } + return ended, used, all +}