From 97e9126e1f10ccef73bd1b44fde89be17985d1f4 Mon Sep 17 00:00:00 2001 From: Geno Roupsky <185580+groupsky@users.noreply.github.com> Date: Thu, 29 Oct 2026 07:05:58 +0200 Subject: [PATCH 01/18] chore: start standing grants (#110) Co-Authored-By: Claude Opus 5.5 From 7bde95f3d3bf9c807b2bcc76314bbb2e409cd2b3 Mon Sep 17 00:00:00 2001 From: Geno Roupsky <185580+groupsky@users.noreply.github.com> Date: Thu, 29 Oct 2026 07:07:08 +0200 Subject: [PATCH 02/18] docs: standing grants and the grant core (#110) Co-Authored-By: Claude Opus 5.5 --- DESIGN.md | 164 ++++++++++++++++++++++++++++++++++++++++++++++-- THREAT-MODEL.md | 38 +++++++++++ 2 files changed, 197 insertions(+), 5 deletions(-) diff --git a/DESIGN.md b/DESIGN.md index 1ab9ab8..19480b1 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -1146,6 +1146,7 @@ Each secret and each key has a rule: A rule may also have `preapprove = true`, for scheduled grants (below). It is off by default, and the config check refuses it with `ask = "never"`. +A rule may have `standing = true` too, for standing grants (below). Any other `ask` value, also an empty one, denies the request. So does a `never` rule for a key without `hosts`, and a `never` rule that does not @@ -1223,6 +1224,25 @@ name exactly one client. The config check refuses all of these (section minute. A notice about the version of a client (section 7) counts toward neither of these limits. +### The grant core + +All grants share one record and one end path. A grant has a **kind**: + +| kind | what it is | audit kind | +|---|---|---| +| window | "Approve N min" (above): from a tap on a request, for minutes | none | +| scheduled | a pre-approval, one time span, N uses (below) | `preapprove` | +| standing | a pre-approval with daily or weekly windows, for weeks (below) | `standing` | + +A later fourth kind is planned in issue 103, step 9: a one-use +pre-approval of a login. It would use the same record. + +The record has the client, the scope, the end, the uses left, and, for a +standing grant, a list of windows and the uses in the current window. A +grant is **live** before its end and, if it has windows, inside one of +them. Every end goes through one path, with one cause, one `grant_ended` +entry and one edit of the message. + ### Scheduled grants A job that runs later, such as a timer at night, cannot tap a button. A @@ -1427,6 +1447,111 @@ the real job fails, and that is a visible sign. The reason is claimed: the person cannot check that "job nightly-backup" exists. A tap means "this client may use this secret in this time", nothing more. +### Standing grants + +A scheduled grant covers one time span. A job that runs every night for +weeks would need a tap each day. A standing grant lets the person approve +it once: **this client may use these secrets in these daily or weekly +windows, N times in each window, until this date.** It needs a tap, and it +is bound to a token that changes at each use. Secrets only; SSH keys are +planned for later. + +**The request.** The same command and the same endpoint as a scheduled +grant, with the audit kind `standing`. `nodkey preapprove` takes one of +`--daily 03:00-05:00` or `--weekly mon,thu 03:00-05:00` (section 15), for +at most 7 windows. With one of them, `--until` is a must, `--uses` means +the uses in each window (default 1), and `--from` is optional (default: +now). A window does not pass midnight and is not longer than `max_window`. +Windows are in broker local time, with the summer-time rules of `hours` +(section 12): a window that opens twice on the day the clocks go back +still has one count, and a window in the skipped hour does not open. To +avoid this, run the broker in UTC. + +**The rule.** A rule may have `standing = true`, off by default, only for +`always` and `window` rules. It is a flag apart from `preapprove`, because +a standing grant is a much wider right: one flag for both would widen every +config that has `preapprove = true`. The same checks as for a scheduled +grant run before any message (known secrets, allowed for the client, count +of grants), with the limits of `[standing]` (section 14): + +| key | default | ceiling | +|---|---|---| +| `max_days` (from now to the end) | 90 | 366 | +| `max_window` (one window) | 8 h | 8 h | +| `max_uses` (per window) | 1 | 10 | +| `max_grants` (per client) | 3 | 20 | +| `warn_days` (warning before the end) | 7 | none | +| `summary` (weekly list, local time) | `"mon 09:00"` | none | + +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**. + +**The token.** After the tap, the broker makes a token: `nkg_`, 32 random +bytes in base62, and a CRC32 checksum, as GitHub does. The broker keeps +only its SHA-256 hash. The token is in the answer to `nodkey preapprove` +once, and nowhere else. The client writes it to the file of `--grant-token +FILE` and never prints it. It works only with the TLS key of its client. +The client refuses a token file that is not its own, or that other users +can read or write (mode 0600 at most). + +**Use and rotation.** `nodkey run --grant-token FILE` sends the token in +the header `Nodkey-Grant-Token`. The broker, in this order: + +1. Checks the hash and the client, that the grant covers the whole + request, that now is inside a window, and that a use is left in it. +2. Makes a new token and writes `grant_used` (`via` = `standing`, with the + hash of the new token) to disk. +3. Sends the silent notice, which counts toward the limits of 6 per client + and 20 in all (as in a scheduled grant). +4. Answers with the secrets and the new token. + +The client writes the new token to a temp file, syncs it, and renames it +over the old file. Only then does it use the secrets. If the write +fails, it does not run the command. It never retries a request that had a +token by itself. For each use, the broker keeps one 32-byte hash until the +grant ends, to know the old tokens. + +**Alarms.** An alarm is a Telegram message with sound, and a `grant_alarm` +entry with a cause. It counts toward the limit of 20 notices in all. One +grant sends at most one alarm in a window. + +| case | request | grant | cause | +|---|---|---|---| +| an old token | denied | **ends** (`alarm`) | `old_token` | +| more uses in a window than allowed | denied | **ends** (`alarm`) | `over_uses` | +| a valid token outside the windows | denied | stays | `outside_window` | +| an unknown token, or the token of another client | denied | none | `unknown_token` (rate limited) | +| a window ended with no use | none | stays | `missed_window` | + +An old token is taken as stolen. A retry after a lost answer looks the +same, and the broker cannot tell them apart. There is no pause and no +Resume: a new grant is one tap. A request without a token goes the +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. + +**End.** A standing grant ends when its end date comes, on Revoke, on an +alarm that ends it, on a renewal, or at a broker stop. A **Deny** of +another request of the same client and secret does **not** end it: an +"Approve once" Deny at noon must not break the night job. The Deny message +says "standing grants stay; use Revoke to end them". At a stop the grants +are dropped, written as `stopped`, and listed in the stop notice. Keeping +them is issue 111. + +**The honest limit.** The token finds a theft; it does not stop it. The +same user on the client can read and rewrite the token file, so a thief +can use the grant first. The real job then finds an old token and fails, +and the alarm comes. Run the job as its own system user (see +`docs/setup.md`). See T36 in THREAT-MODEL.md. + ## 9. Audit log - One file, JSON Lines: one entry per line. The broker opens it once, at @@ -1449,7 +1574,7 @@ the person cannot check that "job nightly-backup" exists. A tap means - A line is at most 64 KiB. The broker refuses a longer entry, so the request is denied. - Events: `broker_started`, `request`, `decision`, `grant_used`, - `grant_ended`, `secret_sent`, `signed`, `denied`, `foreign_tap`, `error`, + `grant_ended`, `grant_alarm`, `secret_sent`, `signed`, `denied`, `foreign_tap`, `error`, `log_recovered`, `broker_stopped`. Each event about a request holds the request ID, the client, the names, the verified host if any, and the claimed command and reason (masked as in section 7). @@ -1489,8 +1614,9 @@ The fields of each event, after `seq`, `time`, `event` and `prev`: | `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` | -| `grant_used` | the request fields; `via`: `grant`, `never_rule` or `scheduled` (then `grant_id` is in the request fields) | -| `grant_ended` | the request fields of the `preapprove` request that made the grant, with its `grant_id`; `cause`: `used_up`, `ended`, `cancelled`, `denied`, `revoked` or `stopped`; `uses_left` | +| `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` | | `secret_sent`, `signed` | the request fields | | `denied` | the request fields; `cause`: `approver`, `expired`, `cancelled`, `not_allowed`, `limit` or `error` | | `foreign_tap` | `user_id`, `chat_id`, `message_id`, `data`: the button data, at most 64 bytes, and `cause`: why it did not count (section 7). An entry with cause `edited_message` can belong to a Deny that counted: its `data` is `d:` and the request ID, and that request has a `decision` entry with `deny` | @@ -1855,7 +1981,7 @@ a path, a query, `OPTIONS *` or `PRI *` gets 404. The types are in | `/v1/secrets` | `run` | `secrets` (each with `var` and `name`), `command`, `dir`, `reason` | `secrets`: each with `var` and `value`, in the order of the request | | `/v1/sign` | `ssh-agent` | `public_key`, `data`, `flags`, `binds`, `commands`, `reason` | `format` and `blob` of the signature | | `/v1/ssh-grant` | `ssh-grant` | `key`, `user`, `hosts`, `seconds` (`--for`), `reason` | `minutes`: how long the grants last | -| `/v1/preapprove` | `preapprove` | `secrets`, `key`, `user`, `hosts`, `from`, `until` (texts, read by the broker), `uses`, `reason` | `id` of the grant, `from` and `until` in RFC 3339, in the broker's local time | +| `/v1/preapprove` | `preapprove` | `secrets`, `key`, `user`, `hosts`, `from`, `until` (texts, read by the broker), `uses`, `reason`; for a standing grant also `windows` (list of day names and `HH:MM-HH:MM`) | `id` of the grant, `from` and `until` in RFC 3339, in the broker's local time; for a standing grant also `grant_token` | | `/v1/preapprove/cancel` | `preapprove --cancel` | `id` | `id`; 403 for an ID that the client does not have | - Each field must be there, none may be `null`, and no other field may be @@ -1901,6 +2027,9 @@ of different versions can fail with only "the answer of the broker is not valid", or a `400`. So each side tells the other its version, and says when they differ. +- `/v1/secrets` may have the header `Nodkey-Grant-Token` with the token of a + standing grant (section 8). The answer then also has `grant_token`, the + new token. Without the header, the request goes the normal way. - Each request has the header `Nodkey-Version` with the client's version, as `nodkey version` prints it, such as `0.2.0` or `dev`. Each answer of the broker has it with the broker's version: with every status, also @@ -2116,6 +2245,9 @@ listed hosts with that key) without a tap. To keep it small: own project, read through its own store entry with its own machine account (see the two `bws` stores in section 14). +A standing grant (section 8) is the better form of this decision: it needs a +tap, it ends by itself, and its token finds a use by something else. + **Not chosen:** matching the command name ("only for `backup.sh`"). The command is claimed. The agent can send any command line it wants. @@ -2259,6 +2391,14 @@ 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 (section 8) +max_days = 90 # from now to the end date; ceiling 366 +max_window = "8h" # one window; ceiling 8h +max_uses = 1 # per window; ceiling 10 +max_grants = 3 # per client; ceiling 20 +warn_days = 7 # warning before the end +summary = "mon 09:00" # weekly list, broker local time + [client.laptop] pin = "sha256:Yf3c...base64..." # printed by `nodkey keygen` @@ -2284,6 +2424,7 @@ clients = ["laptop"] ask = "window" window = "15m" preapprove = true # optional; default false: `nodkey preapprove` may ask for it +standing = true # optional; default false: standing grants (only `always`, `window`) [key.deploy] file = "${CREDENTIALS_DIRECTORY}/ssh-deploy" @@ -2359,6 +2500,11 @@ More rules: "never"`. In `[preapprove]`, `max_ahead` is 1 minute to 31 days, `max_open` is 1 minute to 24 hours, `max_uses` is 1 to 1000 and `max_grants` is 1 to 100. Each has the default of section 8. +- `standing` is `true` or `false`, and is not allowed with `ask = + "never"`. In `[standing]`, `max_days` is 1 to 366, `max_window` is 1 + minute to 8 hours, `max_uses` is 1 to 10, `max_grants` is 1 to 20, + `warn_days` is 1 or more, and `summary` is a day name and `HH:MM`. + Each has the default of section 8. - `max_per_day`, if present, is 1 or more. For no limit, leave it out. So `0` cannot mean "no uses" to a person and "no limit" to the broker. - `check` in `[updates]` is `true` or `false`. Without it, and without @@ -2462,10 +2608,11 @@ other users must not be able to change it. ``` nodkey broker [--config FILE] -nodkey run [--secret VAR=NAME]... [--reason TEXT] -- COMMAND [ARGS...] +nodkey run [--secret VAR=NAME]... [--grant-token FILE] [--reason TEXT] -- COMMAND [ARGS...] nodkey ssh-agent [--socket PATH] nodkey ssh-grant --key NAME --user USER --host NAME... --for DURATION [--reason TEXT] nodkey preapprove (--secret NAME... | --key NAME --user USER --host NAME...) --from TIME --until TIME --uses N [--reason TEXT] +nodkey preapprove --secret NAME... (--daily HH:MM-HH:MM | --weekly DAYS HH:MM-HH:MM) --until TIME [--from TIME] [--uses N] --grant-token FILE [--reason TEXT] nodkey preapprove --cancel ID nodkey keygen --out FILE nodkey audit verify FILE [--anchor SEQ:HASH]... [--anchors FILE|-] @@ -2683,6 +2830,13 @@ on stdout. Its last line on stderr is always Before it, it may print one warning about the version of the broker (section 10). +`nodkey preapprove` with `--daily` or `--weekly` asks for a standing grant +(section 8). `--weekly` takes day names (`mon,thu`) and one window. +`--grant-token FILE` is a must; the client writes the token there after the +tap (mode 0600, new file or its own file) and prints no token. `nodkey run +--grant-token FILE` uses it and rotates it, as section 8 says; if the +file cannot be saved, the command does not run (exit 77). + `nodkey preapprove` asks for a scheduled grant (section 8) and waits for the tap, as `nodkey ssh-grant` does. The times are read as section 8 says; the client only checks their form. After an approval it prints the diff --git a/THREAT-MODEL.md b/THREAT-MODEL.md index 0400935..2190883 100644 --- a/THREAT-MODEL.md +++ b/THREAT-MODEL.md @@ -1026,6 +1026,44 @@ use up to N uses of what it names. After a use, the secret stays on the agent machine (section 5, item 1). Choose short spans, few uses and narrow secrets, as for a "never ask" rule. +**T36. A standing grant is used by something else on the same client.** +A standing grant (DESIGN.md section 8) lets a client use named secrets in +daily or weekly windows for weeks, without a tap. The broker knows the +client only by its TLS key, so the grant also has a token that changes at +each use. The token is in a file on the client. +- **The honest limit.** The token finds a theft; it does not stop it. The + same user can read the token file, and can rewrite it. A thief who + copies the token and uses it first gets the secrets once. The real job + then comes with the old token, and the broker ends the grant and sends + an alarm. A thief who reads the file and writes back the new token each + time can use the grant like the job. Then the one-use-per-window rule + (`over_uses`) and the missed-window alarm are what catch it. +- The broker keeps only the SHA-256 hash of each token. A token works only + with the TLS key of its client, so a copy on another machine gets an + alarm and no secret. +- Old token, or more uses than allowed: the request is denied, the grant + ends and an alarm comes. A retry after a lost answer looks the same, so + the client saves the new token before it uses the secrets and never + retries by itself. A new grant is one tap, so there is no pause state. +- A valid token outside the windows is denied with an alarm, and the grant + stays. An unknown token, or the token of another client, is denied with + a rate-limited alarm; it may be a guess. Alarms have sound, count toward + the 20 notices in all, and one grant sends at most one in a window. +- Run the job as its own system user, so an agent that runs as another + user cannot read the token file. The client refuses a token file that + others can read or write, or that it does not own. Root can still read + it. +- A standing grant is wider than a scheduled grant, so it has its own rule + flag (`standing = true`), its own limits (`[standing]`), and a weekly + 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**. +- Standing grants live in memory and are dropped at a broker stop. The + stop notice lists them (T35). + +Risk that remains: the same as T35, for weeks. Choose few uses, short +windows, narrow secrets and a near end date. + ## 7. Assumptions - The broker host is patched, has no agent on it, and only the owner has From b6ccff00eb498d23769d0885b7fa0c174ef048c2 Mon Sep 17 00:00:00 2001 From: Geno Roupsky <185580+groupsky@users.noreply.github.com> Date: Thu, 29 Oct 2026 07:08:25 +0200 Subject: [PATCH 03/18] feat(config): the standing rule flag and the [standing] section (#110) Co-Authored-By: Claude Opus 5.5 --- internal/config/broker.go | 152 +++++++++++++++++++++++++++++++++ internal/config/broker_test.go | 107 +++++++++++++++++++++++ internal/config/rule.go | 18 +++- 3 files changed, 273 insertions(+), 4 deletions(-) diff --git a/internal/config/broker.go b/internal/config/broker.go index 6905f9f..a92785f 100644 --- a/internal/config/broker.go +++ b/internal/config/broker.go @@ -50,6 +50,7 @@ type Broker struct { Telegram Telegram Updates Updates Preapprove PreapproveLimits + Standing StandingLimits Clients map[string]Client // by name Stores map[string]Store // by name Secrets map[string]Secret // by name @@ -109,6 +110,58 @@ func DefaultPreapprove() PreapproveLimits { } } +// StandingLimits is the [standing] section: the limits of standing grants +// (DESIGN.md section 8). A config without the section allows no standing +// grant: Enabled is false and the other fields are zero. A section, also +// an empty one, sets Enabled and fills each missing key with its default. +// Code that makes a Broker by hand must set them: the zero value allows +// no standing grant at all. +type StandingLimits struct { + // Enabled is true when the config has a [standing] section. + Enabled bool + // MaxDays is the most days from now to the end of a grant. + MaxDays int + // MaxWindow is the longest single window. + MaxWindow time.Duration + // MaxUses is the most uses in one window. + MaxUses int + // MaxGrants is the most standing grants of one client. + MaxGrants int + // WarnDays is how many days before the end the warning comes. It is + // 1 or more and less than MaxDays. + WarnDays int + // SummaryDay and SummaryMinute are the time of the weekly list, in the + // broker's local time. SummaryMinute is the minute of the day, 0 to + // 1439. + SummaryDay time.Weekday + SummaryMinute int +} + +// The defaults and the ceilings of the [standing] section. +const ( + defaultStandingDays = 90 + defaultStandingWindow = 8 * time.Hour + defaultStandingUses = 1 + defaultStandingGrants = 3 + defaultStandingWarnDays = 7 + defaultStandingSummary = "mon 09:00" + maxStandingDays = 366 + maxStandingWindow = 8 * time.Hour + maxStandingUses = 10 + maxStandingGrants = 20 +) + +// DefaultStanding returns the limits that apply with an empty [standing] +// section. They have Enabled set. +func DefaultStanding() StandingLimits { + day, minute, _ := parseSummary(defaultStandingSummary) + return StandingLimits{ + Enabled: true, MaxDays: defaultStandingDays, MaxWindow: defaultStandingWindow, + MaxUses: defaultStandingUses, MaxGrants: defaultStandingGrants, WarnDays: defaultStandingWarnDays, + SummaryDay: day, SummaryMinute: minute, + } +} + // Client is a [client.NAME] section: a machine that may ask. type Client struct { Name string @@ -165,6 +218,7 @@ type ( Telegram telegramSection `toml:"telegram"` Updates updatesSection `toml:"updates"` Preapprove preapproveSection `toml:"preapprove"` + Standing *standingSection `toml:"standing"` Client map[string]clientSection `toml:"client"` Store map[string]storeSection `toml:"store"` Secret map[string]secretSection `toml:"secret"` @@ -186,6 +240,14 @@ type ( MaxUses *int `toml:"max_uses"` MaxGrants *int `toml:"max_grants"` } + standingSection struct { + MaxDays *int `toml:"max_days"` + MaxWindow *string `toml:"max_window"` + MaxUses *int `toml:"max_uses"` + MaxGrants *int `toml:"max_grants"` + WarnDays *int `toml:"warn_days"` + Summary *string `toml:"summary"` + } clientSection struct { Pin string `toml:"pin"` } @@ -275,6 +337,9 @@ func newBroker(f *brokerFile) (*Broker, error) { if b.Preapprove, err = parsePreapprove(&f.Preapprove); err != nil { return nil, err } + if b.Standing, err = parseStanding(f.Standing); err != nil { + return nil, err + } if err := b.addClients(f.Client); err != nil { return nil, err } @@ -429,6 +494,93 @@ func parsePreapprove(s *preapproveSection) (PreapproveLimits, error) { return l, nil } +// parseStanding checks the [standing] section. A nil section means no +// standing grant is allowed. +func parseStanding(s *standingSection) (StandingLimits, error) { + if s == nil { + return StandingLimits{}, nil + } + const sec = "[standing]" + l := DefaultStanding() + count := func(key string, v *int, most int, into *int) error { + if v == nil { + return nil + } + if *v < 1 || *v > most { + return fmt.Errorf("%s: must be from 1 to %d", place(sec, key), most) + } + *into = *v + return nil + } + if err := count("max_days", s.MaxDays, maxStandingDays, &l.MaxDays); err != nil { + return StandingLimits{}, err + } + if s.MaxWindow != nil { + d, err := parseDuration(sec, "max_window", *s.MaxWindow) + if err != nil { + return StandingLimits{}, err + } + if d < time.Minute || d > maxStandingWindow { + return StandingLimits{}, fmt.Errorf("%s: must be from %s to %s", place(sec, "max_window"), + formatDuration(time.Minute), formatDuration(maxStandingWindow)) + } + l.MaxWindow = d + } + if err := count("max_uses", s.MaxUses, maxStandingUses, &l.MaxUses); err != nil { + return StandingLimits{}, err + } + if err := count("max_grants", s.MaxGrants, maxStandingGrants, &l.MaxGrants); err != nil { + return StandingLimits{}, err + } + if s.WarnDays != nil { + if *s.WarnDays < 1 { + return StandingLimits{}, fmt.Errorf("%s: must be 1 or more", place(sec, "warn_days")) + } + l.WarnDays = *s.WarnDays + } + if l.WarnDays >= l.MaxDays { + return StandingLimits{}, fmt.Errorf("%s: must be less than max_days (%d)", place(sec, "warn_days"), l.MaxDays) + } + if s.Summary != nil { + day, minute, err := parseSummary(*s.Summary) + if err != nil { + return StandingLimits{}, fmt.Errorf("%s: must be a day name and a time, such as \"mon 09:00\"", place(sec, "summary")) + } + l.SummaryDay, l.SummaryMinute = day, minute + } + return l, nil +} + +var weekdays = map[string]time.Weekday{ + "sun": time.Sunday, "mon": time.Monday, "tue": time.Tuesday, "wed": time.Wednesday, + "thu": time.Thursday, "fri": time.Friday, "sat": time.Saturday, +} + +// parseSummary reads "mon 09:00": a lower-case three-letter day name, one +// space, and HH:MM from 00:00 to 23:59. Nothing else is accepted. +func parseSummary(s string) (time.Weekday, int, error) { + bad := errors.New("bad summary") + name, clock, ok := strings.Cut(s, " ") + if !ok || len(clock) != 5 || clock[2] != ':' { + return 0, 0, bad + } + day, ok := weekdays[name] + if !ok { + return 0, 0, bad + } + for _, i := range []int{0, 1, 3, 4} { + if clock[i] < '0' || clock[i] > '9' { + return 0, 0, bad + } + } + h := int(clock[0]-'0')*10 + int(clock[1]-'0') + m := int(clock[3]-'0')*10 + int(clock[4]-'0') + if h > 23 || m > 59 { + return 0, 0, bad + } + return day, h*60 + m, nil +} + // parseTimeout parses an optional timeout from minTimeout to most. func parseTimeout(section, key string, s *string, def, most time.Duration) (time.Duration, error) { if s == nil { diff --git a/internal/config/broker_test.go b/internal/config/broker_test.go index bbacbf6..b18ac2a 100644 --- a/internal/config/broker_test.go +++ b/internal/config/broker_test.go @@ -1459,3 +1459,110 @@ func TestLoadBrokerPreapproveLimits(t *testing.T) { } } } + +// TestLoadBrokerStandingFlag: standing is off by default, it is allowed +// for "always" and "window" rules, and it is refused on a "never" rule. +func TestLoadBrokerStandingFlag(t *testing.T) { + e := newBrokerEnv(t) + b, err := LoadBroker(e.write(t)) + if err != nil { + t.Fatal(err) + } + for name, s := range b.Secrets { + if s.Rule.Standing { + t.Errorf("secret %s: Standing is on by default", name) + } + } + for name, k := range b.Keys { + if k.Rule.Standing { + t.Errorf("key %s: Standing is on by default", name) + } + } + b, err = LoadBroker(e.write(t, + edit{"ask = \"window\"\nwindow = \"15m\"\n", "ask = \"window\"\nwindow = \"15m\"\nstanding = true\n"})) + if err != nil { + t.Fatal(err) + } + r := b.Secrets["github-token"].Rule + if !r.Standing || r.Preapprove { + t.Errorf("window rule: Standing = %v, Preapprove = %v, want true and false", r.Standing, r.Preapprove) + } + b, err = LoadBroker(e.write(t, + edit{"ask = \"window\"\nwindow = \"15m\"\n", "ask = \"window\"\nwindow = \"15m\"\nstanding = false\npreapprove = true\n"})) + if err != nil { + t.Fatal(err) + } + if r := b.Secrets["github-token"].Rule; r.Standing || !r.Preapprove { + t.Errorf("standing = false: Standing = %v, Preapprove = %v", r.Standing, r.Preapprove) + } + _, err = LoadBroker(e.write(t, + edit{"ask = \"never\"", "ask = \"never\"\nstanding = true"})) + if err == nil || !strings.Contains(err.Error(), `is not allowed with ask = "never"`) { + t.Errorf("standing on a never rule: error %v, want it refused", err) + } + if _, err := LoadBroker(e.write(t, + edit{"ask = \"window\"\nwindow = \"15m\"\n", "ask = \"window\"\nwindow = \"15m\"\nstanding = \"yes\"\n"})); err == nil { + t.Error("standing = \"yes\" was accepted") + } +} + +// TestLoadBrokerStandingLimits: without the section no standing grant is +// allowed, an empty section has the defaults, each key is read, and a +// value out of its bounds is refused. +func TestLoadBrokerStandingLimits(t *testing.T) { + e := newBrokerEnv(t) + b, err := LoadBroker(e.write(t)) + if err != nil { + t.Fatal(err) + } + if b.Standing != (StandingLimits{}) || b.Standing.Enabled { + t.Errorf("no section: %+v, want the zero value", b.Standing) + } + with := func(body string) edit { return edit{"[client.laptop]", "[standing]\n" + body + "\n\n[client.laptop]"} } + b, err = LoadBroker(e.write(t, with(""))) + if err != nil { + t.Fatal(err) + } + want := StandingLimits{ + Enabled: true, MaxDays: 90, MaxWindow: 8 * time.Hour, MaxUses: 1, MaxGrants: 3, WarnDays: 7, + SummaryDay: time.Monday, SummaryMinute: 9 * 60, + } + if b.Standing != want || DefaultStanding() != want { + t.Errorf("defaults = %+v, want %+v", b.Standing, want) + } + b, err = LoadBroker(e.write(t, with( + "max_days = 366\nmax_window = \"90m\"\nmax_uses = 10\nmax_grants = 20\nwarn_days = 365\nsummary = \"sun 23:59\""))) + if err != nil { + t.Fatal(err) + } + want = StandingLimits{ + Enabled: true, MaxDays: 366, MaxWindow: 90 * time.Minute, MaxUses: 10, MaxGrants: 20, WarnDays: 365, + SummaryDay: time.Sunday, SummaryMinute: 23*60 + 59, + } + if b.Standing != want { + t.Errorf("limits = %+v, want %+v", b.Standing, want) + } + for _, line := range []string{ + "max_days = 0", "max_days = -1", "max_days = 367", "max_days = \"90\"", + "max_window = \"0s\"", "max_window = \"30s\"", "max_window = \"8h1m\"", "max_window = \"-1h\"", "max_window = \"long\"", + "max_uses = 0", "max_uses = -1", "max_uses = 11", + "max_grants = 0", "max_grants = -1", "max_grants = 21", + "warn_days = 0", "warn_days = -1", "warn_days = 90", "warn_days = 91", + "max_days = 7", "max_days = 5\nwarn_days = 5", + "summary = \"\"", "summary = \"mon\"", "summary = \"09:00\"", "summary = \"Mon 09:00\"", "summary = \"monday 09:00\"", + "summary = \"mon 9:00\"", "summary = \"mon 24:00\"", "summary = \"mon 09:60\"", "summary = \"mon 09:00\"", + "summary = \"mon 09:00 \"", "summary = \"mon 0a:00\"", "summary = \"mon 09-00\"", "summary = \"xyz 09:00\"", + "summary = 9", "extra = 1", + } { + if _, err := LoadBroker(e.write(t, with(line))); err == nil { + t.Errorf("%q: the config was accepted", line) + } + } + // A small max_days is fine when warn_days is below it. + if _, err := LoadBroker(e.write(t, with("max_days = 8"))); err != nil { + t.Errorf("max_days = 8: %v", err) + } + if _, err := LoadBroker(e.write(t, with("max_days = 3\nwarn_days = 2"))); err != nil { + t.Errorf("max_days = 3, warn_days = 2: %v", err) + } +} diff --git a/internal/config/rule.go b/internal/config/rule.go index 24c954f..3e39640 100644 --- a/internal/config/rule.go +++ b/internal/config/rule.go @@ -53,6 +53,12 @@ type Rule struct { // nothing. Preapprove bool + // Standing lets the client ask for a standing grant for this secret + // or key (DESIGN.md section 8). The default is false. It is a flag + // apart from Preapprove, because a standing grant is a much wider + // right. It is never set with AskNever. + Standing bool + // The rest is only for AskNever. // Notify sends a silent notice for each use. The default is true. @@ -71,6 +77,7 @@ type Rule struct { type ruleKeys struct { Ask string `toml:"ask"` Preapprove *bool `toml:"preapprove"` + Standing *bool `toml:"standing"` Window *string `toml:"window"` Notify *bool `toml:"notify"` MaxPerDay *int `toml:"max_per_day"` @@ -83,8 +90,8 @@ type ruleKeys struct { func parseRule(section string, k ruleKeys) (Rule, error) { // The keys each kind of rule allows besides ask. allowed := map[Ask]map[string]bool{ - AskAlways: {"preapprove": true}, - AskWindow: {"window": true, "preapprove": true}, + AskAlways: {"preapprove": true, "standing": true}, + AskWindow: {"window": true, "preapprove": true, "standing": true}, AskNever: {"notify": true, "max_per_day": true, "hours": true}, } ask := Ask(k.Ask) @@ -101,6 +108,7 @@ func parseRule(section string, k ruleKeys) (Rule, error) { }{ {"window", k.Window != nil}, {"preapprove", k.Preapprove != nil}, + {"standing", k.Standing != nil}, {"notify", k.Notify != nil}, {"max_per_day", k.MaxPerDay != nil}, {"hours", k.Hours != nil}, @@ -113,7 +121,8 @@ func parseRule(section string, k ruleKeys) (Rule, error) { switch ask { case AskAlways: - return Rule{Ask: AskAlways, Preapprove: k.Preapprove != nil && *k.Preapprove}, nil + return Rule{Ask: AskAlways, Preapprove: k.Preapprove != nil && *k.Preapprove, + Standing: k.Standing != nil && *k.Standing}, nil case AskWindow: if k.Window == nil { return Rule{}, fmt.Errorf(`%s: is missing; ask = "window" needs it`, place(section, "window")) @@ -126,7 +135,8 @@ func parseRule(section string, k ruleKeys) (Rule, error) { return Rule{}, fmt.Errorf("%s: must be at least %s and at most %s", place(section, "window"), formatDuration(MinWindow), formatDuration(MaxWindow)) } - return Rule{Ask: AskWindow, Window: w, Preapprove: k.Preapprove != nil && *k.Preapprove}, nil + return Rule{Ask: AskWindow, Window: w, Preapprove: k.Preapprove != nil && *k.Preapprove, + Standing: k.Standing != nil && *k.Standing}, nil case AskNever: r := Rule{Ask: AskNever, Notify: true} if k.Notify != nil { From 4f2c170313dd3adf86473f4f42f0e5b4872b2136 Mon Sep 17 00:00:00 2001 From: Geno Roupsky <185580+groupsky@users.noreply.github.com> Date: Thu, 29 Oct 2026 07:10:46 +0200 Subject: [PATCH 04/18] feat(approval): grant kinds and windows for standing grants (#110) Co-Authored-By: Claude Opus 5.5 --- internal/approval/check.go | 2 +- internal/approval/decide.go | 2 +- internal/approval/grants.go | 127 +++++++++++-- internal/approval/state.go | 5 +- internal/approval/window.go | 95 ++++++++++ internal/approval/window_test.go | 303 +++++++++++++++++++++++++++++++ 6 files changed, 515 insertions(+), 19 deletions(-) create mode 100644 internal/approval/window.go create mode 100644 internal/approval/window_test.go diff --git a/internal/approval/check.go b/internal/approval/check.go index d73bc85..a9363e6 100644 --- a/internal/approval/check.go +++ b/internal/approval/check.go @@ -281,7 +281,7 @@ func (c *Core) checkPreapprove(r *Request, p *pending) bool { p.subject.NotBefore = r.NotBefore.UTC().Format(time.RFC3339) p.subject.NotAfter = r.NotAfter.UTC().Format(time.RFC3339) p.plan = &grant{ - client: p.client, refs: refs, hostKeys: hostKeys, user: p.user, + kind: kindScheduled, client: p.client, refs: refs, hostKeys: hostKeys, user: p.user, start: r.NotBefore, end: moment{wall: r.NotAfter}, uses: r.Uses, } return true diff --git a/internal/approval/decide.go b/internal/approval/decide.go index c7df242..0b1cd8e 100644 --- a/internal/approval/decide.go +++ b/internal/approval/decide.go @@ -667,7 +667,7 @@ func (c *Core) uncount(p *pending) { } p.counted = nil if g := p.used; g != nil && slices.Contains(c.grants, g) { - g.uses++ // the use did not happen + g.giveBackUse() // the use did not happen } p.used = nil } diff --git a/internal/approval/grants.go b/internal/approval/grants.go index ec670bd..c06658a 100644 --- a/internal/approval/grants.go +++ b/internal/approval/grants.go @@ -20,8 +20,10 @@ const noLimit = -1 // host keys and for one remote user, from its start until its end, for a // number of uses. type grant struct { - // id is the ID of a scheduled grant. It is "" for the other grants, - // which have no entry in the audit log of their own. + // kind says what the grant is. The zero value is a window grant. + kind grantKind + // id is the ID of a scheduled or standing grant. It is "" for a window + // grant, which has no entry in the audit log of its own. id string client string // refs are the secrets of the grant, or the one key. @@ -54,8 +56,32 @@ type grant struct { prompt Prompt // approved is when the person approved it. approved time.Time + + // windows are the daily or weekly windows of a standing grant. A grant + // without windows is live all the time before its end. + windows []window + // perWindow is the number of uses in each opening of a window of a + // standing grant. + perWindow int + // winKey is the latest opening of a window that had a use, and winUsed + // the uses in it. A later opening starts the count again. + winKey windowKey + winUsed int } +// grantKind is the kind of a grant (DESIGN.md section 8, "The grant core"). +type grantKind int + +const ( + // kindWindow: "Approve N min" or nodkey ssh-grant. It must be the zero + // value. + kindWindow grantKind = iota + // kindScheduled: a pre-approval with one time span. + kindScheduled + // kindStanding: a pre-approval with daily or weekly windows. + kindStanding +) + // msgUpdate is a call to the approver about the message of a scheduled // grant: End, and for a grant that ended with uses left also Notify. The // calls wait in Core.msgQueue, so they run one after the other, in the @@ -81,15 +107,66 @@ const ( holdFirstWait = 10 * time.Second ) -// scheduled reports whether g comes from nodkey preapprove. -func (g *grant) scheduled() bool { return g.id != "" } +// scheduled reports whether g is a scheduled grant. +func (g *grant) scheduled() bool { return g.kind == kindScheduled } + +// standing reports whether g is a standing grant. +func (g *grant) standing() bool { return g.kind == kindStanding } + +// audited reports whether g comes from nodkey preapprove, of any kind: it +// has an ID, entries in the audit log and a message. +func (g *grant) audited() bool { return g.kind != kindWindow } // live reports whether g may be used at now: not before its start on the // wall clock, and before its end on both clocks (moment.before), so a // grant is never usable at or after its end. A wall clock that is set back // cannot make it last longer, because the monotonic end stays. +// +// A grant with windows is live only inside one of them, read on the wall +// clock (window.go). func (g *grant) live(now moment) bool { - return !now.wall.Before(g.start) && now.before(g.end) + if now.wall.Before(g.start) || !now.before(g.end) { + return false + } + if len(g.windows) == 0 { + return true + } + _, _, in := windowAt(g.windows, now.wall) + return in +} + +// tryUse takes one use of the standing grant g at now, if it may be used. +// It tells apart a grant that is outside its windows and one whose uses in +// this window are spent, for the alarms. A standing grant without a valid +// window cannot be used. +func (g *grant) tryUse(now moment) useResult { + if !g.standing() || len(g.windows) == 0 || now.wall.Before(g.start) || !now.before(g.end) { + return useNone + } + key, _, in := windowAt(g.windows, now.wall) + if !in { + return useOutsideWindow + } + // A later opening starts the count again. An earlier or equal one keeps + // it, so a wall clock set back cannot give more uses. + if key.after(g.winKey) { + g.winKey, g.winUsed = key, 0 + } + if g.winUsed >= g.perWindow { + return useOverUses + } + g.winUsed++ + return useOK +} + +// giveBackUse takes back the use that tryUse took, because the use did +// not happen. +func (g *grant) giveBackUse() { + if g.standing() { + g.winUsed = max(g.winUsed-1, 0) + return + } + g.uses++ } // covers reports whether g covers the item r of a request of client with @@ -119,7 +196,7 @@ func (c *Core) endWhere(cause func(*grant) audit.EndCause) []audit.GrantEnded { if why == "" { return false } - if g.scheduled() { + if g.audited() { ended = append(ended, audit.GrantEnded{Subject: g.origin, Cause: why, UsesLeft: max(g.uses, 0)}) c.queueMessage(g, why) } @@ -311,7 +388,8 @@ func (c *Core) deliverEnded() { // watchGrants ends the scheduled grants when their time is up, and hands // their entries out, until ctx ends or the core stops. A timer follows the // monotonic clock only; a wall clock that moves forward is found by the -// next request, which sweeps first (begin). +// next request, which sweeps first (begin). It also wakes at the end of +// the current window of each standing grant. func (c *Core) watchGrants(ctx context.Context) { for { c.mu.Lock() @@ -341,14 +419,31 @@ func (c *Core) watchGrants(ctx context.Context) { } } -// nextEnd returns the earliest end of a scheduled grant, on the monotonic -// clock. Core.mu must be held. +// 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 +// is early or late, and the one who reads it looks again. Core.mu must be +// held. func (c *Core) nextEnd() (time.Time, bool) { var next time.Time found := false + take := func(t time.Time) { + if !found || t.Before(next) { + next, found = t, true + } + } + now := c.now() for _, g := range c.grants { - if g.scheduled() && (!found || g.end.mono.Before(next)) { - next, found = g.end.mono, true + if !g.audited() { + continue + } + take(g.end.mono) + if g.standing() && now.before(g.end) { + if _, left, in := windowAt(g.windows, now.wall); in { + take(now.mono.Add(left)) + } } } return next, found @@ -361,7 +456,7 @@ func (c *Core) nextEnd() (time.Time, bool) { func (c *Core) dropScheduled() { c.mu.Lock() c.queueEnded(c.endWhere(func(g *grant) audit.EndCause { - if g.scheduled() { + if g.audited() { return audit.EndStopped } return "" @@ -389,7 +484,7 @@ func (c *Core) FlushMessages() { func (c *Core) revoke(a Answer) error { now := c.now() c.sweepGrants(now) - i := slices.IndexFunc(c.grants, func(g *grant) bool { return g.scheduled() && g.reqID == a.RequestID }) + i := slices.IndexFunc(c.grants, func(g *grant) bool { return g.audited() && g.reqID == a.RequestID }) if i < 0 { return ErrUnknownRequest } @@ -431,7 +526,7 @@ func (c *Core) CancelGrant(client, id string) (audit.GrantEnded, bool) { // ssh-grant with the scope k is valid now. func (c *Core) granted(k grantKey, now moment) bool { return slices.ContainsFunc(c.grants, func(g *grant) bool { - return !g.scheduled() && g.covers(k.client, k.ref, k.hostKey, k.user) && g.live(now) + return g.kind == kindWindow && g.covers(k.client, k.ref, k.hostKey, k.user) && g.live(now) }) } @@ -440,14 +535,14 @@ func (c *Core) granted(k grantKey, now moment) bool { // the later end. Core.mu must be held. func (c *Core) addWindowGrant(k grantKey, end moment) { for _, g := range c.grants { - if !g.scheduled() && g.covers(k.client, k.ref, k.hostKey, k.user) && len(g.refs) == 1 { + if g.kind == kindWindow && g.covers(k.client, k.ref, k.hostKey, k.user) && len(g.refs) == 1 { if g.end.mono.Before(end.mono) { g.end = end } return } } - g := &grant{client: k.client, refs: []ref{k.ref}, user: k.user, end: end, uses: noLimit} + g := &grant{kind: kindWindow, client: k.client, refs: []ref{k.ref}, user: k.user, end: end, uses: noLimit} if k.hostKey != "" { g.hostKeys = []string{k.hostKey} } diff --git a/internal/approval/state.go b/internal/approval/state.go index fb0f58a..44685d4 100644 --- a/internal/approval/state.go +++ b/internal/approval/state.go @@ -272,7 +272,10 @@ func (c *Core) deny(cs *clientState, p *pending, res *Result, now moment, seq ui // A scheduled grant ends too, also one that has not started yet; its // entry follows the entries of res. res.ended = append(res.ended, c.endWhere(func(g *grant) audit.EndCause { - if g.client == p.client && g.touches(p.items) { + // A standing grant stays: it lasts for weeks, and a Deny at noon + // must not break the night job. Only Revoke, an alarm, a renewal + // and its end date end it (DESIGN.md section 8). + if g.client == p.client && !g.standing() && g.touches(p.items) { return audit.EndDenied } return "" diff --git a/internal/approval/window.go b/internal/approval/window.go new file mode 100644 index 0000000..76df431 --- /dev/null +++ b/internal/approval/window.go @@ -0,0 +1,95 @@ +package approval + +import "time" + +// anyDay is the set of weekdays of a daily window: all seven. +const anyDay weekdays = 1<<7 - 1 + +// weekdays is a set of weekdays. Bit n is time.Weekday(n). +type weekdays uint8 + +// has reports whether d is in the set. +func (s weekdays) has(d time.Weekday) bool { + return d >= time.Sunday && d <= time.Saturday && s&(1<= 0 && w.from < w.to && w.to <= 24*time.Hour +} + +// sinceMidnight returns the time of day that t shows on its own clock. +func sinceMidnight(t time.Time) time.Duration { + hh, mm, ss := t.Clock() + return time.Duration(hh)*time.Hour + time.Duration(mm)*time.Minute + + time.Duration(ss)*time.Second + time.Duration(t.Nanosecond()) +} + +// holds reports whether t is inside w, read on t's own clock. +func (w window) holds(t time.Time) bool { + if !w.valid() || !w.days.has(t.Weekday()) { + return false + } + d := sinceMidnight(t) + return w.from <= d && d < w.to +} + +// windowKey names one opening of a window: the calendar date on the wall +// clock and the index of the window in the list of the grant. It is not an +// instant, so a window that opens twice on the day the clocks go back has +// one key, and one count of uses. +type windowKey struct { + day day + index int +} + +// after reports whether k is a later opening than o. +func (k windowKey) after(o windowKey) bool { + if k.day != o.day { + return k.day.after(o.day) + } + return k.index > o.index +} + +// windowAt returns the first window of ws that holds t, with its key and +// the time left until the window ends, counted on the clock of t. ok is +// false if no window holds t. +func windowAt(ws []window, t time.Time) (key windowKey, left time.Duration, ok bool) { + for i, w := range ws { + if w.holds(t) { + return windowKey{day: dayOf(t), index: i}, w.to - sinceMidnight(t), true + } + } + return windowKey{}, 0, false +} + +// useResult is what a try to use a grant gives. The broker turns the last +// two into alarms (DESIGN.md section 8). +type useResult int + +const ( + // useOK: the use is taken. + useOK useResult = iota + // useNone: the grant cannot be used, for a reason that is no alarm: + // it has not started or has ended, or it has no uses left in total. + useNone + // useOverUses: the grant is live and inside a window, but the uses + // of this window are spent (cause over_uses). + useOverUses + // useOutsideWindow: the grant is live, but now is in none of its + // windows (cause outside_window). + useOutsideWindow +) diff --git a/internal/approval/window_test.go b/internal/approval/window_test.go new file mode 100644 index 0000000..ff8be23 --- /dev/null +++ b/internal/approval/window_test.go @@ -0,0 +1,303 @@ +package approval + +import ( + "testing" + "time" + + "github.com/code6x/nodkey/internal/audit" +) + +func mustZone(t *testing.T, name string) *time.Location { + t.Helper() + loc, err := time.LoadLocation(name) + if err != nil { + t.Skipf("no zone data for %s: %v", name, err) + } + return loc +} + +func hm(h, m int) time.Duration { return time.Duration(h)*time.Hour + time.Duration(m)*time.Minute } + +func TestWindowHolds(t *testing.T) { + utc := time.UTC + // 2026-10-26 is a Monday. + daily := window{days: anyDay, from: hm(3, 0), to: hm(5, 0)} + monThu := window{days: 1< Date: Thu, 29 Oct 2026 07:11:56 +0200 Subject: [PATCH 05/18] feat(grantkey): the standing grant token (#110) Co-Authored-By: Claude Opus 5.5 --- AGENTS.md | 2 + internal/grantkey/grantkey.go | 122 ++++++++++++++++++++++++++ internal/grantkey/grantkey_test.go | 135 +++++++++++++++++++++++++++++ 3 files changed, 259 insertions(+) create mode 100644 internal/grantkey/grantkey.go create mode 100644 internal/grantkey/grantkey_test.go diff --git a/AGENTS.md b/AGENTS.md index f1e2cc3..b193238 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -28,6 +28,8 @@ Planned. Each v1 issue adds its part. - `internal/version/` - versions of nodkey: the rule of a version that the other side sends, release versions, and how two compare (DESIGN.md section 10). + - `internal/grantkey/` - the token of a standing grant: make, parse, + hash, and hide it in every format (DESIGN.md section 8). - `internal/release/` - the newest release of nodkey on GitHub: the lookup for `nodkey version`, and the broker's check for a newer release, with its notice (DESIGN.md section 15). Its tests use a fake diff --git a/internal/grantkey/grantkey.go b/internal/grantkey/grantkey.go new file mode 100644 index 0000000..47efdf9 --- /dev/null +++ b/internal/grantkey/grantkey.go @@ -0,0 +1,122 @@ +// Package grantkey is the token of a standing grant (DESIGN.md section 8). +// +// A token is "nkg_", 43 base62 characters (32 random bytes) and 6 base62 +// characters of the CRC32 (IEEE) of the part before. All tokens have the +// same length. The broker keeps only the SHA-256 hash of a token. +package grantkey + +import ( + "crypto/rand" + "crypto/sha256" + "crypto/subtle" + "errors" + "fmt" + "hash/crc32" + "math/big" +) + +const ( + prefix = "nkg_" + alphabet = "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz" + randBytes = 32 + randChars = 43 // 62^43 > 2^256 + sumChars = 6 // 62^6 > 2^32 + + // Length is the length of every token. + Length = len(prefix) + randChars + sumChars +) + +var ( + // ErrFormat is returned by Parse for anything that is not a token. + // It never holds the input. + ErrFormat = errors.New("grantkey: not a grant token") + bigBase = big.NewInt(62) +) + +// Token is a grant token. It hides its value in every format. +type Token struct { + v string +} + +// String returns a redacted form. +func (Token) String() string { return "nkg_…" } + +// GoString returns a redacted form. +func (Token) GoString() string { return "grantkey.Token{nkg_…}" } + +// Format prints a redacted form for every verb. +func (Token) Format(f fmt.State, _ rune) { _, _ = f.Write([]byte("nkg_…")) } + +// Reveal returns the secret value. Use it only to write the token to its +// file or to a header. +func (t Token) Reveal() string { return t.v } + +// New makes a new random token. +func New() (Token, error) { + b := make([]byte, randBytes) + if _, err := rand.Read(b); err != nil { + return Token{}, errors.New("grantkey: no random bytes") + } + body := prefix + encode(new(big.Int).SetBytes(b), randChars) + return Token{v: body + checksum(body)}, nil +} + +// Parse checks the prefix, the length, the alphabet and the checksum. +// Its errors never hold the input. +func Parse(s string) (Token, error) { + if len(s) != Length || s[:len(prefix)] != prefix { + return Token{}, ErrFormat + } + for i := len(prefix); i < len(s); i++ { + if indexOf(s[i]) < 0 { + return Token{}, ErrFormat + } + } + body := s[:len(prefix)+randChars] + if subtle.ConstantTimeCompare([]byte(s[len(body):]), []byte(checksum(body))) != 1 { + return Token{}, ErrFormat + } + // The random part must fit in 32 bytes. + n := new(big.Int) + for i := len(prefix); i < len(body); i++ { + n.Mul(n, bigBase) + n.Add(n, big.NewInt(int64(indexOf(body[i])))) + } + if n.BitLen() > randBytes*8 { + return Token{}, ErrFormat + } + return Token{v: s}, nil +} + +// Hash returns the SHA-256 hash of the full token string. +func Hash(t Token) [32]byte { return sha256.Sum256([]byte(t.v)) } + +// Equal compares two hashes in constant time. +func Equal(a, b [32]byte) bool { return subtle.ConstantTimeCompare(a[:], b[:]) == 1 } + +func checksum(body string) string { + return encode(new(big.Int).SetUint64(uint64(crc32.ChecksumIEEE([]byte(body)))), sumChars) +} + +func indexOf(c byte) int { + switch { + case c >= '0' && c <= '9': + return int(c - '0') + case c >= 'A' && c <= 'Z': + return int(c-'A') + 10 + case c >= 'a' && c <= 'z': + return int(c-'a') + 36 + } + return -1 +} + +// encode writes n in base62, padded with '0' to width characters. +func encode(n *big.Int, width int) string { + out := make([]byte, width) + for i := width - 1; i >= 0; i-- { + var m big.Int + n.DivMod(n, bigBase, &m) + out[i] = alphabet[m.Int64()] + } + return string(out) +} diff --git a/internal/grantkey/grantkey_test.go b/internal/grantkey/grantkey_test.go new file mode 100644 index 0000000..3a4aa2c --- /dev/null +++ b/internal/grantkey/grantkey_test.go @@ -0,0 +1,135 @@ +package grantkey + +import ( + "fmt" + "strings" + "testing" +) + +func mustNew(t testing.TB) Token { + t.Helper() + tok, err := New() + if err != nil { + t.Fatal(err) + } + return tok +} + +func TestRoundTrip(t *testing.T) { + for i := 0; i < 200; i++ { + tok := mustNew(t) + if len(tok.Reveal()) != Length { + t.Fatalf("length %d", len(tok.Reveal())) + } + got, err := Parse(tok.Reveal()) + if err != nil || got.Reveal() != tok.Reveal() { + t.Fatalf("round trip failed: %v", err) + } + if !Equal(Hash(got), Hash(tok)) { + t.Fatal("hash differs") + } + } +} + +func TestUnique(t *testing.T) { + seen := map[string]bool{} + for i := 0; i < 5000; i++ { + v := mustNew(t).Reveal() + if seen[v] { + t.Fatal("duplicate token") + } + seen[v] = true + } +} + +func TestHashDiffers(t *testing.T) { + if Equal(Hash(mustNew(t)), Hash(mustNew(t))) { + t.Fatal("equal hashes") + } +} + +func TestParseRejects(t *testing.T) { + good := mustNew(t).Reveal() + flip := func(s string, i int) string { + c := byte('0') + if s[i] == '0' { + c = '1' + } + return s[:i] + string(c) + s[i+1:] + } + cases := map[string]string{ + "empty": "", + "prefix": "nkx_" + good[4:], + "no prefix": good[4:], + "short": good[:Length-1], + "long": good + "0", + "bad char": good[:10] + "-" + good[11:], + "underscore": good[:10] + "_" + good[11:], + "unicode": good[:10] + "é" + good[12:], + "checksum": flip(good, Length-1), + "body changed": flip(good, 10), + "space": good + " ", + "newline": good + "\n", + "leading ws": " " + good[:Length-1], + "inner ws": good[:10] + " " + good[11:], + "nul": good[:10] + "\x00" + good[11:], + } + for name, in := range cases { + _, err := Parse(in) + if err == nil { + t.Errorf("%s: accepted", name) + continue + } + if strings.Contains(err.Error(), in) && in != "" { + t.Errorf("%s: error holds input", name) + } + } +} + +func TestParseRejectsOverflow(t *testing.T) { + // 43 'z' is above 2^256, with a valid checksum. + body := prefix + strings.Repeat("z", randChars) + if _, err := Parse(body + checksum(body)); err == nil { + t.Fatal("accepted a value above 32 bytes") + } +} + +func TestNeverPrinted(t *testing.T) { + tok := mustNew(t) + v := tok.Reveal() + body := v[len(prefix):] + for _, f := range []string{"%v", "%+v", "%#v", "%s", "%q", "%x", "%d", "%T"} { + for _, arg := range []any{tok, &tok, []Token{tok}, map[string]Token{"a": tok}, struct{ T Token }{tok}} { + out := fmt.Sprintf(f, arg) + if strings.Contains(out, v) || strings.Contains(out, body[:12]) { + t.Errorf("%s leaks: %s", f, out) + } + } + } + if fmt.Sprint(tok) != "nkg_…" || tok.String() != "nkg_…" { + t.Error("unexpected redaction") + } +} + +func FuzzParse(f *testing.F) { + f.Add(mustNew(f).Reveal()) + f.Add("") + f.Add("nkg_") + f.Add(prefix + strings.Repeat("z", randChars+sumChars)) + f.Fuzz(func(t *testing.T, s string) { + tok, err := Parse(s) + if err != nil { + if len(s) >= 8 && strings.Contains(err.Error(), s) { + t.Fatal("error holds input") + } + return + } + if tok.Reveal() != s || len(s) != Length { + t.Fatal("accepted but not equal") + } + body := s[:len(prefix)+randChars] + if body+checksum(body) != s { + t.Fatal("re-encode differs") + } + }) +} From 817b8f75ddb1bb74393fdf2f6e5ffd008f870546 Mon Sep 17 00:00:00 2001 From: Geno Roupsky <185580+groupsky@users.noreply.github.com> Date: Thu, 29 Oct 2026 07:18:08 +0200 Subject: [PATCH 06/18] feat: standing grant requests in nodkey preapprove (#110) Co-Authored-By: Claude Opus 5.5 --- internal/api/api.go | 7 + internal/api/api_test.go | 130 +++++- internal/api/decode.go | 13 +- internal/api/types.go | 181 +++++++- internal/client/preapprove.go | 162 +++++++- internal/client/preapprove_standing_test.go | 433 ++++++++++++++++++++ internal/client/tokenfile.go | 121 ++++++ internal/client/tokenfile_other.go | 11 + internal/client/tokenfile_unix.go | 18 + 9 files changed, 1062 insertions(+), 14 deletions(-) create mode 100644 internal/client/preapprove_standing_test.go create mode 100644 internal/client/tokenfile.go create mode 100644 internal/client/tokenfile_other.go create mode 100644 internal/client/tokenfile_unix.go diff --git a/internal/api/api.go b/internal/api/api.go index 94fd345..0da8441 100644 --- a/internal/api/api.go +++ b/internal/api/api.go @@ -70,6 +70,11 @@ const ShortCodeHeader = "Nodkey-Short-Code" // decides nothing with it. Read it with Version. const VersionHeader = "Nodkey-Version" +// GrantTokenHeader holds the token of a standing grant (DESIGN.md +// sections 8 and 10). nodkey run sends it with the request for secrets. The +// value is a secret: never print or log it. +const GrantTokenHeader = "Nodkey-Grant-Token" //nolint:gosec // G101: a header name, not a credential. + // Version returns the version in the header VersionHeader of a request or // an answer. It returns "" when there is no such header, more than one, // or a value that version.Valid refuses. Such a value counts as no @@ -123,6 +128,8 @@ const ( // MaxTime is the most bytes of a time in a PreapproveRequest. Valid // times are shorter. MaxTime = 64 + // MaxWindows is the most windows of a standing grant. + MaxWindows = 7 // MaxKeys is the most keys that one client may use, so that a // KeysAnswer stays small. The broker refuses to start with more. MaxKeys = 256 diff --git a/internal/api/api_test.go b/internal/api/api_test.go index 24c0508..8be8a21 100644 --- a/internal/api/api_test.go +++ b/internal/api/api_test.go @@ -29,7 +29,7 @@ var allTypes = []any{ SecretsRequest{}, SecretRef{}, SecretsAnswer{}, SecretValue{}, SignRequest{}, SignAnswer{}, GrantRequest{}, GrantAnswer{}, - PreapproveRequest{}, PreapproveAnswer{}, PreapproveCancelRequest{}, PreapproveCancelAnswer{}, + PreapproveRequest{}, Window{}, PreapproveAnswer{}, PreapproveCancelRequest{}, PreapproveCancelAnswer{}, ErrorAnswer{}, } @@ -96,7 +96,15 @@ func fullValues() []any { Secrets: []string{"backup-token"}, Key: "k", User: "u", Hosts: []string{"web-1"}, From: "2026-10-28T03:00", Until: "2026-10-28T04:00+02:00", Uses: 2, Reason: "r", }, + &PreapproveRequest{ + Secrets: []string{"backup-token"}, Hosts: []string{}, From: "2026-10-28T03:00", Until: "2026-12-28T04:00+02:00", Uses: 1, + Windows: []Window{{Kind: "daily", Days: []string{}, From: "03:00", To: "05:00"}, + {Kind: "weekly", Days: []string{"mon", "thu"}, From: "00:00", To: "23:59"}}, + }, + &Window{Kind: "weekly", Days: []string{"sun"}, From: "01:05", To: "02:10"}, &PreapproveAnswer{ID: "AAAAAAAAAAAAAAAAAAAAAA", From: "2026-10-28T03:00:00+02:00", Until: "2026-10-28T04:00:00+02:00"}, + &PreapproveAnswer{ID: "AAAAAAAAAAAAAAAAAAAAAA", From: "2026-10-28T03:00:00+02:00", Until: "2026-10-28T04:00:00+02:00", + GrantToken: "not-a-real-token"}, &PreapproveCancelRequest{ID: "AAAAAAAAAAAAAAAAAAAAAA"}, &PreapproveCancelAnswer{ID: "AAAAAAAAAAAAAAAAAAAAAA"}, &ErrorAnswer{Error: "Forbidden"}, @@ -776,3 +784,123 @@ func TestParseTime(t *testing.T) { } } } + +// TestPreapproveWindows: the windows of a standing grant are optional, +// capped, and checked strictly; an old request has no field for them. +func TestPreapproveWindows(t *testing.T) { + good := Window{Kind: "daily", Days: []string{}, From: "03:00", To: "05:00"} + base := func(ws ...Window) *PreapproveRequest { + return &PreapproveRequest{Secrets: []string{"s"}, Hosts: []string{}, From: "a", Until: "b", Uses: 1, Windows: ws} + } + data, err := json.Marshal(base()) + if err != nil || strings.Contains(string(data), "windows") { + t.Errorf("a request without windows is %s, %v; want no windows field", data, err) + } + if err := base(slices.Repeat([]Window{good}, MaxWindows)...).Check(); err != nil { + t.Errorf("%d windows: %v", MaxWindows, err) + } + for _, tt := range []struct { + name string + w []Window + }{ + {"too many", slices.Repeat([]Window{good}, MaxWindows+1)}, + {"kind", []Window{{Kind: "monthly", From: "03:00", To: "05:00"}}}, + {"empty kind", []Window{{From: "03:00", To: "05:00"}}}, + {"long kind", []Window{{Kind: strings.Repeat("a", MaxName+1), From: "03:00", To: "05:00"}}}, + {"daily with days", []Window{{Kind: "daily", Days: []string{"mon"}, From: "03:00", To: "05:00"}}}, + {"weekly without days", []Window{{Kind: "weekly", From: "03:00", To: "05:00"}}}, + {"weekly day twice", []Window{{Kind: "weekly", Days: []string{"mon", "mon"}, From: "03:00", To: "05:00"}}}, + {"weekly bad day", []Window{{Kind: "weekly", Days: []string{"Mon"}, From: "03:00", To: "05:00"}}}, + {"weekly 8 days", []Window{{Kind: "weekly", Days: slices.Repeat([]string{"mon"}, 8), From: "03:00", To: "05:00"}}}, + {"to before from", []Window{{Kind: "daily", From: "05:00", To: "03:00"}}}, + {"empty span", []Window{{Kind: "daily", From: "03:00", To: "03:00"}}}, + {"hour 24", []Window{{Kind: "daily", From: "03:00", To: "24:00"}}}, + {"minute 60", []Window{{Kind: "daily", From: "03:60", To: "04:00"}}}, + {"no zero pad", []Window{{Kind: "daily", From: "3:00", To: "05:00"}}}, + {"seconds", []Window{{Kind: "daily", From: "03:00:00", To: "05:00"}}}, + {"letters", []Window{{Kind: "daily", From: "ab:cd", To: "05:00"}}}, + {"long time", []Window{{Kind: "daily", From: strings.Repeat("1", MaxTime+1), To: "05:00"}}}, + } { + r := base(tt.w...) + if r.Check() == nil { + t.Errorf("%s: Check passes", tt.name) + } + data, err := json.Marshal(r) + if err != nil { + t.Fatal(err) + } + if _, err := decodeInto(&PreapproveRequest{}, data); err == nil { + t.Errorf("%s: the request is decoded", tt.name) + } + } + pre := `{"secrets":[],"key":"","user":"","hosts":[],"from":"","until":"","uses":1,"reason":""` + win := `{"kind":"daily","days":[],"from":"03:00","to":"05:00"}` + if _, err := decodeInto(&PreapproveRequest{}, []byte(pre+`,"windows":[`+win+`]}`)); err != nil { + t.Errorf("a valid request: %v", err) + } + if _, err := decodeInto(&PreapproveRequest{}, []byte(pre+`}`)); err != nil { + t.Errorf("a request without windows: %v", err) + } + for _, bad := range []string{ + pre + `,"windows":[]}`, + pre + `,"windows":null}`, + pre + `,"windows":[null]}`, + pre + `,"windows":{}}`, + pre + `,"windows":[` + win + `],"windows":[` + win + `]}`, + pre + `,"windows":[{"kind":"daily","days":[],"from":"03:00","to":"05:00","x":1}]}`, + pre + `,"windows":[{"kind":"daily","days":[],"from":"03:00"}]}`, + pre + `,"windows":[{"kind":"daily","days":null,"from":"03:00","to":"05:00"}]}`, + pre + `,"windows":[{"Kind":"daily","days":[],"from":"03:00","to":"05:00"}]}`, + pre + `,"windows":[{"kind":"weekly","days":["mon",null],"from":"03:00","to":"05:00"}]}`, + } { + if _, err := decodeInto(&PreapproveRequest{}, []byte(bad)); err == nil { + t.Errorf("%s: the request was decoded", bad) + } + } +} + +// TestPreapproveAnswerToken: grant_token is optional, a string, capped, +// and nothing else is allowed. +func TestPreapproveAnswerToken(t *testing.T) { + pre := `{"id":"AAAAAAAAAAAAAAAAAAAAAA","from":"a","until":"b"` + for _, ok := range []string{pre + `}`, pre + `,"grant_token":"x"}`} { + if _, err := decodeInto(&PreapproveAnswer{}, []byte(ok)); err != nil { + t.Errorf("%s: %v", ok, err) + } + } + for _, bad := range []string{ + pre + `,"grant_token":null}`, + pre + `,"grant_token":1}`, + pre + `,"grant_token":"` + strings.Repeat("a", MaxGrantToken+1) + `"}`, + pre + `,"grant_token":"x","grant_token":"y"}`, + pre + `,"token":"x"}`, + `{"id":"AAAAAAAAAAAAAAAAAAAAAA","from":"a","grant_token":"x"}`, + } { + if _, err := decodeInto(&PreapproveAnswer{}, []byte(bad)); err == nil { + t.Errorf("%s: the answer was decoded", bad) + } + } +} + +func TestGrantTokenHeader(t *testing.T) { + if GrantTokenHeader != "Nodkey-Grant-Token" { //nolint:gosec // G101: a header name, not a credential. + t.Errorf("GrantTokenHeader = %q", GrantTokenHeader) + } +} + +// TestPreapproveAnswerHidesToken: no way of printing or logging the answer +// shows the token. +func TestPreapproveAnswerHidesToken(t *testing.T) { + tok := strings.Repeat("q", 12) + "-made-up" + a := PreapproveAnswer{ID: "AAAAAAAAAAAAAAAAAAAAAA", From: "a", Until: "b", GrantToken: tok} + var buf strings.Builder + slog.New(slog.NewTextHandler(&buf, nil)).Info("x", "answer", a) + slog.New(slog.NewJSONHandler(&buf, nil)).Info("x", "answer", &a) + for _, out := range []string{ + fmt.Sprintf("%v %+v %#v %s %q %x %d", a, a, a, a, a, a, a), fmt.Sprintf("%v %+v %#v", &a, &a, &a), buf.String(), + } { + if strings.Contains(out, tok) { + t.Error("an output holds the token") + } + } +} diff --git a/internal/api/decode.go b/internal/api/decode.go index 2a38647..70d94dd 100644 --- a/internal/api/decode.go +++ b/internal/api/decode.go @@ -20,6 +20,7 @@ var ( errTrailing = errors.New("api: more data after the JSON value") errTooMany = errors.New("api: a list has more items than its cap") errTooLong = errors.New("api: a value is longer than its cap") + errBadWindow = errors.New("api: a window is not valid") errBadVar = errors.New("api: a variable name is not valid") errRead = errors.New("api: the answer could not be read") errTooLarge = errors.New("api: the answer is larger than api.MaxAnswer") @@ -34,6 +35,12 @@ type field func(raw json.RawMessage) error // null, and data must have no other field. Names must match exactly; Go's // usual decoding would also take a name in another case. func decodeObject(data []byte, fields map[string]field) error { + return decodeObjectOptional(data, fields, nil) +} + +// decodeObjectOptional is decodeObject, but the fields named in optional +// may be missing. When present, they are checked as the others are. +func decodeObjectOptional(data []byte, fields map[string]field, optional map[string]bool) error { dec := json.NewDecoder(bytes.NewReader(data)) if t, err := dec.Token(); err != nil || t != json.Delim('{') { return errNotObject @@ -73,8 +80,10 @@ func decodeObject(data []byte, fields map[string]field) error { if _, err := dec.Token(); !errors.Is(err, io.EOF) { return errTrailing } - if len(seen) != len(fields) { - return errMissing + for name := range fields { + if !seen[name] && !optional[name] { + return errMissing + } } return nil } diff --git a/internal/api/types.go b/internal/api/types.go index e2b8e3e..7fe5f23 100644 --- a/internal/api/types.go +++ b/internal/api/types.go @@ -6,6 +6,7 @@ import ( "fmt" "io" "log/slog" + "slices" ) // The requests and the answers. Each type decodes strictly @@ -510,17 +511,27 @@ type PreapproveRequest struct { // the person typed them (ParseTime). The broker reads them. From string `json:"from"` Until string `json:"until"` - // Uses is how many uses the grant has. + // Uses is how many uses the grant has. With Windows, it is the uses + // in each window. Uses int64 `json:"uses"` // Reason is the reason from --reason, or "". It is claimed. Reason string `json:"reason"` + // Windows makes the request one for a standing grant (DESIGN.md + // section 8): 1 to MaxWindows windows. Nil for a scheduled grant; then + // MarshalJSON does not write the field. + Windows []Window `json:"windows"` } // Check checks the caps of the request. func (r *PreapproveRequest) Check() error { - if len(r.Secrets) > MaxSecrets || len(r.Hosts) > MaxGrantHosts { + if len(r.Secrets) > MaxSecrets || len(r.Hosts) > MaxGrantHosts || len(r.Windows) > MaxWindows { return errTooMany } + for i := range r.Windows { + if err := r.Windows[i].Check(); err != nil { + return err + } + } if len(r.Key) > MaxName || len(r.User) > MaxName || len(r.From) > MaxTime || len(r.Until) > MaxTime { return errTooLong } @@ -532,18 +543,26 @@ func (r *PreapproveRequest) Check() error { return nil } -// MarshalJSON writes a nil list as []. +// MarshalJSON writes a nil list as [], and no field "windows" for nil +// windows. func (r PreapproveRequest) MarshalJSON() ([]byte, error) { type plain PreapproveRequest p := plain(r) p.Secrets, p.Hosts = orEmpty(p.Secrets), orEmpty(p.Hosts) + if r.Windows == nil { + // The shallower field wins over the one in plain. + return json.Marshal(struct { + plain + Windows *struct{} `json:"windows,omitempty"` + }{plain: p}) + } return json.Marshal(p) } // UnmarshalJSON decodes strictly, and checks the caps. func (r *PreapproveRequest) UnmarshalJSON(data []byte) error { var v PreapproveRequest - err := decodeObject(data, map[string]field{ + err := decodeObjectOptional(data, map[string]field{ "secrets": textList(&v.Secrets, MaxSecrets), "key": text(&v.Key), "user": text(&v.User), @@ -552,10 +571,14 @@ func (r *PreapproveRequest) UnmarshalJSON(data []byte) error { "until": text(&v.Until), "uses": integer(&v.Uses), "reason": text(&v.Reason), - }) + "windows": list(&v.Windows, MaxWindows, object[Window]()), + }, map[string]bool{"windows": true}) if err != nil { return err } + if v.Windows != nil && len(v.Windows) == 0 { + return errType // an empty list of windows is not a request + } if err := v.Check(); err != nil { return err } @@ -571,16 +594,56 @@ type PreapproveAnswer struct { // broker's local time. From string `json:"from"` Until string `json:"until"` + // GrantToken is the token of a standing grant, in the answer to + // a request with windows, and only there. It is a secret: never print + // or log it. The client checks it with grantkey.Parse. + GrantToken string `json:"grant_token"` } -// UnmarshalJSON decodes strictly. +// String returns the answer without the token. +func (a PreapproveAnswer) String() string { + t := "no token" + if a.GrantToken != "" { + t = "token hidden" + } + return "PreapproveAnswer{" + a.ID + ", " + a.From + ", " + a.Until + ", " + t + "}" +} + +// GoString returns the same text as String. +func (a PreapproveAnswer) GoString() string { return a.String() } + +// Format prints the same text as String for every verb. +func (a PreapproveAnswer) Format(f fmt.State, _ rune) { _, _ = io.WriteString(f, a.String()) } + +// LogValue returns a text without the token, so slog never logs it. +func (a PreapproveAnswer) LogValue() slog.Value { return slog.StringValue(a.String()) } + +// MarshalJSON writes no field "grant_token" when there is no token. +func (a PreapproveAnswer) MarshalJSON() ([]byte, error) { + type plain PreapproveAnswer + if a.GrantToken == "" { + return json.Marshal(struct { + plain + GrantToken *struct{} `json:"grant_token,omitempty"` + }{plain: plain(a)}) + } + return json.Marshal(plain(a)) +} + +// MaxGrantToken is the most bytes of GrantToken that the client reads. +// A token is shorter (grantkey.Length). +const MaxGrantToken = 128 + +// UnmarshalJSON decodes strictly. grant_token may be missing. func (a *PreapproveAnswer) UnmarshalJSON(data []byte) error { var v PreapproveAnswer - err := decodeObject(data, map[string]field{"id": text(&v.ID), "from": text(&v.From), "until": text(&v.Until)}) + err := decodeObjectOptional(data, map[string]field{ + "id": text(&v.ID), "from": text(&v.From), "until": text(&v.Until), "grant_token": text(&v.GrantToken), + }, map[string]bool{"grant_token": true}) if err != nil { return err } - if len(v.ID) > MaxName || len(v.From) > MaxTime || len(v.Until) > MaxTime { + if len(v.ID) > MaxName || len(v.From) > MaxTime || len(v.Until) > MaxTime || len(v.GrantToken) > MaxGrantToken { return errTooLong } *a = v @@ -644,3 +707,105 @@ func (a *ErrorAnswer) UnmarshalJSON(data []byte) error { *a = v return nil } + +// The kinds of a Window. +const ( + WindowDaily = "daily" + WindowWeekly = "weekly" +) + +// Weekdays are the day names of a weekly Window, in the order of the week. +var Weekdays = [7]string{"mon", "tue", "wed", "thu", "fri", "sat", "sun"} + +// Window is one daily or weekly window of a standing grant, in the local +// time of the broker. It does not pass midnight: From is before To. +type Window struct { + // Kind is WindowDaily or WindowWeekly. + Kind string `json:"kind"` + // Days are the day names of a weekly window (Weekdays), 1 to 7, each + // once. Empty for a daily window. + Days []string `json:"days"` + // From and To are "HH:MM", 24 hours. To is after From. + From string `json:"from"` + To string `json:"to"` +} + +// Check checks the caps and the form of the window. +func (w *Window) Check() error { + if len(w.Days) > len(Weekdays) { + return errTooMany + } + if len(w.Kind) > MaxName || len(w.From) > MaxTime || len(w.To) > MaxTime { + return errTooLong + } + switch w.Kind { + case WindowDaily: + if len(w.Days) != 0 { + return errBadWindow + } + case WindowWeekly: + if len(w.Days) == 0 { + return errBadWindow + } + seen := make(map[string]bool, len(w.Days)) + for _, d := range w.Days { + if !slices.Contains(Weekdays[:], d) || seen[d] { + return errBadWindow + } + seen[d] = true + } + default: + return errBadWindow + } + from, ok1 := ParseClock(w.From) + to, ok2 := ParseClock(w.To) + if !ok1 || !ok2 || from >= to { + return errBadWindow + } + return nil +} + +// ParseClock reads "HH:MM" (two digits each, 00:00 to 23:59) and returns +// the minutes since midnight. +func ParseClock(s string) (int, bool) { + if len(s) != 5 || s[2] != ':' { + return 0, false + } + for _, i := range []int{0, 1, 3, 4} { + if s[i] < '0' || s[i] > '9' { + return 0, false + } + } + h, m := int(s[0]-'0')*10+int(s[1]-'0'), int(s[3]-'0')*10+int(s[4]-'0') + if h > 23 || m > 59 { + return 0, false + } + return h*60 + m, true +} + +// MarshalJSON writes nil days as []. +func (w Window) MarshalJSON() ([]byte, error) { + type plain Window + p := plain(w) + p.Days = orEmpty(p.Days) + return json.Marshal(p) +} + +// UnmarshalJSON decodes strictly, and checks the caps and the form. +func (w *Window) UnmarshalJSON(data []byte) error { + var v Window + err := decodeObject(data, map[string]field{ + "kind": text(&v.Kind), + "days": textList(&v.Days, len(Weekdays)), + "from": text(&v.From), + "to": text(&v.To), + }) + if err != nil { + return err + } + if err := v.Check(); err != nil { + return err + } + *w = v + return nil +} diff --git a/internal/client/preapprove.go b/internal/client/preapprove.go index 7cc64dd..611586d 100644 --- a/internal/client/preapprove.go +++ b/internal/client/preapprove.go @@ -8,12 +8,14 @@ import ( "io" "net/http" "os" + "slices" "strconv" "strings" "sync/atomic" "time" "github.com/code6x/nodkey/internal/api" + "github.com/code6x/nodkey/internal/grantkey" "github.com/code6x/nodkey/internal/sshauth" "github.com/code6x/nodkey/internal/transport" ) @@ -21,6 +23,10 @@ import ( // preapproveUsage is the help of nodkey preapprove. const preapproveUsage = `Usage: nodkey preapprove (--secret NAME... | --key NAME --user USER --host NAME...) --from TIME --until TIME --uses N [--reason TEXT] + nodkey preapprove --secret NAME... + (--daily HH:MM-HH:MM | --weekly DAYS HH:MM-HH:MM) + --until TIME [--from TIME] [--uses N] + --grant-token FILE [--reason TEXT] nodkey preapprove --cancel ID Ask the broker now for a grant that you use later, and wait until a person @@ -42,12 +48,24 @@ is gone. A job that runs at night can ask for its grant when it is planned. 2026-10-28T03:00, which is the broker's local time. Give an offset near the change to summer time. --uses N how many uses the grant has + --daily W a standing grant (secrets only): the window W, such as + 03:00-05:00, every day, in the broker's local time + --weekly D W the same, on the days D, such as mon,thu (mon tue wed + thu fri sat sun). Give --daily or --weekly, not both. + With one of them, --until is a must, --from is optional + (default: now), --uses is the number in each window + (default 1), and the grant needs no tap after the first. + --grant-token FILE + for a standing grant: the file where the token is + written. The token is never printed. It must be a new + file, or your own file with mode 0600. --reason TEXT why the grant is needed. Without it, the reason is the environment variable NODKEY_REASON. --cancel ID end a grant of this machine now, without a tap; ID is what nodkey preapprove printed on its standard output -After an approval, the ID of the grant is printed on standard output. The +After an approval, the ID of the grant is printed on standard output. For a +standing grant, nodkey run --grant-token FILE uses the token. The client config is ~/.config/nodkey/client.toml. nodkey preapprove exits with 0 when the person approved or the grant is cancelled, or with 64 (wrong usage), 69 (the broker cannot be reached) or 77 (denied, or not a grant of @@ -95,6 +113,11 @@ type preapproveOptions struct { from, until string // as typed uses int cancel string // --cancel: the ID; the other options are not given + // windows is the standing grant: one window, from --daily or --weekly. + // Nil for a scheduled grant. + windows []api.Window + // grantToken is the file of --grant-token, for a standing grant. + grantToken string } // runPreapprove runs nodkey preapprove, and returns the exit code. @@ -113,6 +136,15 @@ func (g granter) runPreapprove(ctx context.Context, args []string, stdout, w io. if !o.reasonSet { reason = lookupEnv(g.environ(), reasonVar) } + if o.windows != nil { + // Fail before the tap, not after: the token would have no place. + if err := CheckGrantTokenFile(o.grantToken); err != nil { + return fail(stderr, exitUsage, err.Error()) + } + if o.from == "" { + o.from = g.now().Format(time.RFC3339) + } + } path, body, err := newPreapprove(o, reason) if err != nil { return fail(stderr, exitUsage, err.Error()) @@ -172,9 +204,19 @@ func (g granter) runPreapprove(ctx context.Context, args []string, stdout, w io. return cancelled(ctx, stderr) } if !valid { - mayHaveStarted() + if o.windows != nil && validGrantID(ans.ID) { + // A standing grant without a usable answer has no use. End it. + cancelLeftover(ctx, b, ans.ID, stderr) + } else { + mayHaveStarted() + } return fail(stderr, exitUnavailable, "the answer of the broker is not valid") } + if err := saveAnswerToken(o, ans.GrantToken); err != nil { + // A standing grant without its token is of no use. End it. + cancelLeftover(ctx, b, ans.ID, stderr) + return fail(stderr, exitUnavailable, err.Error()+"; the grant "+ans.ID+" is cancelled") + } // The grant is made. The ID goes to stdout for a script that cancels. _, _ = fmt.Fprintln(stdout, ans.ID) _, _ = fmt.Fprintf(stderr, "nodkey: grant %s: %s from %s until %s (exit %d)\n", @@ -182,6 +224,30 @@ func (g granter) runPreapprove(ctx context.Context, args []string, stdout, w io. return exitOK } +// saveAnswerToken checks the token of the answer against the request and +// saves it. A scheduled grant must come with no token; a standing grant +// must come with a token that grantkey.Parse accepts. The errors never +// hold the token. +func saveAnswerToken(o *preapproveOptions, raw string) error { + if o.windows == nil { + if raw != "" { + return errors.New("the answer of the broker is not valid: it has a token for a grant that is not a standing grant") + } + return nil + } + if raw == "" { + return errors.New("the answer of the broker is not valid: it has no token") + } + tok, err := grantkey.Parse(raw) + if err != nil { + return errors.New("the answer of the broker is not valid: its token is not a token") + } + if err := SaveGrantToken(o.grantToken, tok); err != nil { + return err + } + return nil +} + // cancelWait is the most time that cancelLeftover waits for the broker. const cancelWait = 30 * time.Second @@ -254,7 +320,8 @@ func parsePreapproveArgs(args []string) (*preapproveOptions, error) { rest = rest[1:] name, value, hasValue := splitOption(arg) switch name { - case "secret", "key", "user", "host", "from", "until", "uses", "reason", "cancel": + case "secret", "key", "user", "host", "from", "until", "uses", "reason", "cancel", + "daily", "weekly", "grant-token": case "h", "help": return nil, errors.New("--help must be the only argument") default: @@ -277,6 +344,20 @@ func parsePreapproveArgs(args []string) (*preapproveOptions, error) { } var err error switch name { + case "daily": + err = addWindow(o, given, name, value, "") + case "weekly": + if len(rest) == 0 { + return nil, errors.New("--weekly needs days and a window, such as --weekly mon,thu 03:00-05:00") + } + var span string + span, rest = rest[0], rest[1:] + err = addWindow(o, given, name, value, span) + case "grant-token": + if value == "" || len(value) > 4096 || strings.ContainsRune(value, 0) { + err = errors.New("--grant-token: give the path of a file") + } + o.grantToken = value case "secret": err = addSecret(o, seen, value) case "key": @@ -327,6 +408,12 @@ func checkPreapproveGiven(o *preapproveOptions, given map[string]bool) error { } return nil } + if o.windows != nil { + return checkStandingGiven(o, given) + } + if given["grant-token"] { + return errors.New("--grant-token is for a standing grant: give --daily or --weekly") + } switch { case len(o.secrets) > 0 && (given["key"] || given["user"] || len(o.hosts) > 0): return errors.New("give secrets with --secret, or a key with --key, --user and --host, not both") @@ -386,6 +473,7 @@ func newPreapprove(o *preapproveOptions, reason string) (string, []byte, error) req := api.PreapproveRequest{ Secrets: o.secrets, Key: o.key, User: o.user, Hosts: o.hosts, From: o.from, Until: o.until, Uses: int64(o.uses), Reason: claimedText(reason), + Windows: o.windows, } if err := req.Check(); err != nil { return "", nil, errors.New("the request is over a cap of the broker") @@ -398,3 +486,71 @@ func newPreapprove(o *preapproveOptions, reason string) (string, []byte, error) } return path, body, nil } + +// checkStandingGiven checks the options of a standing grant: secrets, +// --until and --grant-token are a must; --from and --uses are optional. +func checkStandingGiven(o *preapproveOptions, given map[string]bool) error { + switch { + case given["key"] || given["user"] || len(o.hosts) > 0: + return errors.New("a standing grant is for secrets only; give them with --secret") + case len(o.secrets) == 0: + return errors.New("give the secrets with --secret NAME") + case !given["until"]: + return errors.New("a standing grant needs its end: give --until TIME, such as --until 2026-12-28T00:00") + case !given["grant-token"]: + return errors.New("a standing grant needs --grant-token FILE, the file where the token is written") + } + if !given["uses"] { + o.uses = 1 + } + return nil +} + +// addWindow adds the window of --daily (days "") or --weekly to o. Only one +// of the two may be given, and only once. For --weekly, days is the list +// of day names; span is the window "HH:MM-HH:MM". +func addWindow(o *preapproveOptions, given map[string]bool, option, value, span string) error { + if given["daily"] && option == "weekly" || given["weekly"] && option == "daily" { + return errors.New("give --daily or --weekly, not both") + } + w := api.Window{Kind: api.WindowDaily, Days: []string{}} + if option == "weekly" { + w.Kind = api.WindowWeekly + days, err := parseDays(value) + if err != nil { + return fmt.Errorf("--weekly: %w", err) + } + w.Days = days + } else { + span = value + } + from, to, ok := strings.Cut(span, "-") + if !ok { + return fmt.Errorf("--%s %.64q: give a window such as 03:00-05:00", option, span) + } + w.From, w.To = from, to + if err := w.Check(); err != nil { + return fmt.Errorf("--%s %.64q: give a window of two times HH:MM on one day, such as 03:00-05:00, with the first before the second", option, span) + } + o.windows = []api.Window{w} + return nil +} + +// parseDays reads day names such as "mon,thu": lower case, each once. It +// returns them in the order of the week. +func parseDays(s string) ([]string, error) { + var days []string + for name := range strings.SplitSeq(s, ",") { + if !slices.Contains(api.Weekdays[:], name) { + return nil, fmt.Errorf("%.64q is not a day; use mon, tue, wed, thu, fri, sat or sun, with commas, such as mon,thu", name) + } + if slices.Contains(days, name) { + return nil, fmt.Errorf("the day %s is given twice", name) + } + days = append(days, name) + } + slices.SortFunc(days, func(a, b string) int { + return slices.Index(api.Weekdays[:], a) - slices.Index(api.Weekdays[:], b) + }) + return days, nil +} diff --git a/internal/client/preapprove_standing_test.go b/internal/client/preapprove_standing_test.go new file mode 100644 index 0000000..400256c --- /dev/null +++ b/internal/client/preapprove_standing_test.go @@ -0,0 +1,433 @@ +package client + +import ( + "encoding/json" + "errors" + "io" + "net/http" + "os" + "path/filepath" + "strings" + "sync" + "testing" + + "github.com/code6x/nodkey/internal/api" + "github.com/code6x/nodkey/internal/grantkey" +) + +const testGrantID = "AAAAAAAAAAAAAAAAAAAAAA" + +// standingArgs are the arguments of a standing grant that writes its token +// to file, and then extra. +func standingArgs(file string, extra ...string) []string { + return append([]string{"--secret", "backup-token", "--daily", "03:00-05:00", "--until", "2026-12-28T00:00", + "--grant-token", file}, extra...) +} + +func newToken(t testing.TB) grantkey.Token { + t.Helper() + tok, err := grantkey.New() + if err != nil { + t.Fatal(err) + } + return tok +} + +// standingBroker answers a preapprove request with a grant that has the +// token field token ("" for none), and a cancel with the ID. +type standingBroker struct { + *fakeBroker + mu sync.Mutex + preBodies []string + cancels []string +} + +func newStandingBroker(t *testing.T, token string) *standingBroker { + t.Helper() + sb := &standingBroker{} + sb.fakeBroker = newFakeBroker(t, func(w http.ResponseWriter, r *http.Request) { + body, _ := io.ReadAll(r.Body) + sb.mu.Lock() + defer sb.mu.Unlock() + if r.URL.Path == api.PathPreapproveCancel { + sb.cancels = append(sb.cancels, string(body)) + answer(http.StatusOK, `{"id":"`+testGrantID+`"}`)(w, r) + return + } + sb.preBodies = append(sb.preBodies, string(body)) + a := `{"id":"` + testGrantID + `","from":"2026-10-15T12:00:00Z","until":"2026-12-28T00:00:00Z"` + if token != "" { + a += `,"grant_token":"` + token + `"` + } + answer(http.StatusOK, a+"}")(w, r) + }) + return sb +} + +func (sb *standingBroker) bodies() ([]string, []string) { + sb.mu.Lock() + defer sb.mu.Unlock() + return append([]string(nil), sb.preBodies...), append([]string(nil), sb.cancels...) +} + +// TestPreapproveStanding: the token goes to the file with mode 0600, is +// never printed, and the request has the window and the defaults. +func TestPreapproveStanding(t *testing.T) { + t.Parallel() + tok := newToken(t) + sb := newStandingBroker(t, tok.Reveal()) + file := filepath.Join(t.TempDir(), "grant.token") + code, stdout, stderr := preIn(t.Context(), testGranter(sb.b, nil), standingArgs(file)...) + mustNotLeak(t, tok.Reveal(), stdout, stderr) + if code != exitOK || stdout != testGrantID+"\n" { + t.Fatalf("exit code %d, stdout %q, stderr %q", code, stdout, stderr) + } + got, err := os.ReadFile(file) //nolint:gosec // G304: a test path. + if err != nil || string(got) != tok.Reveal()+"\n" { + t.Errorf("token file: %v (content matches: %v)", err, string(got) == tok.Reveal()+"\n") + } + if fi, _ := os.Stat(file); fi == nil || fi.Mode().Perm() != 0o600 { + t.Errorf("token file mode %v, want 0600", fi) + } + bodies, cancels := sb.bodies() + if len(bodies) != 1 || len(cancels) != 0 { + t.Fatalf("%d requests, %d cancels", len(bodies), len(cancels)) + } + var req api.PreapproveRequest + if err := json.Unmarshal([]byte(bodies[0]), &req); err != nil { + t.Fatal(err) + } + w := req.Windows + if len(w) != 1 || w[0].Kind != "daily" || w[0].From != "03:00" || w[0].To != "05:00" || req.Uses != 1 || + req.From != noon.Format("2006-01-02T15:04:05Z07:00") || req.Until != "2026-12-28T00:00" || + len(req.Secrets) != 1 || req.Secrets[0] != "backup-token" { + t.Errorf("request %+v", req) + } + if entries, _ := os.ReadDir(filepath.Dir(file)); len(entries) != 1 { + t.Errorf("%d files in the directory, want only the token file", len(entries)) + } +} + +// TestPreapproveStandingWeeklyOwnFile: weekly days are sorted; the old +// token file of the user (mode 0600) is replaced. +func TestPreapproveStandingWeeklyOwnFile(t *testing.T) { + t.Parallel() + tok := newToken(t) + sb := newStandingBroker(t, tok.Reveal()) + file := filepath.Join(t.TempDir(), "grant.token") + if err := os.WriteFile(file, []byte("old\n"), 0o600); err != nil { + t.Fatal(err) + } + code, _, stderr := preIn(t.Context(), testGranter(sb.b, nil), + "--secret", "s", "--weekly", "thu,mon", "03:00-05:00", "--from", "2026-10-16T00:00", "--until", "2026-12-28T00:00", + "--uses", "3", "--grant-token", file) + if code != exitOK { + t.Fatalf("exit code %d, stderr %q", code, stderr) + } + if got, _ := os.ReadFile(file); string(got) != tok.Reveal()+"\n" { //nolint:gosec // G304: a test path. + t.Error("the token file was not replaced") + } + bodies, _ := sb.bodies() + var req api.PreapproveRequest + if err := json.Unmarshal([]byte(bodies[0]), &req); err != nil { + t.Fatal(err) + } + if len(req.Windows) != 1 || req.Windows[0].Kind != "weekly" || strings.Join(req.Windows[0].Days, ",") != "mon,thu" || + req.Uses != 3 || req.From != "2026-10-16T00:00" { + t.Errorf("request %+v", req) + } +} + +// TestPreapproveStandingBadAnswers: no token, a bad token, a token for a +// scheduled grant, and a file that cannot be saved all fail, cancel the +// grant, and never print the token. +func TestPreapproveStandingBadAnswers(t *testing.T) { + t.Parallel() + good := newToken(t).Reveal() + for _, tt := range []struct { + name, token string + dirGone bool + }{ + {"no token", "", false}, + {"bad checksum", good[:len(good)-1] + map[bool]string{true: "0", false: "1"}[good[len(good)-1] == '1'], false}, + {"not a token", "hello", false}, + {"cannot write", good, true}, + } { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + sb := newStandingBroker(t, tt.token) + dir := t.TempDir() + file := filepath.Join(dir, "grant.token") + args := standingArgs(file) + if tt.dirGone { + // The directory is there for the check before the request, + // and gone for the save. + file = filepath.Join(dir, "sub", "grant.token") + args = standingArgs(file) + if err := os.Mkdir(filepath.Join(dir, "sub"), 0o700); err != nil { + t.Fatal(err) + } + sb2 := newFakeBroker(t, func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path == api.PathPreapproveCancel { + answer(http.StatusOK, `{"id":"`+testGrantID+`"}`)(w, r) + return + } + _ = os.RemoveAll(filepath.Join(dir, "sub")) + answer(http.StatusOK, `{"id":"`+testGrantID+`","from":"2026-10-15T12:00:00Z","until":"2026-12-28T00:00:00Z","grant_token":"`+tt.token+`"}`)(w, r) + }) + sb.fakeBroker = sb2 + } + code, stdout, stderr := preIn(t.Context(), testGranter(sb.b, nil), args...) + mustNotLeak(t, good, stdout, stderr) + if tt.token != "" { + mustNotLeak(t, tt.token, stdout, stderr) + } + if code != exitUnavailable || stdout != "" || !strings.Contains(stderr, testGrantID) || + !strings.HasSuffix(stderr, "(exit 69)\n") { + t.Errorf("exit code %d, stdout %q, stderr %q", code, stdout, stderr) + } + if _, err := os.Stat(file); err == nil { + t.Error("a token file exists") + } + if tt.dirGone { + return + } + if _, cancels := sb.bodies(); len(cancels) != 1 || !strings.Contains(cancels[0], testGrantID) { + t.Errorf("cancels %q, want one", cancels) + } + }) + } +} + +// TestPreapproveScheduledRefusesToken: without --daily or --weekly an +// answer with a token fails closed, and the grant is cancelled. +func TestPreapproveScheduledRefusesToken(t *testing.T) { + t.Parallel() + tok := newToken(t) + sb := newStandingBroker(t, tok.Reveal()) + code, stdout, stderr := preIn(t.Context(), testGranter(sb.b, nil), + "--secret", "s", "--from", "2026-10-15T12:00:00Z", "--until", "2026-10-15T13:00:00Z", "--uses", "1") + mustNotLeak(t, tok.Reveal(), stdout, stderr) + if code != exitUnavailable || stdout != "" { + t.Errorf("exit code %d, stdout %q, stderr %q", code, stdout, stderr) + } + if _, cancels := sb.bodies(); len(cancels) != 1 { + t.Errorf("%d cancels, want 1", len(cancels)) + } +} + +// TestPreapproveScheduledUnchanged: the request of a scheduled grant has no +// windows, and an answer without a token passes. +func TestPreapproveScheduledUnchanged(t *testing.T) { + t.Parallel() + sb := newStandingBroker(t, "") + code, stdout, stderr := preIn(t.Context(), testGranter(sb.b, nil), + "--secret", "s", "--from", "2026-10-15T12:00:00Z", "--until", "2026-10-15T13:00:00Z", "--uses", "2") + if code != exitOK || stdout != testGrantID+"\n" { + t.Fatalf("exit code %d, stdout %q, stderr %q", code, stdout, stderr) + } + bodies, _ := sb.bodies() + if strings.Contains(bodies[0], "windows") { + t.Errorf("request %s has windows", bodies[0]) + } +} + +// TestPreapproveStandingBadFile: a bad token file is refused before the +// broker is asked. +func TestPreapproveStandingBadFile(t *testing.T) { + t.Parallel() + dir := t.TempDir() + loose := filepath.Join(dir, "loose") + if err := os.WriteFile(loose, nil, 0o600); err != nil { + t.Fatal(err) + } + if err := os.Chmod(loose, 0o640); err != nil { //nolint:gosec // G302: the test makes a loose file. + t.Fatal(err) + } + target := filepath.Join(dir, "target") + if err := os.WriteFile(target, nil, 0o600); err != nil { + t.Fatal(err) + } + link := filepath.Join(dir, "link") + if err := os.Symlink(target, link); err != nil { + t.Fatal(err) + } + for _, file := range []string{loose, link, dir} { + sb := newStandingBroker(t, newToken(t).Reveal()) + code, stdout, stderr := preIn(t.Context(), testGranter(sb.b, nil), standingArgs(file)...) + if code != exitUsage || stdout != "" || sb.requests.Load() != 0 || !strings.Contains(stderr, "grant token file") { + t.Errorf("%s: exit code %d, stdout %q, stderr %q, %d requests", filepath.Base(file), code, stdout, stderr, sb.requests.Load()) + } + } +} + +// TestPreapproveStandingFlags: the flags of a standing grant. +func TestPreapproveStandingFlags(t *testing.T) { + t.Parallel() + ok := func(extra ...string) []string { + return append([]string{"--secret", "s", "--grant-token", "f", "--until", "2026-12-28T00:00"}, extra...) + } + for _, tt := range []struct { + name string + args []string + ok bool + uses int + }{ + {"daily", ok("--daily", "03:00-05:00"), true, 1}, + {"weekly", ok("--weekly", "mon,thu", "03:00-05:00"), true, 1}, + {"weekly equals", ok("--weekly=sun", "03:00-05:00", "--uses", "4"), true, 4}, + {"both", ok("--daily", "03:00-05:00", "--weekly", "mon", "03:00-05:00"), false, 0}, + {"both reverse", ok("--weekly", "mon", "03:00-05:00", "--daily", "03:00-05:00"), false, 0}, + {"daily twice", ok("--daily", "03:00-05:00", "--daily", "03:00-05:00"), false, 0}, + {"bad time", ok("--daily", "3:00-05:00"), false, 0}, + {"reversed", ok("--daily", "05:00-03:00"), false, 0}, + {"midnight", ok("--daily", "23:00-24:00"), false, 0}, + {"no dash", ok("--daily", "03:00"), false, 0}, + {"seconds", ok("--daily", "03:00:00-05:00:00"), false, 0}, + {"bad day", ok("--weekly", "monday", "03:00-05:00"), false, 0}, + {"upper day", ok("--weekly", "Mon", "03:00-05:00"), false, 0}, + {"day twice", ok("--weekly", "mon,mon", "03:00-05:00"), false, 0}, + {"empty day", ok("--weekly", "mon,", "03:00-05:00"), false, 0}, + {"weekly without window", ok("--weekly", "mon"), false, 0}, + {"weekly window is an option", ok("--weekly", "mon", "--uses", "2"), false, 0}, + {"no until", []string{"--secret", "s", "--grant-token", "f", "--daily", "03:00-05:00"}, false, 0}, + {"no grant-token", []string{"--secret", "s", "--until", "2026-12-28T00:00", "--daily", "03:00-05:00"}, false, 0}, + {"no secret", []string{"--grant-token", "f", "--until", "2026-12-28T00:00", "--daily", "03:00-05:00"}, false, 0}, + {"key", []string{"--key", "k", "--user", "u", "--host", "h", "--grant-token", "f", "--until", "2026-12-28T00:00", "--daily", "03:00-05:00"}, false, 0}, + {"bad until", append([]string{"--daily", "03:00-05:00", "--secret", "s", "--grant-token", "f"}, "--until", "soon"), false, 0}, + {"zero uses", ok("--daily", "03:00-05:00", "--uses", "0"), false, 0}, + {"empty token path", []string{"--secret", "s", "--grant-token", "", "--until", "2026-12-28T00:00", "--daily", "03:00-05:00"}, false, 0}, + {"token twice", ok("--daily", "03:00-05:00", "--grant-token", "g"), false, 0}, + {"cancel mixed", []string{"--cancel", testGrantID, "--daily", "03:00-05:00"}, false, 0}, + {"token without window", []string{"--secret", "s", "--from", "2026-10-15T12:00", "--until", "2026-12-28T00:00", "--uses", "1", "--grant-token", "f"}, false, 0}, + } { + o, err := parsePreapproveArgs(tt.args) + if (err == nil) != tt.ok { + t.Errorf("%s: error %v, want ok=%v", tt.name, err, tt.ok) + continue + } + if err == nil && (o.uses != tt.uses || len(o.windows) != 1) { + t.Errorf("%s: uses %d windows %v", tt.name, o.uses, o.windows) + } + } + // A scheduled request is as before: --from and --uses are a must. + if _, err := parsePreapproveArgs([]string{"--secret", "s", "--until", "2026-12-28T00:00"}); err == nil { + t.Error("a scheduled grant without --from and --uses passes") + } +} + +// TestSaveGrantToken: the helper writes with mode 0600, replaces its own +// file, refuses files that are not safe, and never follows a link. +func TestSaveGrantToken(t *testing.T) { + t.Parallel() + tok := newToken(t) + dir := t.TempDir() + file := filepath.Join(dir, "t") + if err := SaveGrantToken(file, tok); err != nil { + t.Fatal(err) + } + if fi, _ := os.Stat(file); fi.Mode().Perm() != 0o600 { + t.Errorf("mode %v", fi.Mode()) + } + tok2 := newToken(t) + if err := SaveGrantToken(file, tok2); err != nil { + t.Fatal(err) + } + if got, _ := os.ReadFile(file); string(got) != tok2.Reveal()+"\n" { //nolint:gosec // test path or test file mode. + t.Error("not replaced") + } + // Bad modes of an existing file. + for _, mode := range []os.FileMode{0o640, 0o604, 0o660, 0o666, 0o602} { + if err := os.Chmod(file, mode); err != nil { + t.Fatal(err) + } + err := SaveGrantToken(file, tok) + if !errors.Is(err, ErrTokenFile) { + t.Errorf("mode %v: error %v", mode, err) + } + if got, _ := os.ReadFile(file); string(got) != tok2.Reveal()+"\n" { //nolint:gosec // test path or test file mode. + t.Errorf("mode %v: the file changed", mode) + } + } + if err := os.Chmod(file, 0o400); err != nil { + t.Fatal(err) + } + if err := SaveGrantToken(file, tok); err != nil { + t.Errorf("mode 0400: %v", err) + } + // A link is never followed and never replaced. + target := filepath.Join(dir, "target") + if err := os.WriteFile(target, []byte("keep"), 0o600); err != nil { + t.Fatal(err) + } + link := filepath.Join(dir, "link") + if err := os.Symlink(target, link); err != nil { + t.Fatal(err) + } + if err := SaveGrantToken(link, tok); !errors.Is(err, ErrTokenFile) { + t.Errorf("link: %v", err) + } + if got, _ := os.ReadFile(target); string(got) != "keep" { //nolint:gosec // test path or test file mode. + t.Error("the target of the link changed") + } + // A directory, a missing directory, an empty path, a bad token. + if err := SaveGrantToken(dir, tok); !errors.Is(err, ErrTokenFile) { + t.Errorf("directory: %v", err) + } + if err := SaveGrantToken(filepath.Join(dir, "no", "t"), tok); !errors.Is(err, ErrTokenFile) { + t.Errorf("missing directory: %v", err) + } + if err := SaveGrantToken("", tok); !errors.Is(err, ErrTokenFile) { + t.Errorf("empty path: %v", err) + } + if err := SaveGrantToken(filepath.Join(dir, "x"), grantkey.Token{}); !errors.Is(err, ErrTokenFile) { + t.Errorf("zero token: %v", err) + } + // No temporary file is left. + entries, _ := os.ReadDir(dir) + for _, e := range entries { + if strings.Contains(e.Name(), ".tmp-") { + t.Errorf("left over: %s", e.Name()) + } + } +} + +// TestSaveGrantTokenNoLeak: an error never holds the token. +func TestSaveGrantTokenNoLeak(t *testing.T) { + t.Parallel() + tok := newToken(t) + err := SaveGrantToken(filepath.Join(t.TempDir(), "no", "t"), tok) + if err == nil || strings.Contains(err.Error(), tok.Reveal()) { + t.Errorf("the error is nil, or holds the token: %v", err == nil) + } +} + +// TestPreapproveStandingBadTimeCancels: a standing grant whose answer has a +// bad time is cancelled, and no file is written. +func TestPreapproveStandingBadTimeCancels(t *testing.T) { + t.Parallel() + tok := newToken(t) + var mu sync.Mutex + cancels := 0 + fb := newFakeBroker(t, func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path == api.PathPreapproveCancel { + mu.Lock() + cancels++ + mu.Unlock() + answer(http.StatusOK, `{"id":"`+testGrantID+`"}`)(w, r) + return + } + answer(http.StatusOK, `{"id":"`+testGrantID+`","from":"x","until":"y","grant_token":"`+tok.Reveal()+`"}`)(w, r) + }) + file := filepath.Join(t.TempDir(), "grant.token") + code, stdout, stderr := preIn(t.Context(), testGranter(fb.b, nil), standingArgs(file)...) + mustNotLeak(t, tok.Reveal(), stdout, stderr) + mu.Lock() + defer mu.Unlock() + if code != exitUnavailable || cancels != 1 { + t.Errorf("exit code %d, %d cancels, stderr %q", code, cancels, stderr) + } + if _, err := os.Stat(file); err == nil { + t.Error("a token file exists") + } +} diff --git a/internal/client/tokenfile.go b/internal/client/tokenfile.go new file mode 100644 index 0000000..a03d018 --- /dev/null +++ b/internal/client/tokenfile.go @@ -0,0 +1,121 @@ +package client + +import ( + "crypto/rand" + "encoding/hex" + "errors" + "fmt" + "io/fs" + "os" + "path/filepath" + + "github.com/code6x/nodkey/internal/grantkey" +) + +// The token file of a standing grant (DESIGN.md sections 8 and 15) holds +// one token and a line feed. Only its owner may read and write it. These +// helpers are for nodkey preapprove, which writes the first token, and for +// nodkey run, which writes each new token. + +// ErrTokenFile is returned for a token file that the client refuses, or +// cannot save. Errors that wrap it never hold the token. +var ErrTokenFile = errors.New("grant token file") + +// CheckGrantTokenFile checks that path is free to use as a token file: +// either nothing is there, or a regular file that the user owns, with no +// permission for the group and for others (mode 0600 at most). A symbolic +// link is refused. It does not read the file. +func CheckGrantTokenFile(path string) error { + if path == "" { + return fmt.Errorf("%w: the path is empty", ErrTokenFile) + } + fi, err := os.Lstat(path) //nolint:gosec // G703: the path is the one that the person gave. + switch { + case errors.Is(err, fs.ErrNotExist): + return nil + case err != nil: + return fmt.Errorf("%w: cannot look at %q: %s", ErrTokenFile, path, errText(err)) + } + return checkTokenFileInfo(path, fi) +} + +// checkTokenFileInfo checks the Lstat result fi of the file path. +func checkTokenFileInfo(path string, fi fs.FileInfo) error { + switch { + case !fi.Mode().IsRegular(): + return fmt.Errorf("%w: %q is not a regular file", ErrTokenFile, path) + case !ownedByUser(fi): + return fmt.Errorf("%w: %q is not owned by this user", ErrTokenFile, path) + case fi.Mode().Perm()&0o077 != 0: + return fmt.Errorf("%w: %q can be read or written by other users; use mode 0600", ErrTokenFile, path) + } + return nil +} + +// SaveGrantToken writes tok to the file path so that a reader sees the old +// token or the new one, never a part. It checks the file as +// CheckGrantTokenFile does, makes a new file with mode 0600 in the same +// directory (it never follows a link and never reuses a name), writes +// and syncs it, renames it over path, and syncs the directory. On an error +// no new file is left. Its errors never hold the token. +func SaveGrantToken(path string, tok grantkey.Token) (err error) { + if err := CheckGrantTokenFile(path); err != nil { + return err + } + if _, perr := grantkey.Parse(tok.Reveal()); perr != nil { + return fmt.Errorf("%w: not a token", ErrTokenFile) + } + dir := filepath.Dir(path) + var rnd [8]byte + if _, err := rand.Read(rnd[:]); err != nil { + return fmt.Errorf("%w: no random bytes", ErrTokenFile) + } + tmp := filepath.Join(dir, "."+filepath.Base(path)+".tmp-"+hex.EncodeToString(rnd[:])) + f, err := os.OpenFile(tmp, os.O_WRONLY|os.O_CREATE|os.O_EXCL|noFollow, 0o600) //nolint:gosec // G304: the path is the one that the person gave. + if err != nil { + return fmt.Errorf("%w: cannot make a file in %q: %s", ErrTokenFile, dir, errText(err)) + } + defer func() { + if err != nil { + _ = os.Remove(tmp) + } + }() + if err = f.Chmod(0o600); err == nil { + _, err = f.WriteString(tok.Reveal() + "\n") + } + if err == nil { + err = f.Sync() + } + if cerr := f.Close(); err == nil { + err = cerr + } + if err != nil { + return fmt.Errorf("%w: cannot write the file in %q: %s", ErrTokenFile, dir, errText(err)) + } + // The file may have changed while the broker was asked. + if err = CheckGrantTokenFile(path); err != nil { + return err + } + if err = os.Rename(tmp, path); err != nil { + return fmt.Errorf("%w: cannot replace %q: %s", ErrTokenFile, path, errText(err)) + } + d, err := os.Open(dir) //nolint:gosec // G304: the directory of the path that the person gave. + if err == nil { + err = d.Sync() + _ = d.Close() + } + if err != nil { + // The new file is in place; do not remove it. + return fmt.Errorf("%w: cannot sync the directory %q: %s", ErrTokenFile, dir, errText(err)) + } + return nil +} + +// errText is the reason of an operating system error, without the path. +func errText(err error) string { + var pe *fs.PathError + if errors.As(err, &pe) { + err = pe.Err + } + return err.Error() +} diff --git a/internal/client/tokenfile_other.go b/internal/client/tokenfile_other.go new file mode 100644 index 0000000..8f0fff7 --- /dev/null +++ b/internal/client/tokenfile_other.go @@ -0,0 +1,11 @@ +//go:build !unix + +package client + +import "io/fs" + +// ownedByUser fails closed where the owner cannot be checked. +func ownedByUser(fs.FileInfo) bool { return false } + +// noFollow is not there; the name is new, and O_EXCL refuses a link. +const noFollow = 0 diff --git a/internal/client/tokenfile_unix.go b/internal/client/tokenfile_unix.go new file mode 100644 index 0000000..d40c21b --- /dev/null +++ b/internal/client/tokenfile_unix.go @@ -0,0 +1,18 @@ +//go:build unix + +package client + +import ( + "io/fs" + "os" + "syscall" +) + +// ownedByUser reports whether the user of this process owns the file. +func ownedByUser(fi fs.FileInfo) bool { + st, ok := fi.Sys().(*syscall.Stat_t) + return ok && int(st.Uid) == os.Geteuid() +} + +// noFollow makes open refuse a symbolic link. +const noFollow = syscall.O_NOFOLLOW From 6cbb427a182daed682ca7b29a4fc80662de2cb79 Mon Sep 17 00:00:00 2001 From: Geno Roupsky <185580+groupsky@users.noreply.github.com> Date: Thu, 29 Oct 2026 07:20:15 +0200 Subject: [PATCH 07/18] feat: nodkey run --grant-token (#110) Co-Authored-By: Claude Opus 5.5 --- internal/api/secrets_token_test.go | 40 ++++++ internal/api/types.go | 44 ++++++- internal/client/client.go | 13 ++ internal/client/run.go | 88 ++++++++++++- internal/client/run_grant_test.go | 190 +++++++++++++++++++++++++++++ 5 files changed, 367 insertions(+), 8 deletions(-) create mode 100644 internal/api/secrets_token_test.go create mode 100644 internal/client/run_grant_test.go diff --git a/internal/api/secrets_token_test.go b/internal/api/secrets_token_test.go new file mode 100644 index 0000000..0c775d1 --- /dev/null +++ b/internal/api/secrets_token_test.go @@ -0,0 +1,40 @@ +package api + +import ( + "encoding/json" + "fmt" + "strings" + "testing" +) + +func TestSecretsAnswerGrantToken(t *testing.T) { + t.Parallel() + tok := "nkg_" + strings.Repeat("A", 40) + a := SecretsAnswer{Secrets: []SecretValue{{Var: "A", Value: []byte("v")}}, GrantToken: tok} + b, err := a.Encode() + if err != nil { + t.Fatal(err) + } + var got SecretsAnswer + if err := json.Unmarshal(b, &got); err != nil || got.GrantToken != tok { + t.Fatalf("round trip: %v", err) + } + if s := fmt.Sprintf("%v %+v %#v", got, got, got); strings.Contains(s, tok) { + t.Error("the token is shown") + } + // Without a token: still valid. Unknown field and long token: refused. + if err := json.Unmarshal([]byte(`{"secrets":[]}`), &got); err != nil { + t.Errorf("no token: %v", err) + } + for _, in := range []string{`{"secrets":[],"x":"y"}`, + `{"secrets":[],"grant_token":"` + strings.Repeat("a", MaxGrantToken+1) + `"}`, + `{"secrets":[],"grant_token":1}`} { + if err := json.Unmarshal([]byte(in), &got); err == nil { + t.Errorf("accepted %.40s", in) + } + } + a.GrantToken = `a"b` + if _, err := a.Encode(); err == nil { + t.Error("encoded a token that needs escaping") + } +} diff --git a/internal/api/types.go b/internal/api/types.go index 7fe5f23..fc8156d 100644 --- a/internal/api/types.go +++ b/internal/api/types.go @@ -164,6 +164,10 @@ func (s *SecretRef) UnmarshalJSON(data []byte) error { // of its pool, which nobody clears. type SecretsAnswer struct { Secrets []SecretValue `json:"secrets"` + // GrantToken is the new token of a standing grant (DESIGN.md section + // 8), in the answer to a request with the header GrantTokenHeader, and + // only there. It is a secret: never print or log it. + GrantToken string `json:"grant_token"` } // Check checks the caps of the answer: at most MaxSecrets values, each at @@ -172,6 +176,9 @@ func (a *SecretsAnswer) Check() error { if len(a.Secrets) > MaxSecrets { return errTooMany } + if len(a.GrantToken) > MaxGrantToken { + return errTooLong + } total := 0 for _, s := range a.Secrets { total += len(s.Value) @@ -182,12 +189,26 @@ func (a *SecretsAnswer) Check() error { return nil } +// MarshalJSON writes no field "grant_token" when there is no token. The +// broker writes the answer with Encode; this is for tests. +func (a SecretsAnswer) MarshalJSON() ([]byte, error) { + type plain SecretsAnswer + if a.GrantToken == "" { + return json.Marshal(struct { + plain + GrantToken *struct{} `json:"grant_token,omitempty"` + }{plain: plain(a)}) + } + return json.Marshal(plain(a)) +} + // UnmarshalJSON decodes strictly, and checks the caps. func (a *SecretsAnswer) UnmarshalJSON(data []byte) error { var v SecretsAnswer - err := decodeObject(data, map[string]field{ - "secrets": list(&v.Secrets, MaxSecrets, object[SecretValue]()), - }) + err := decodeObjectOptional(data, map[string]field{ + "secrets": list(&v.Secrets, MaxSecrets, object[SecretValue]()), + "grant_token": text(&v.GrantToken), + }, map[string]bool{"grant_token": true}) if err != nil { return err } @@ -216,7 +237,16 @@ func (a *SecretsAnswer) Encode() ([]byte, error) { tail = `"}` end = `]}` ) + const tokenKey = `],"grant_token":"` n := len(head) + len(end) + if a.GrantToken != "" { + for _, c := range []byte(a.GrantToken) { + if c != '_' && (c < '0' || c > '9') && (c < 'a' || c > 'z') && (c < 'A' || c > 'Z') { + return nil, errBadVar // a token needs no escaping in JSON + } + } + n += len(tokenKey) + len(a.GrantToken) + len(`"}`) - len(end) + } for i, s := range a.Secrets { if !ValidVar(s.Var) { return nil, errBadVar @@ -240,6 +270,11 @@ func (a *SecretsAnswer) Encode() ([]byte, error) { b = base64.StdEncoding.AppendEncode(b, s.Value) b = append(b, tail...) } + if a.GrantToken != "" { + b = append(b, tokenKey...) + b = append(b, a.GrantToken...) + return append(b, `"}`...), nil + } return append(b, end...), nil } @@ -253,6 +288,9 @@ func (a *SecretsAnswer) Clear() { // String returns a text without the values. func (a SecretsAnswer) String() string { + if a.GrantToken != "" { + return fmt.Sprintf("api.SecretsAnswer{%d values, token hidden}", len(a.Secrets)) + } return fmt.Sprintf("api.SecretsAnswer{%d values}", len(a.Secrets)) } diff --git a/internal/client/client.go b/internal/client/client.go index 200456a..a7c2f9e 100644 --- a/internal/client/client.go +++ b/internal/client/client.go @@ -29,6 +29,7 @@ import ( "net/url" "os" "path/filepath" + "slices" "strings" "sync" "time" @@ -233,6 +234,15 @@ func Post[T any, P interface { *T json.Unmarshaler }](ctx context.Context, b *Broker, path string, body []byte, onCode func(code string)) (*T, error) { + return postWith[T, P](ctx, b, path, body, onCode, nil) +} + +// postWith is Post with more request headers: extra, if not nil, may set +// them. They are secrets: Post never shows them. +func postWith[T any, P interface { + *T + json.Unmarshaler +}](ctx context.Context, b *Broker, path string, body []byte, onCode func(code string), extra http.Header) (*T, error) { reqCtx, cancel := context.WithTimeout(ctx, b.timeout) defer cancel() tr := &trace{onCode: onCode} @@ -252,6 +262,9 @@ func Post[T any, P interface { req.GetBody = nil req.Close = true req.Header.Set("Content-Type", api.ContentType) + for k, v := range extra { + req.Header[k] = slices.Clone(v) + } b.versions.setVersion(req.Header) defer b.client.CloseIdleConnections() resp, err := b.client.Do(req) diff --git a/internal/client/run.go b/internal/client/run.go index a98ad49..55b6407 100644 --- a/internal/client/run.go +++ b/internal/client/run.go @@ -8,6 +8,7 @@ import ( "fmt" "io" "io/fs" + "net/http" "os" "os/exec" "os/signal" @@ -19,6 +20,7 @@ import ( "unicode" "github.com/code6x/nodkey/internal/api" + "github.com/code6x/nodkey/internal/grantkey" "github.com/code6x/nodkey/internal/transport" ) @@ -35,7 +37,7 @@ const ( ) // runUsage is the help of nodkey run. -const runUsage = `Usage: nodkey run [--secret VAR=NAME]... [--reason TEXT] -- COMMAND [ARGS...] +const runUsage = `Usage: nodkey run [--secret VAR=NAME]... [--grant-token FILE] [--reason TEXT] -- COMMAND [ARGS...] Ask the broker for secrets, and wait until a person approves the request on the phone. Then run COMMAND, with the value of the secret NAME in the @@ -45,6 +47,11 @@ environment variable VAR. nodkey never prints a value. [A-Z_][A-Z0-9_]{0,63}, and must not be a name whose value programs show or run, such as PATH, LD_PRELOAD or GIT_SSH_COMMAND. NAME is a secret of the broker. + --grant-token FILE the token file of a standing grant (nodkey preapprove). + With it, no tap is needed inside a window of the grant. + nodkey replaces the token in FILE with a new one before + it runs COMMAND, and never prints it. If FILE cannot be + saved, COMMAND does not run (exit 77). --reason TEXT why COMMAND needs the secrets. Without it, the reason is the environment variable NODKEY_REASON. @@ -158,6 +165,15 @@ func (r runner) run(ctx context.Context, args []string, w io.Writer, beforeExec if err != nil { return fail(stderr, exitUsage, err.Error()) } + // Check the token file before any request, so a bad file asks no one. + var hdr http.Header + if o.grantToken != "" { + tok, err := readGrantToken(o.grantToken) + if err != nil { + return fail(stderr, exitUsage, err.Error()) + } + hdr = http.Header{api.GrantTokenHeader: {tok.Reveal()}} + } configPath, err := r.configPath() if err != nil { return fail(stderr, exitUnavailable, err.Error()) @@ -173,12 +189,22 @@ func (r runner) run(ctx context.Context, args []string, w io.Writer, beforeExec if err := r.notDumpable(); err != nil { return fail(stderr, exitUnavailable, "cannot mark nodkey as not dumpable: "+err.Error()) } - ans, err := Post[api.SecretsAnswer](ctx, b, api.PathSecrets, body, func(code string) { + // A request with a token is never sent again (postWith sends once). + ans, err := postWith[api.SecretsAnswer](ctx, b, api.PathSecrets, body, func(code string) { _, _ = fmt.Fprintf(stderr, "nodkey: waiting for approval (request %s)\n", code) - }) + }, hdr) if err != nil { return requestFailed(ctx, stderr, err) } + // The new token is saved before the values are used (DESIGN.md section + // 8). If it cannot be, the command does not run. + if err := rotateToken(o.grantToken, ans.GrantToken); err != nil { + ans.Clear() + if errors.Is(err, ErrTokenFile) { + return fail(stderr, exitDenied, err.Error()) // DESIGN.md section 15: exit 77 + } + return fail(stderr, exitUnavailable, err.Error()) + } envv, err := commandEnv(env, o.secrets, ans) ans.Clear() // best effort: envv holds the values as Go strings if err != nil { @@ -275,7 +301,9 @@ type runOptions struct { secrets []api.SecretRef reason string reasonSet bool - command []string + // grantToken is the file of --grant-token ("" for none). + grantToken string + command []string } // parseArgs reads the arguments of nodkey run: the options, "--", and the @@ -306,7 +334,7 @@ func parseArgs(args []string) (*runOptions, error) { } name, value, hasValue := splitOption(arg) switch name { - case "secret", "reason": + case "secret", "reason", "grant-token": case "h", "help": // With other arguments, help would exit with 0 and run // nothing, which looks like a command that ran. @@ -324,6 +352,16 @@ func parseArgs(args []string) (*runOptions, error) { } value, rest = rest[0], rest[1:] } + if name == "grant-token" { + if o.grantToken != "" { + return nil, errors.New("--grant-token is given twice") + } + if value == "" { + return nil, errors.New("--grant-token needs a file") + } + o.grantToken = value + continue + } if name == "reason" { if o.reasonSet { return nil, errors.New("--reason is given twice") @@ -628,3 +666,43 @@ func (in *interrupts) stop() { } }) } + +// readGrantToken checks the token file and reads the token from it. The +// errors never hold the token. +func readGrantToken(path string) (grantkey.Token, error) { + if err := CheckGrantTokenFile(path); err != nil { + return grantkey.Token{}, err + } + data, err := os.ReadFile(path) //nolint:gosec // G304: the path is the one that the person gave. + if err != nil { + return grantkey.Token{}, fmt.Errorf("%w: cannot read %q: %s", ErrTokenFile, path, errText(err)) + } + tok, err := grantkey.Parse(strings.TrimSuffix(string(data), "\n")) + if err != nil { + return grantkey.Token{}, fmt.Errorf("%w: %q does not hold a token", ErrTokenFile, path) + } + return tok, nil +} + +// rotateToken saves the new token raw of the answer to the file path. With +// a file, the answer must have a valid new token. Without one, the answer +// must have none. The errors never hold the token. +func rotateToken(path, raw string) error { + if path == "" { + if raw != "" { + return errors.New("the answer of the broker is not valid: it has a token for a request without one") + } + return nil + } + if raw == "" { + return errors.New("the answer of the broker is not valid: it has no new token; the command does not run") + } + tok, err := grantkey.Parse(raw) + if err != nil { + return errors.New("the answer of the broker is not valid: its new token is not a token; the command does not run") + } + if err := SaveGrantToken(path, tok); err != nil { + return fmt.Errorf("%w; the command does not run", err) + } + return nil +} diff --git a/internal/client/run_grant_test.go b/internal/client/run_grant_test.go new file mode 100644 index 0000000..40c8f97 --- /dev/null +++ b/internal/client/run_grant_test.go @@ -0,0 +1,190 @@ +package client + +import ( + "net/http" + "os" + "path/filepath" + "strings" + "sync" + "testing" + + "github.com/code6x/nodkey/internal/api" +) + +// grantRunBroker answers a secrets request with one value and the token +// newTok ("" for none), and keeps the token header of each request. +type grantRunBroker struct { + *fakeBroker + mu sync.Mutex + headers []string +} + +func newGrantRunBroker(t *testing.T, newTok string) *grantRunBroker { + t.Helper() + gb := &grantRunBroker{} + gb.fakeBroker = newFakeBroker(t, func(w http.ResponseWriter, r *http.Request) { + gb.mu.Lock() + gb.headers = append(gb.headers, r.Header.Get(api.GrantTokenHeader)) + gb.mu.Unlock() + a := api.SecretsAnswer{Secrets: []api.SecretValue{{Var: "GH_TOKEN", Value: []byte("made-up-value")}}, GrantToken: newTok} + body, err := a.Encode() + if err != nil { + t.Error(err) + return + } + answer(http.StatusOK, string(body))(w, r) + }) + return gb +} + +func (gb *grantRunBroker) seen() []string { + gb.mu.Lock() + defer gb.mu.Unlock() + return append([]string(nil), gb.headers...) +} + +func writeToken(t *testing.T, dir, tok string) string { + t.Helper() + f := filepath.Join(dir, "grant") + if err := os.WriteFile(f, []byte(tok+"\n"), 0o600); err != nil { + t.Fatal(err) + } + return f +} + +func runGrant(t *testing.T, gb *grantRunBroker, fx *fakeExec, args ...string) (int, string) { + t.Helper() + var b *Broker + if gb != nil { + b = gb.b + } + return runIn(t.Context(), testRunner(b, nil, fx), + append(append([]string{"--secret", "GH_TOKEN=github-token"}, args...), "--", self(t))...) +} + +// TestRunGrantToken: the header carries the token, and the new token is in +// the file before the command starts. +func TestRunGrantToken(t *testing.T) { + t.Parallel() + oldTok, newTok := newToken(t).Reveal(), newToken(t).Reveal() + f := writeToken(t, t.TempDir(), oldTok) + gb := newGrantRunBroker(t, newTok) + fx := &fakeExec{} + var inFile string + r := testRunner(gb.b, nil, fx) + r.exec = func(path string, argv, envv []string) error { + data, _ := os.ReadFile(f) + inFile = string(data) + return fx.exec(path, argv, envv) + } + code, stderr := runIn(t.Context(), r, "--secret", "GH_TOKEN=github-token", "--grant-token", f, "--", self(t)) + if code != exitOK { + t.Fatalf("exit %d; stderr %q", code, safe(stderr)) + } + if h := gb.seen(); len(h) != 1 || h[0] != oldTok { + t.Errorf("the request had %d token headers, or a wrong one", len(h)) + } + if inFile != newTok+"\n" { + t.Error("the file did not hold the new token when the command started") + } + mustNotLeak(t, oldTok, stderr) + mustNotLeak(t, newTok, stderr) + if len(fx.ran()) != 1 { + t.Error("the command did not start") + } +} + +// TestRunGrantTokenDeny: every failure runs no command. +func TestRunGrantTokenDeny(t *testing.T) { + t.Parallel() + oldTok := newToken(t).Reveal() + for _, tt := range []struct { + name string + newTk func() string + setup func(t *testing.T, dir, f string) + want int + }{ + {"no new token", func() string { return "" }, nil, exitUnavailable}, + {"bad new token", func() string { return "nkg_" + strings.Repeat("a", 10) }, nil, exitUnavailable}, + {"save fails", func() string { return newToken(t).Reveal() }, func(t *testing.T, dir, f string) { + // The directory cannot take a new file; reading still works. + if err := os.Chmod(dir, 0o500); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = os.Chmod(dir, 0o700) }) + }, exitDenied}, + } { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + dir := t.TempDir() + f := writeToken(t, dir, oldTok) + if tt.setup != nil { + tt.setup(t, dir, f) + } + nt := tt.newTk() + gb := newGrantRunBroker(t, nt) + fx := &fakeExec{} + code, stderr := runGrant(t, gb, fx, "--grant-token", f) + if code != tt.want { + t.Errorf("exit %d, want %d; stderr %q", code, tt.want, safe(stderr)) + } + if len(fx.ran()) != 0 { + t.Error("the command started") + } + if n := len(gb.seen()); n != 1 { + t.Errorf("%d requests, want 1 (no retry)", n) + } + mustNotLeak(t, oldTok, stderr) + if nt != "" { + mustNotLeak(t, nt, stderr) + } + if data, _ := os.ReadFile(f); string(data) != oldTok+"\n" { + t.Error("the token file changed") + } + }) + } +} + +// TestRunGrantTokenBadFile: a bad file makes no request. +func TestRunGrantTokenBadFile(t *testing.T) { + t.Parallel() + dir := t.TempDir() + open := filepath.Join(dir, "open") + if err := os.WriteFile(open, []byte(newToken(t).Reveal()+"\n"), 0o600); err != nil { + t.Fatal(err) + } + if err := os.Chmod(open, 0o640); err != nil { + t.Fatal(err) + } + junk := writeToken(t, dir, "not a token") + if err := os.Rename(junk, filepath.Join(dir, "junk")); err != nil { + t.Fatal(err) + } + for name, f := range map[string]string{"missing": filepath.Join(dir, "none"), "mode 0640": open, + "not a token": filepath.Join(dir, "junk"), "a directory": dir} { + t.Run(name, func(t *testing.T) { + t.Parallel() + gb := newGrantRunBroker(t, newToken(t).Reveal()) + fx := &fakeExec{} + code, stderr := runGrant(t, gb, fx, "--grant-token", f) + if code == exitOK || len(fx.ran()) != 0 || len(gb.seen()) != 0 { + t.Errorf("exit %d, %d requests, %d commands; stderr %q", code, len(gb.seen()), len(fx.ran()), safe(stderr)) + } + }) + } +} + +// TestRunTokenWithoutFlag: an answer with a token, for a request without +// one, is refused. +func TestRunTokenWithoutFlag(t *testing.T) { + t.Parallel() + gb := newGrantRunBroker(t, newToken(t).Reveal()) + fx := &fakeExec{} + code, stderr := runGrant(t, gb, fx) + if code != exitUnavailable || len(fx.ran()) != 0 { + t.Errorf("exit %d, %d commands; stderr %q", code, len(fx.ran()), safe(stderr)) + } + if h := gb.seen(); len(h) != 1 || h[0] != "" { + t.Error("the request had a token header, or was sent again") + } +} From cf4b0f806adfcbda992f5d5c4d693e16d791320b Mon Sep 17 00:00:00 2001 From: Geno Roupsky <185580+groupsky@users.noreply.github.com> Date: Thu, 29 Oct 2026 07:28:49 +0200 Subject: [PATCH 08/18] feat(broker): make standing grants (#110) Co-Authored-By: Claude Opus 5.5 --- DESIGN.md | 38 +++- internal/approval/approver.go | 3 + internal/approval/check.go | 4 +- internal/approval/decide.go | 31 +++- internal/approval/export_test.go | 16 ++ internal/approval/grants.go | 36 +++- internal/approval/request.go | 16 ++ internal/approval/standing.go | 135 ++++++++++++++ internal/approval/standing_test.go | 158 ++++++++++++++++ internal/approval/state.go | 4 +- internal/audit/event.go | 44 ++++- internal/audit/event_test.go | 53 ++++++ internal/broker/preapprove.go | 99 +++++++++- internal/broker/standing_test.go | 212 ++++++++++++++++++++++ internal/telegram/preapprove_text_test.go | 105 +++++++++++ internal/telegram/text.go | 66 ++++++- 16 files changed, 990 insertions(+), 30 deletions(-) create mode 100644 internal/approval/standing.go create mode 100644 internal/approval/standing_test.go create mode 100644 internal/broker/standing_test.go diff --git a/DESIGN.md b/DESIGN.md index 19480b1..51bd0e7 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -1485,7 +1485,37 @@ of grants), with the limits of `[standing]` (section 14): 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**. +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 +is: + +``` +🗓 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 +A Deny of other requests does not end this grant. Use Revoke to end it. +— claimed by the agent — +reason: job nightly-backup +expires 14:07:00 (120 s) +[ Approve ] [ Deny ] +``` + +The broker denies the request before any message when `[standing]` is +missing, when a secret has no `standing = true`, when the end is not set +or is more than `max_days` away, when a window is longer than +`max_window`, when the uses in a window are more than `max_uses`, when the +client already has `max_grants` live standing grants, or when a window is +not valid. Windows must not overlap. After **Approve** the broker makes the token and keeps its +hash only (`grant_id`, the windows and the uses in each window go with +it). The audit entry of the decision has the kind `standing`, with +`not_before`, `not_after`, `windows` and `uses`. **The token.** After the tap, the broker makes a token: `nkg_`, 32 random bytes in base62, and a CRC32 checksum, as GitHub does. The broker keeps @@ -1624,10 +1654,12 @@ The fields of each event, after `seq`, `time`, `event` and `prev`: | `log_recovered` | `torn_length` and `torn_sha256`: the length and the SHA-256 of the torn line | The request fields are `request_id`, `client`, `kind` (`secrets`, -`ssh_login`, `ssh_grant` or `preapprove`), `secrets`, `key`, `hosts`, +`ssh_login`, `ssh_grant`, `preapprove` or `standing`), `secrets`, `key`, `hosts`, `host_key` (a SHA256 fingerprint), `remote_user`, and the claimed `command` and `reason`. A `preapprove` request that was approved also has -`grant_id`, `uses`, `not_before` and `not_after` (UTC, RFC 3339); the +`grant_id`, `uses`, `not_before` and `not_after` (UTC, RFC 3339). A +`standing` request that was approved also has `windows` (such as +`daily 03:00-05:00`), and its `uses` are the uses in each window; the entries of a use and of an end have `grant_id`. The broker writes `broker_started` when it has passed every check of its diff --git a/internal/approval/approver.go b/internal/approval/approver.go index 30959ac..6b0c57d 100644 --- a/internal/approval/approver.go +++ b/internal/approval/approver.go @@ -143,6 +143,9 @@ type Prompt struct { // RemoteUser or Secrets say what it covers. Uses int From, Until time.Time + // Windows are the windows of a KindStanding request, such as + // "daily 03:00-05:00". Uses is then the number of uses in each. + Windows []string // GrantID and UsesLeft are set only in the notice that a scheduled // grant ended with uses left: the grant's ID and the uses it had left. GrantID string diff --git a/internal/approval/check.go b/internal/approval/check.go index a9363e6..f903a17 100644 --- a/internal/approval/check.go +++ b/internal/approval/check.go @@ -42,7 +42,7 @@ func (c *Core) check(r *Request, p *pending) bool { ok = false } switch r.Kind { - case audit.KindSecrets, audit.KindSSHLogin, audit.KindSSHGrant, audit.KindPreapprove: + case audit.KindSecrets, audit.KindSSHLogin, audit.KindSSHGrant, audit.KindPreapprove, audit.KindStanding: p.kind = r.Kind p.subject.Kind = r.Kind default: @@ -58,6 +58,8 @@ func (c *Core) check(r *Request, p *pending) bool { ok = c.checkGrant(r, p) && ok case audit.KindPreapprove: ok = c.checkPreapprove(r, p) && ok + case audit.KindStanding: + ok = c.checkStanding(r, p) && ok } return ok } diff --git a/internal/approval/decide.go b/internal/approval/decide.go index 0b1cd8e..8142206 100644 --- a/internal/approval/decide.go +++ b/internal/approval/decide.go @@ -116,9 +116,12 @@ func (p *pending) prompt(now moment) Prompt { if p.kind == audit.KindSSHGrant { pr.GrantEnds = p.grantEnds.wall } - if p.kind == audit.KindPreapprove { + if p.kind.IsPreapprove() { loc := now.wall.Location() pr.Uses, pr.From, pr.Until = p.plan.uses, p.plan.start.In(loc), p.plan.end.wall.In(loc) + if p.kind == audit.KindStanding { + pr.Uses, pr.Windows = p.plan.perWindow, slices.Clone(p.subject.Windows) + } } return pr } @@ -160,7 +163,7 @@ func (c *Core) Decide(ctx context.Context, r *Request) *Result { // grant nothing, so the request ends then too. p.deadline = earlier(p.deadline, p.grantEnds) } - if p.kind == audit.KindPreapprove { + if p.kind.IsPreapprove() { // Its grant ends at a fixed time, as an SSH grant's do: after it, // an approval would grant nothing. end := p.plan.end.wall @@ -199,7 +202,11 @@ func (c *Core) leave(p *pending) { delete(c.ids, p.id) delete(c.codes, p.code) if p.preCounted { - c.client(p.client).preapproving-- + if p.kind == audit.KindStanding { + c.client(p.client).standingPending-- + } else { + c.client(p.client).preapproving-- + } } switch p.state { case stateQueued: @@ -226,13 +233,20 @@ func (c *Core) begin(p *pending) *Result { now := c.now() c.sweepGrants(now) cs := c.client(p.client) - if p.kind == audit.KindPreapprove { + if p.kind.IsPreapprove() { // The grants that are approved and not ended, and the requests // that wait for a tap (StartGrants checks the first again). - if c.scheduledCount(p.client)+cs.preapproving >= c.cfg.Preapprove.MaxGrants { - return c.denied(p, audit.CauseLimit) + if p.kind == audit.KindStanding { + if c.standingCount(p.client)+cs.standingPending >= c.cfg.Standing.MaxGrants { + return c.denied(p, audit.CauseLimit) + } + cs.standingPending++ + } else { + if c.scheduledCount(p.client)+cs.preapproving >= c.cfg.Preapprove.MaxGrants { + return c.denied(p, audit.CauseLimit) + } + cs.preapproving++ } - cs.preapproving++ p.preCounted = true } if vias, ok := c.covered(p, now); ok { @@ -551,7 +565,7 @@ func (c *Core) Answer(a Answer) error { return ErrNotAllowed } res.Allowed = true - if p.kind == audit.KindPreapprove { + if p.kind.IsPreapprove() { c.approvePlan(p, res) } case audit.AnswerApproveWindow: @@ -651,6 +665,7 @@ func (c *Core) approvePlan(p *pending, res *Result) { g.origin = res.Subject g.origin.Secrets = slices.Clone(res.Subject.Secrets) g.origin.Hosts = slices.Clone(res.Subject.Hosts) + g.origin.Windows = slices.Clone(res.Subject.Windows) res.plan = &g res.GrantID = g.id loc := c.clock.Wall().Location() diff --git a/internal/approval/export_test.go b/internal/approval/export_test.go index 705fc19..7a5ca45 100644 --- a/internal/approval/export_test.go +++ b/internal/approval/export_test.go @@ -43,3 +43,19 @@ func (c *Core) Uses(client string, key bool, name string, at time.Time) int { } return c.uses[useKey{client, ref{key: key, name: name}}].on(useDay(rule, at)) } + +// StandingGrants returns the token hash and the number of old hashes of +// each live standing grant of client, with its windows and uses per window. +func (c *Core) StandingGrants(client string) (hashes [][32]byte, oldHashes []int, windows []int, perWindow []int) { + c.mu.Lock() + defer c.mu.Unlock() + for _, g := range c.grants { + if g.standing() && g.client == client { + hashes = append(hashes, g.tokenHash) + oldHashes = append(oldHashes, len(g.oldHashes)) + windows = append(windows, len(g.windows)) + perWindow = append(perWindow, g.perWindow) + } + } + return +} diff --git a/internal/approval/grants.go b/internal/approval/grants.go index c06658a..387d4e2 100644 --- a/internal/approval/grants.go +++ b/internal/approval/grants.go @@ -67,6 +67,11 @@ type grant struct { // the uses in it. A later opening starts the count again. winKey windowKey winUsed int + // tokenHash is the SHA-256 hash of the current token of a standing + // grant, and oldHashes are the hashes of the tokens that it had + // before: one for each use. The grant never holds a token. + tokenHash [32]byte + oldHashes [][32]byte } // grantKind is the kind of a grant (DESIGN.md section 8, "The grant core"). @@ -561,6 +566,18 @@ func (c *Core) scheduledCount(client string) int { return n } +// standingCount returns how many standing grants client has. Core.mu must +// be held. +func (c *Core) standingCount(client string) int { + n := 0 + for _, g := range c.grants { + if g.standing() && g.client == client { + n++ + } + } + return n +} + // findScheduled returns the scheduled grant that p uses now: of the same // client, live, with a use left, and covering every item of p. A request // that has an item outside one grant is not covered by it at all; grants @@ -620,12 +637,23 @@ func (c *Core) startScheduled(r *Result, now moment) { return } c.sweepGrants(now) - if !now.before(g.end) || c.scheduledCount(g.client) >= c.cfg.Preapprove.MaxGrants { + if !now.before(g.end) { return } - for _, ref := range g.refs { - if c.client(g.client).denied[ref] > r.seq { - return // a Deny came after the tap + if g.standing() { + // A Deny of another request does not touch a standing grant + // (DESIGN.md section 8). + if c.standingCount(g.client) >= c.cfg.Standing.MaxGrants { + return + } + } else { + if c.scheduledCount(g.client) >= c.cfg.Preapprove.MaxGrants { + return + } + for _, ref := range g.refs { + if c.client(g.client).denied[ref] > r.seq { + return // a Deny came after the tap + } } } c.grants = append(c.grants, g) diff --git a/internal/approval/request.go b/internal/approval/request.go index 76c8fed..c866337 100644 --- a/internal/approval/request.go +++ b/internal/approval/request.go @@ -45,6 +45,13 @@ type Request struct { Uses int NotBefore, NotAfter time.Time + // Windows and TokenHash are for a KindStanding request, which names + // secrets only. Uses is then the number of uses in each window. + // TokenHash is the SHA-256 hash of the token that the broker made for + // the grant (grantkey.Hash); the core never sees the token. + Windows []WindowSpec + TokenHash [32]byte + // Claim is the claimed text, made safe by the caller (see Claim). Claim Claim @@ -55,6 +62,15 @@ type Request struct { OnWait func(shortCode string) } +// WindowSpec is one daily or weekly window of a KindStanding request, in +// the broker's local time. It does not pass midnight: From is before To, +// and To is at most 24 hours. +type WindowSpec struct { + // Days are the weekdays of a weekly window. Empty means every day. + Days []time.Weekday + From, To time.Duration // time of day +} + // Result is the decision about one request. Anything that is not a clear // approval is a deny. type Result struct { diff --git a/internal/approval/standing.go b/internal/approval/standing.go new file mode 100644 index 0000000..ae1ad43 --- /dev/null +++ b/internal/approval/standing.go @@ -0,0 +1,135 @@ +package approval + +import ( + "fmt" + "slices" + "strings" + "time" + + "github.com/code6x/nodkey/internal/config" +) + +// maxStandingWindows is the most windows of a standing grant. +const maxStandingWindows = 7 // api.MaxWindows + +// MaxStandingWindows is the most windows of a standing grant. +const MaxStandingWindows = maxStandingWindows + +// dayNames are the names of the weekdays, indexed by time.Weekday. +var dayNames = [7]string{"sun", "mon", "tue", "wed", "thu", "fri", "sat"} + +// clockText shows a time of day as "HH:MM". +func clockText(d time.Duration) string { + m := int(d / time.Minute) + return fmt.Sprintf("%02d:%02d", m/60, m%60) +} + +// windowText shows a window as the audit log and the message do: "daily +// 03:00-05:00" or "mon,thu 03:00-05:00". Days are in the order of the +// week, Monday first. +func windowText(w window) string { + days := "daily" + if w.days != anyDay { + var names []string + for _, d := range []time.Weekday{time.Monday, time.Tuesday, time.Wednesday, time.Thursday, + time.Friday, time.Saturday, time.Sunday} { + if w.days.has(d) { + names = append(names, dayNames[d]) + } + } + days = strings.Join(names, ",") + } + return days + " " + clockText(w.from) + "-" + clockText(w.to) +} + +// toWindow converts a spec, and reports whether it is valid. A window with +// days that repeat or are not weekdays is not valid. +func toWindow(s WindowSpec) (window, bool) { + w := window{days: anyDay, from: s.From, to: s.To} + if len(s.Days) > 0 { + w.days = 0 + for _, d := range s.Days { + if d < time.Sunday || d > time.Saturday || w.days.has(d) { + return window{}, false + } + w.days |= 1 << uint(d) + } + } + // Whole minutes only: a message shows exactly what is granted. + return w, w.valid() && w.from%time.Minute == 0 && w.to%time.Minute == 0 +} + +// checkStanding checks a request for a standing grant (DESIGN.md section +// 8): secrets only, each with standing = true in a rule that is not +// "never", the end inside max_days, 1 to 7 valid windows each not longer +// than max_window, and the uses per window inside max_uses. A config without +// [standing] allows nothing. The count of grants is checked in begin. +func (c *Core) checkStanding(r *Request, p *pending) bool { + lim := c.cfg.Standing + ok := lim.Enabled && r.HostKey == nil && r.For == 0 && r.Key == "" && r.RemoteUser == "" && + len(r.Hosts) == 0 && len(r.Secrets) > 0 && p.client != "" && r.TokenHash != [32]byte{} + var refs []ref + seen := make(map[string]bool) + for _, name := range r.Secrets { + if seen[name] { + continue + } + seen[name] = true + s, known := c.cfg.Secrets[name] + if !known || p.client == "" || !slices.Contains(s.Clients, p.client) || + !s.Rule.Standing || s.Rule.Ask == config.AskNever || !validRule(s.Rule, s.Clients) { + ok = false + continue + } + p.items = append(p.items, item{ref: ref{name: name}, rule: s.Rule}) + p.subject.Secrets = append(p.subject.Secrets, name) + refs = append(refs, ref{name: name}) + } + if len(refs) > MaxPreapproveSecrets { + ok = false + } + if len(r.Windows) < 1 || len(r.Windows) > maxStandingWindows { + ok = false + } + var ws []window + var texts []string + for _, spec := range r.Windows { + w, valid := toWindow(spec) + if !valid || w.to-w.from > lim.MaxWindow { + ok = false + continue + } + // Windows must not overlap: a use counts in the first window that + // holds it, so an overlap would give more uses in one span. + if slices.ContainsFunc(ws, func(o window) bool { + return o.days&w.days != 0 && o.from < w.to && w.from < o.to + }) { + ok = false + continue + } + ws = append(ws, w) + texts = append(texts, windowText(w)) + } + // The times are whole seconds. The end is after now and not more than + // max_days away, and the start is before the end. + if r.Uses < 1 || r.Uses > lim.MaxUses || r.NotBefore.IsZero() || r.NotAfter.IsZero() || + r.NotBefore.Nanosecond() != 0 || r.NotAfter.Nanosecond() != 0 || + !r.NotBefore.Before(r.NotAfter) || !p.start.wall.Before(r.NotAfter) || + r.NotAfter.Sub(p.start.wall) > time.Duration(lim.MaxDays)*24*time.Hour { + ok = false + } + if !ok || len(refs) == 0 || len(ws) != len(r.Windows) { + return false + } + p.once = true // the one "Approve" button + p.subject.Uses = r.Uses + p.subject.NotBefore = r.NotBefore.UTC().Format(time.RFC3339) + p.subject.NotAfter = r.NotAfter.UTC().Format(time.RFC3339) + p.subject.Windows = texts + p.plan = &grant{ + kind: kindStanding, client: p.client, refs: refs, + start: r.NotBefore, end: moment{wall: r.NotAfter}, uses: noLimit, + windows: ws, perWindow: r.Uses, tokenHash: r.TokenHash, oldHashes: [][32]byte{}, + } + return true +} diff --git a/internal/approval/standing_test.go b/internal/approval/standing_test.go new file mode 100644 index 0000000..646205a --- /dev/null +++ b/internal/approval/standing_test.go @@ -0,0 +1,158 @@ +package approval_test + +// Tests of the request for a standing grant (DESIGN.md section 8). Times +// are relative to noon, the start of the fake clock. + +import ( + "testing" + "testing/synctest" + "time" + + "github.com/code6x/nodkey/internal/approval" + "github.com/code6x/nodkey/internal/audit" + "github.com/code6x/nodkey/internal/config" + "github.com/code6x/nodkey/internal/grantkey" +) + +// standEnv starts a core with [standing] on and standing = true on the +// secrets github-token and npm-token. +func standEnv(t *testing.T, edit ...func(*config.Broker)) *env { + t.Helper() + return newEnv(t, func(cfg *config.Broker) { + cfg.Standing = config.DefaultStanding() + cfg.Standing.MaxUses = 3 + for _, name := range []string{"github-token", "npm-token"} { + s := cfg.Secrets[name] + s.Rule.Standing = true + cfg.Secrets[name] = s + } + for _, f := range edit { + f(cfg) + } + }) +} + +func night() []approval.WindowSpec { + return []approval.WindowSpec{{From: 3 * time.Hour, To: 5 * time.Hour}} +} + +// standReq asks for a standing grant for 30 days from noon, with the hash +// of tok. +func standReq(client string, tok grantkey.Token, names ...string) *approval.Request { + return &approval.Request{ + Client: client, Kind: audit.KindStanding, Secrets: names, Uses: 2, Windows: night(), + NotBefore: noon, NotAfter: noon.Add(30 * 24 * time.Hour), TokenHash: grantkey.Hash(tok), + Claim: approval.Claim{Reason: "job nightly-backup"}, + } +} + +func newToken(t *testing.T) grantkey.Token { + t.Helper() + tok, err := grantkey.New() + if err != nil { + t.Fatal(err) + } + return tok +} + +// TestStandingGrantKeepsOnlyTheHash: an approved request makes a standing +// grant that holds the hash of the token, no old hashes, and the windows. +func TestStandingGrantKeepsOnlyTheHash(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + e := standEnv(t) + defer e.stop() + tok := newToken(t) + res := e.grant(standReq("laptop", tok, "github-token"), 1) + p := e.fake.Asked()[0].Prompt + if p.Kind != audit.KindStanding || p.Uses != 2 || len(p.Windows) != 1 || p.Windows[0] != "daily 03:00-05:00" || !p.Once { + t.Errorf("the prompt is %+v", p) + } + if res.Subject.Kind != audit.KindStanding || res.Subject.Uses != 2 || len(res.Subject.Windows) != 1 { + t.Errorf("the subject is %+v", res.Subject) + } + hashes, old, ws, per := e.core.StandingGrants("laptop") + if len(hashes) != 1 || hashes[0] != grantkey.Hash(tok) || old[0] != 0 || ws[0] != 1 || per[0] != 2 { + t.Errorf("the grant: %d hashes, %v old, %v windows, %v per window", len(hashes), old, ws, per) + } + }) +} + +// TestStandingRefused: each check is a deny without a message. +func TestStandingRefused(t *testing.T) { + tests := []struct { + name string + edit func(*approval.Request) + cfg func(*config.Broker) + }{ + {"no [standing]", nil, func(c *config.Broker) { c.Standing = config.StandingLimits{} }}, + {"a rule without standing", func(r *approval.Request) { r.Secrets = []string{"db-password"} }, nil}, + {"a never rule", func(r *approval.Request) { r.Secrets = []string{"backup-token"}; r.Client = "ci" }, nil}, + {"a key", func(r *approval.Request) { r.Key, r.RemoteUser, r.Hosts = "web", "deploy", []string{"web-1"} }, nil}, + {"no windows", func(r *approval.Request) { r.Windows = nil }, nil}, + {"8 windows", func(r *approval.Request) { + r.Windows = nil + for range 8 { + r.Windows = append(r.Windows, night()[0]) + } + }, nil}, + {"no token hash", func(r *approval.Request) { r.TokenHash = [32]byte{} }, nil}, + {"over max_uses", func(r *approval.Request) { r.Uses = 4 }, nil}, + {"no uses", func(r *approval.Request) { r.Uses = 0 }, nil}, + {"over max_days", func(r *approval.Request) { r.NotAfter = noon.Add(91 * 24 * time.Hour) }, nil}, + {"no end", func(r *approval.Request) { r.NotAfter = time.Time{} }, nil}, + {"the end has passed", func(r *approval.Request) { r.NotBefore = noon.Add(-2 * time.Hour); r.NotAfter = noon.Add(-time.Hour) }, nil}, + {"the end is before the start", func(r *approval.Request) { r.NotBefore = noon.Add(40 * 24 * time.Hour) }, nil}, + {"part of a second", func(r *approval.Request) { r.NotAfter = r.NotAfter.Add(time.Millisecond) }, nil}, + {"a window over max_window", func(r *approval.Request) { + r.Windows = []approval.WindowSpec{{From: time.Hour, To: 9*time.Hour + time.Minute}} + }, nil}, + {"a window that is backwards", func(r *approval.Request) { + r.Windows = []approval.WindowSpec{{From: 5 * time.Hour, To: 3 * time.Hour}} + }, nil}, + {"a window past midnight", func(r *approval.Request) { + r.Windows = []approval.WindowSpec{{From: 20 * time.Hour, To: 25 * time.Hour}} + }, nil}, + {"a window with part of a minute", func(r *approval.Request) { + r.Windows = []approval.WindowSpec{{From: time.Hour, To: 2*time.Hour + time.Second}} + }, nil}, + {"windows that overlap", func(r *approval.Request) { + r.Windows = []approval.WindowSpec{{From: 3 * time.Hour, To: 5 * time.Hour}, {From: 4 * time.Hour, To: 6 * time.Hour}} + }, nil}, + {"a window twice", func(r *approval.Request) { r.Windows = append(night(), night()...) }, nil}, + {"a bad day", func(r *approval.Request) { + r.Windows = []approval.WindowSpec{{Days: []time.Weekday{7}, From: time.Hour, To: 2 * time.Hour}} + }, nil}, + {"a day twice", func(r *approval.Request) { + r.Windows = []approval.WindowSpec{{Days: []time.Weekday{time.Monday, time.Monday}, From: time.Hour, To: 2 * time.Hour}} + }, nil}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + e := standEnv(t, func(c *config.Broker) { + if tt.cfg != nil { + tt.cfg(c) + } + }) + defer e.stop() + r := standReq("laptop", newToken(t), "github-token") + if tt.edit != nil { + tt.edit(r) + } + e.refuses(r, audit.CauseNotAllowed) + }) + }) + } +} + +// TestStandingMaxGrants: a client with max_grants live standing grants +// gets a deny with cause limit for the next, before any message. +func TestStandingMaxGrants(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + e := standEnv(t, func(c *config.Broker) { c.Standing.MaxGrants = 2 }) + defer e.stop() + e.grant(standReq("laptop", newToken(t), "github-token"), 1) + e.grant(standReq("laptop", newToken(t), "npm-token"), 2) + e.refuses(standReq("laptop", newToken(t), "github-token"), audit.CauseLimit) + }) +} diff --git a/internal/approval/state.go b/internal/approval/state.go index 44685d4..685bf4d 100644 --- a/internal/approval/state.go +++ b/internal/approval/state.go @@ -163,6 +163,8 @@ type clientState struct { // preapproving counts the preapprove requests that wait for a tap or // are being decided. preapproving int + // standingPending is the same for standing grants. + standingPending int // denied holds the number of the newest Deny for each secret or key // (see Core.decisions). A grant from an earlier tap does not start. denied map[ref]uint64 @@ -313,7 +315,7 @@ func (c *Core) neverApplies(client string, it item, now moment) bool { // scheduled grant is never covered: it asks for new grants. A scheduled // grant is not looked at here (findScheduled). func (c *Core) covered(p *pending, now moment) ([]audit.Via, bool) { - if p.kind == audit.KindSSHGrant || p.kind == audit.KindPreapprove || len(p.items) == 0 { + if p.kind == audit.KindSSHGrant || p.kind.IsPreapprove() || len(p.items) == 0 { return nil, false } vias := make([]audit.Via, len(p.items)) diff --git a/internal/audit/event.go b/internal/audit/event.go index 136de49..cbdde4e 100644 --- a/internal/audit/event.go +++ b/internal/audit/event.go @@ -38,6 +38,9 @@ const ( // KindPreapprove is nodkey preapprove: a scheduled grant for later // (DESIGN.md section 8). KindPreapprove Kind = "preapprove" + // KindStanding is nodkey preapprove with daily or weekly windows: a + // standing grant (DESIGN.md section 8). + KindStanding Kind = "standing" ) // Answer is the button the approver pressed. @@ -62,6 +65,7 @@ const ( ViaGrant Via = "grant" // a grant from an "Approve N min" tap or from nodkey ssh-grant ViaNeverRule Via = "never_rule" // a rule with ask = "never" ViaScheduled Via = "scheduled" // a scheduled grant from nodkey preapprove + ViaStanding Via = "standing" // a standing grant from nodkey preapprove with windows ) // Cause says why a request was denied. @@ -88,6 +92,8 @@ const ( EndDenied EndCause = "denied" // the person denied a later request for it EndStopped EndCause = "stopped" // the broker stopped EndRevoked EndCause = "revoked" // the person pressed Revoke + EndAlarm EndCause = "alarm" // a standing grant met an old token or too many uses (DESIGN.md section 8) + EndRenewed EndCause = "renewed" // a standing grant was replaced by its renewal ) // TapCause says why a button press did not count (DESIGN.md section 7). @@ -170,8 +176,19 @@ type Subject struct { Uses int `json:"uses,omitempty"` NotBefore string `json:"not_before,omitempty"` NotAfter string `json:"not_after,omitempty"` + // Windows are the windows of a KindStanding request, such as + // "daily 03:00-05:00" or "mon,thu 03:00-05:00", in the broker's local + // time. Uses is then the number of uses in each window. + Windows []string `json:"windows,omitempty"` } +// IsPreapprove reports whether k is a request for a grant that nodkey +// preapprove makes: a scheduled grant or a standing grant. +func (k Kind) IsPreapprove() bool { return k == KindPreapprove || k == KindStanding } + +// maxWindowEntries is the most windows in a standing request (api.MaxWindows). +const maxWindowEntries = 7 + // BrokerStarted is the first entry of each broker run. type BrokerStarted struct { // Version is the nodkey version, such as "1.2.0" or "dev". @@ -319,6 +336,10 @@ var ( // namePattern is the name rule of DESIGN.md section 14, for clients, // secrets, keys and known hosts. namePattern = regexp.MustCompile(`^[A-Za-z0-9][A-Za-z0-9._-]{0,63}$`) + + // windowPattern is the form of a window in a standing request: + // "daily 03:00-05:00" or "mon,thu 03:00-05:00". + windowPattern = regexp.MustCompile(`^(daily|(mon|tue|wed|thu|fri|sat|sun)(,(mon|tue|wed|thu|fri|sat|sun)){0,6}) ([01][0-9]|2[0-3]):[0-5][0-9]-([01][0-9]|2[0-3]):[0-5][0-9]$`) // remoteUserPattern is the rule for remote user names in DESIGN.md // section 6. remoteUserPattern = regexp.MustCompile(`^[A-Za-z0-9._][A-Za-z0-9._-]{0,31}$`) @@ -363,12 +384,27 @@ func (s *Subject) validate(needRequest bool) error { if s.HostKey != "" || (len(s.Secrets) > 0 && s.Key != "") { return errors.New("a preapprove request names secrets or one key, not both, and has no single host key") } + case KindStanding: + if s.HostKey != "" || s.Key != "" || len(s.Hosts) > 0 || s.RemoteUser != "" { + return errors.New("a standing request names secrets only") + } default: return errors.New("kind is not known") } - if s.Kind != KindPreapprove && (s.Uses != 0 || s.NotBefore != "" || s.NotAfter != "") { + if !s.Kind.IsPreapprove() && (s.Uses != 0 || s.NotBefore != "" || s.NotAfter != "") { return errors.New("only a preapprove request has uses and a time span") } + if s.Kind != KindStanding && len(s.Windows) > 0 { + return errors.New("only a standing request has windows") + } + if len(s.Windows) > maxWindowEntries { + return errors.New("too many windows") + } + for _, w := range s.Windows { + if !windowPattern.MatchString(w) { + return errors.New("a window must look like \"daily 03:00-05:00\"") + } + } if s.GrantID != "" && !requestIDPattern.MatchString(s.GrantID) { return errors.New("grant_id must be 16 bytes in base64url without padding") } @@ -445,7 +481,7 @@ func (e GrantUsed) validate() error { return err } switch e.Via { - case ViaGrant, ViaNeverRule, ViaScheduled: + case ViaGrant, ViaNeverRule, ViaScheduled, ViaStanding: return nil } return errors.New("via is not known") @@ -455,11 +491,11 @@ func (e GrantEnded) validate() error { if err := e.Subject.validate(true); err != nil { return err } - if e.Kind != KindPreapprove || e.GrantID == "" || e.UsesLeft < 0 { + if !e.Kind.IsPreapprove() || e.GrantID == "" || e.UsesLeft < 0 { return errors.New("it needs a preapprove request, a grant_id and a number of uses left") } switch e.Cause { - case EndUsedUp, EndEnded, EndCancelled, EndDenied, EndStopped, EndRevoked: + case EndUsedUp, EndEnded, EndCancelled, EndDenied, EndStopped, EndRevoked, EndAlarm, EndRenewed: return nil } return errors.New("cause is not known") diff --git a/internal/audit/event_test.go b/internal/audit/event_test.go index ea2e08b..2e08b96 100644 --- a/internal/audit/event_test.go +++ b/internal/audit/event_test.go @@ -186,3 +186,56 @@ func TestPreapproveEvents(t *testing.T) { bad("secrets and a key", Request{Subject: both}) bad("via that is not known", GrantUsed{Subject: secretsSubject(), Via: "scheduled2"}) } + +// TestStandingEvents: the kind standing, its windows, and the end causes +// alarm and renewed are accepted; what a standing request may not hold is +// refused. +func TestStandingEvents(t *testing.T) { + good := func(name string, ev Event) { + t.Helper() + if _, _, err := encodeEvent(ev); err != nil { + t.Errorf("%s: %v", name, err) + } + } + bad := func(name string, ev Event) { + t.Helper() + if _, _, err := encodeEvent(ev); err == nil { + t.Errorf("%s: the event was accepted", name) + } + } + ok := preapproveSubject() + ok.Kind = KindStanding + ok.Windows = []string{"daily 03:00-05:00", "mon,thu 10:30-11:00"} + good("a standing request", Request{Subject: ok}) + good("a standing decision", Decision{Subject: ok, UserID: 1, Answer: AnswerApproveOnce}) + good("a standing use", GrantUsed{Subject: ok, Via: ViaStanding}) + for _, c := range []EndCause{EndAlarm, EndRenewed, EndRevoked, EndStopped, EndEnded} { + good("grant_ended "+string(c), GrantEnded{Subject: ok, Cause: c}) + } + // The new causes are for grants; a scheduled grant accepts them too in + // the log, because the cause is checked, not the kind. + withKey := ok + withKey.Key = "deploy" + bad("a key in a standing request", Request{Subject: withKey}) + withUser := ok + withUser.RemoteUser = "deploy" + bad("a remote user in a standing request", Request{Subject: withUser}) + for _, w := range []string{"", "daily", "daily 3:00-5:00", "daily 03:00-24:00", "xyz 03:00-05:00", "mon,mon,mon,mon,mon,mon,mon,mon 03:00-05:00", + "daily 03:00-05:00\nA", "daily 03:00-05:00 ", "weekly 03:00-05:00"} { + s := ok + s.Windows = []string{w} + bad("the window "+w, Request{Subject: s}) + } + many := ok + many.Windows = make([]string, 8) + for i := range many.Windows { + many.Windows[i] = "daily 03:00-05:00" + } + bad("8 windows", Request{Subject: many}) + sched := preapproveSubject() + sched.Windows = []string{"daily 03:00-05:00"} + bad("windows in a scheduled request", Request{Subject: sched}) + sec := secretsSubject() + sec.Windows = []string{"daily 03:00-05:00"} + bad("windows in a secrets request", Request{Subject: sec}) +} diff --git a/internal/broker/preapprove.go b/internal/broker/preapprove.go index 5432d0a..af2337c 100644 --- a/internal/broker/preapprove.go +++ b/internal/broker/preapprove.go @@ -14,6 +14,7 @@ import ( "github.com/code6x/nodkey/internal/api" "github.com/code6x/nodkey/internal/approval" "github.com/code6x/nodkey/internal/audit" + "github.com/code6x/nodkey/internal/grantkey" "github.com/code6x/nodkey/internal/sshauth" "github.com/code6x/nodkey/internal/transport" ) @@ -32,7 +33,17 @@ func (rq *request) preapprove(ctx context.Context, r *http.Request) { return } cl := approval.Claim{Reason: freeText(req.Reason, maxReasonRunes)} - sub, from, until, ok := rq.s.checkPreapprove(rq.client, req) + standing := len(req.Windows) > 0 + var sub audit.Subject + var from, until time.Time + var ok bool + var specs []approval.WindowSpec + if standing { + rq.kind = audit.KindStanding + sub, from, until, specs, ok = rq.s.checkStanding(rq.client, req) + } else { + sub, from, until, ok = rq.s.checkPreapprove(rq.client, req) + } sub.Reason = cl.Reason rq.pre = sub rq.releaseBody() @@ -40,10 +51,24 @@ func (rq *request) preapprove(ctx context.Context, r *http.Request) { rq.deny(sub, audit.CauseNotAllowed, "", http.StatusForbidden) return } - res, _, ok := rq.decide(ctx, &approval.Request{ + ar := &approval.Request{ Client: rq.client, Kind: audit.KindPreapprove, Secrets: sub.Secrets, Key: sub.Key, RemoteUser: sub.RemoteUser, Hosts: sub.Hosts, Uses: int(req.Uses), NotBefore: from, NotAfter: until, Claim: cl, - }) + } + var token grantkey.Token + if standing { + // The broker makes the token now; the core keeps its hash only. + // The token goes into the answer once, and nowhere else. + var err error + if token, err = grantkey.New(); err != nil { + rq.s.log.Error("broker: cannot make a grant token; the request is denied") + rq.recordDenied(sub, audit.CauseError, audit.CodeApproverFailed, false) + rq.answer(http.StatusServiceUnavailable) + return + } + ar.Kind, ar.Windows, ar.TokenHash = audit.KindStanding, specs, grantkey.Hash(token) + } + res, _, ok := rq.decide(ctx, ar) if !ok { return } @@ -83,9 +108,13 @@ func (rq *request) preapprove(ctx context.Context, r *http.Request) { return } rq.answered = true - err = transport.WriteJSON(rq.w, http.StatusOK, api.PreapproveAnswer{ + ans := api.PreapproveAnswer{ ID: res.GrantID, From: res.GrantFrom.Format(time.RFC3339), Until: res.GrantUntil.Format(time.RFC3339), - }) + } + if standing { + ans.GrantToken = token.Reveal() + } + err = transport.WriteJSON(rq.w, http.StatusOK, ans) if err == nil { // writeBody does not flush; a hang-up shows here, not at Write. if err = http.NewResponseController(rq.w).Flush(); errors.Is(err, http.ErrNotSupported) { @@ -99,6 +128,66 @@ func (rq *request) preapprove(ctx context.Context, r *http.Request) { } } +// checkStanding checks the names, the times and the windows of a request +// for a standing grant: secrets only, each in the config and allowed for +// client. From and until are read as for a scheduled grant. The approval +// core checks the rules, the limits and the windows against [standing]. +func (s *Service) checkStanding(client string, req *api.PreapproveRequest) (sub audit.Subject, from, until time.Time, specs []approval.WindowSpec, ok bool) { + ok = req.Uses >= 1 && req.Uses <= maxPreapproveUses && len(req.Secrets) > 0 && + req.Key == "" && req.User == "" && len(req.Hosts) == 0 + var err1, err2 error + from, err1 = api.ParseTime(req.From, time.Local) + until, err2 = api.ParseTime(req.Until, time.Local) + if err1 != nil || err2 != nil { + ok = false + } + seen := make(map[string]bool, len(req.Secrets)) + for _, name := range req.Secrets { + if seen[name] { + continue + } + seen[name] = true + sec, known := s.cfg.Secrets[name] + if !known || !slices.Contains(sec.Clients, client) { + ok = false + continue + } + sub.Secrets = append(sub.Secrets, sec.Name) + } + for _, w := range req.Windows { + spec, valid := windowSpec(w) + ok = ok && valid + specs = append(specs, spec) + } + return sub, from, until, specs, ok +} + +// windowSpec reads a window of the request. It reports false for a window +// that does not have the form that api.Window.Check allows. +func windowSpec(w api.Window) (approval.WindowSpec, bool) { + from, ok1 := api.ParseClock(w.From) + to, ok2 := api.ParseClock(w.To) + spec := approval.WindowSpec{From: time.Duration(from) * time.Minute, To: time.Duration(to) * time.Minute} + ok := ok1 && ok2 + switch w.Kind { + case api.WindowDaily: + ok = ok && len(w.Days) == 0 + case api.WindowWeekly: + ok = ok && len(w.Days) > 0 + for _, d := range w.Days { + i := slices.Index(api.Weekdays[:], d) + if i < 0 { + ok = false + continue + } + spec.Days = append(spec.Days, time.Weekday((i+1)%7)) // api.Weekdays starts on Monday + } + default: + ok = false + } + return spec, ok +} + // dropGrant cancels the grant id of this client in the core, and writes its // grant_ended entry, as cancelGrant does. func (rq *request) dropGrant(id string) { diff --git a/internal/broker/standing_test.go b/internal/broker/standing_test.go new file mode 100644 index 0000000..6529840 --- /dev/null +++ b/internal/broker/standing_test.go @@ -0,0 +1,212 @@ +package broker + +import ( + "bytes" + "net/http" + "strings" + "testing" + "time" + + "github.com/code6x/nodkey/internal/api" + "github.com/code6x/nodkey/internal/audit" + "github.com/code6x/nodkey/internal/config" + "github.com/code6x/nodkey/internal/grantkey" +) + +// standingOn turns on [standing] with the default limits and standing = true +// for the secrets github-token and npm-token. +func standingOn(cfg *config.Broker) { + cfg.Standing = config.DefaultStanding() + cfg.Standing.MaxUses = 3 + for _, name := range []string{"github-token", "npm-token"} { + s := cfg.Secrets[name] + s.Rule.Standing = true + cfg.Secrets[name] = s + } +} + +// standReq is a request for a standing grant for the secrets: daily from +// 03:00 to 05:00, 2 uses in each window, until 2026-11-14. +func standReq(names ...string) api.PreapproveRequest { + return api.PreapproveRequest{ + Secrets: names, From: "2026-10-14T12:00:00Z", Until: "2026-11-14T00:00:00Z", Uses: 2, + Reason: "job nightly-backup", + Windows: []api.Window{{Kind: api.WindowDaily, From: "03:00", To: "05:00"}}, + } +} + +// TestStandingGrantIsMade: the message, the answer with a token, the +// entries, and the token nowhere but in the answer. +func TestStandingGrantIsMade(t *testing.T) { + e := newEnv(t, envOptions{config: standingOn}) + c := e.httpClient("laptop") + r := standReq("github-token", "npm-token") + r.Windows = append(r.Windows, api.Window{Kind: api.WindowWeekly, Days: []string{"thu", "mon"}, From: "10:30", To: "11:00"}) + p := e.start(c, api.PathPreapprove, r) + pr := e.waitAsked(1)[0].Prompt + if pr.Kind != audit.KindStanding || strings.Join(pr.Secrets, ",") != "github-token,npm-token" || pr.Uses != 2 || + !pr.Once || pr.WindowMinutes != 0 || !pr.From.Equal(noon) || !pr.Until.Equal(time.Date(2026, 11, 14, 0, 0, 0, 0, time.UTC)) || + strings.Join(pr.Windows, "|") != "daily 03:00-05:00|mon,thu 10:30-11:00" || pr.Claim.Reason != "job nightly-backup" { + t.Errorf("the message shows %+v", pr) + } + p.waitCode(t) + e.tap(pr.RequestID, audit.AnswerApproveOnce) + res := p.wait(t) + mustStatus(t, res, http.StatusOK) + ans, err := api.ReadAnswer[api.PreapproveAnswer](bytes.NewReader(res.body)) + if err != nil || len(ans.ID) != 22 { + t.Fatalf("the answer was not read: %v", err) + } + tok, err := grantkey.Parse(ans.GrantToken) + if err != nil { + t.Fatalf("the token does not parse: %v", err) + } + events := requestEvents(e.audit.waitFor(t, 2), pr.RequestID) + mustEvents(t, events, "request decision:approve_once") + d := events[1].(audit.Decision) + if d.Kind != audit.KindStanding || d.Uses != 2 || d.NotBefore != "2026-10-14T12:00:00Z" || d.NotAfter != "2026-11-14T00:00:00Z" || + strings.Join(d.Windows, "|") != "daily 03:00-05:00|mon,thu 10:30-11:00" || d.GrantID != ans.ID { + t.Errorf("the decision entry is %+v", d.Subject) + } + for name, text := range map[string]string{ + "the audit log": encodeEvents(t, e.audit.all()), "the approver": e.approverTexts(), + "the logs": e.logs.String(), "the notices": strings.Join(e.notices.wentOut(), "\n"), + } { + if strings.Contains(text, tok.Reveal()) || strings.Contains(text, ans.GrantToken) { + t.Errorf("the token is in %s", name) + } + } + if n := e.main.fetched(); n != 0 { + t.Errorf("a standing grant fetched %d values", n) + } + // The scheduled grants are not touched: this grant is no scheduled one. + if _, ok := e.core.CancelGrant("laptop", ans.ID); ok { + t.Error("the cancel of a scheduled grant ended a standing grant") + } +} + +// TestStandingRefusals: each check is a deny, with no message and no grant. +func TestStandingRefusals(t *testing.T) { + e := newEnv(t, envOptions{config: func(cfg *config.Broker) { + standingOn(cfg) + s := cfg.Secrets["npm-token"] + s.Rule.Standing = false + cfg.Secrets["npm-token"] = s + }}) + c := e.httpClient("laptop") + with := func(change func(*api.PreapproveRequest)) api.PreapproveRequest { + r := standReq("github-token") + change(&r) + return r + } + win := func(kind, from, to string, days ...string) []api.Window { + return []api.Window{{Kind: kind, Days: days, From: from, To: to}} + } + tests := []struct { + name string + r api.PreapproveRequest + }{ + {"a secret without standing = true", standReq("npm-token")}, + {"one secret without standing = true", standReq("github-token", "npm-token")}, + {"a never rule", standReq("backup-token")}, + {"an unknown secret", standReq("nope")}, + {"no secret", standReq()}, + {"a key", with(func(r *api.PreapproveRequest) { r.Key, r.User, r.Hosts = "web", "deploy", []string{"web-1"} })}, + {"no uses", with(func(r *api.PreapproveRequest) { r.Uses = 0 })}, + {"too many uses", with(func(r *api.PreapproveRequest) { r.Uses = 4 })}, + {"the end after max_days", with(func(r *api.PreapproveRequest) { r.Until = "2027-01-12T12:00:01Z" })}, + {"the end in the past", with(func(r *api.PreapproveRequest) { r.Until = "2026-10-14T11:00:00Z"; r.From = "2026-10-14T10:00:00Z" })}, + {"the end before the start", with(func(r *api.PreapproveRequest) { r.From = "2026-12-01T00:00:00Z" })}, + {"no end", with(func(r *api.PreapproveRequest) { r.Until = "" })}, + {"a window longer than max_window", with(func(r *api.PreapproveRequest) { r.Windows = win(api.WindowDaily, "03:00", "11:01") })}, + {"a window that is backwards", with(func(r *api.PreapproveRequest) { r.Windows = win(api.WindowDaily, "05:00", "03:00") })}, + {"a window of one clock time", with(func(r *api.PreapproveRequest) { r.Windows = win(api.WindowDaily, "05:00", "05:00") })}, + {"a window with a bad kind", with(func(r *api.PreapproveRequest) { r.Windows = win("monthly", "03:00", "05:00") })}, + {"a weekly window with no day", with(func(r *api.PreapproveRequest) { r.Windows = win(api.WindowWeekly, "03:00", "05:00") })}, + {"a weekly window with a bad day", with(func(r *api.PreapproveRequest) { r.Windows = win(api.WindowWeekly, "03:00", "05:00", "xyz") })}, + {"a daily window with days", with(func(r *api.PreapproveRequest) { r.Windows = win(api.WindowDaily, "03:00", "05:00", "mon") })}, + {"a window that is not a time", with(func(r *api.PreapproveRequest) { r.Windows = win(api.WindowDaily, "3am", "05:00") })}, + } + for _, tt := range tests { + before := len(e.audit.all()) + res := e.post(t.Context(), c, api.PathPreapprove, tt.r) + if res.err != nil || res.status != http.StatusForbidden && res.status != http.StatusBadRequest || res.code != "" { + t.Errorf("%s: %d, %v; want a deny without a wait", tt.name, res.status, res.err) + continue + } + if res.status == http.StatusBadRequest { + continue // the protocol refused the body before any check + } + got := e.audit.waitFor(t, before+2)[before:] + mustEvents(t, got, "request denied:not_allowed") + if k := subjectOf(got[0]).Kind; k != audit.KindStanding { + t.Errorf("%s: kind %q", tt.name, k) + } + } + if len(e.approver.Asked()) != 0 { + t.Error("a refused request was shown") + } +} + +// TestStandingOffByDefault: without [standing], no standing grant. +func TestStandingOffByDefault(t *testing.T) { + e := newEnv(t, envOptions{config: func(cfg *config.Broker) { + standingOn(cfg) + cfg.Standing = config.StandingLimits{} + }}) + mustStatus(t, e.post(t.Context(), e.httpClient("laptop"), api.PathPreapprove, standReq("github-token")), http.StatusForbidden) + if len(e.approver.Asked()) != 0 { + t.Error("a message was sent") + } +} + +// TestStandingMaxGrants: a client with max_grants live grants gets a deny +// for the next one, before any message. +func TestStandingMaxGrants(t *testing.T) { + e := newEnv(t, envOptions{config: func(cfg *config.Broker) { + standingOn(cfg) + cfg.Standing.MaxGrants = 2 + }}) + c := e.httpClient("laptop") + e.preapprove(c, standReq("github-token"), 1) + e.preapprove(c, standReq("npm-token"), 2) + before := len(e.audit.all()) + res := e.post(t.Context(), c, api.PathPreapprove, standReq("github-token")) + mustStatus(t, res, http.StatusTooManyRequests) + mustEvents(t, e.audit.waitFor(t, before+2)[before:], "request denied:limit") + if n := len(e.approver.Asked()); n != 2 { + t.Errorf("%d messages, want 2", n) + } +} + +// TestStandingRevokeAndStop: Revoke ends a +// standing grant; a stop drops one and lists it in the stop notice. +func TestStandingRevokeAndStop(t *testing.T) { + e := newEnv(t, envOptions{config: standingOn}) + c := e.httpClient("laptop") + revoked := e.preapprove(c, standReq("github-token"), 1) + before := len(e.audit.all()) + id := e.waitAsked(1)[0].Prompt.RequestID + e.tap(id, audit.AnswerRevoke) + events := e.audit.waitFor(t, before+1)[before:] + mustEvents(t, events, "grant_ended:revoked") + if end := events[0].(audit.GrantEnded); end.GrantID != revoked.ID || end.Kind != audit.KindStanding { + t.Errorf("entry %+v", end) + } + + kept := e.preapprove(c, standReq("npm-token"), 2) + e.stopAndWait() + sent := e.notices.wentOut() + if len(sent) != 1 || !strings.Contains(sent[0], kept.ID+": laptop, npm-token") || strings.Contains(sent[0], revoked.ID) { + t.Errorf("the stop notice: %q", sent) + } + var stopped bool + for _, ev := range e.audit.all() { + if g, ok := ev.(audit.GrantEnded); ok && g.GrantID == kept.ID && g.Cause == audit.EndStopped { + stopped = true + } + } + if !stopped { + t.Error("no grant_ended:stopped for the standing grant") + } +} diff --git a/internal/telegram/preapprove_text_test.go b/internal/telegram/preapprove_text_test.go index 4ce98e2..a3f6380 100644 --- a/internal/telegram/preapprove_text_test.go +++ b/internal/telegram/preapprove_text_test.go @@ -153,3 +153,108 @@ func TestScheduledNotice(t *testing.T) { t.Error("a notice with an unknown via was accepted") } } + +// 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 \ +// --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 +A Deny of other requests does not end this grant. Use Revoke to end it. +— claimed by the agent — +reason: job nightly-backup +expires 14:07:00 (120 s) +[ Approve ] [ Deny ]` + +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.Until = time.Date(2026, 12, 31, 0, 0, 0, 0, time.UTC) + return p +} + +// TestStandingMessageAsInDesign: the message of a standing grant is +// exactly the example of DESIGN.md, which still shows it. +func TestStandingMessageAsInDesign(t *testing.T) { + doc, err := os.ReadFile(filepath.Join("..", "..", "DESIGN.md")) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(string(doc), "```\n"+designStanding+"\n```") { + t.Errorf("DESIGN.md does not show this message any more:\n%s", designStanding) + } + e := newEnv(t) + shown, err := e.a.Ask(context.Background(), standingPrompt()) + if err != nil { + t.Fatal(err) + } + if shown.Text != designStanding { + t.Errorf("text:\n%s\nwant:\n%s", shown.Text, designStanding) + } +} + +// TestStandingPromptIsStrict: checkPrompt refuses what a standing prompt +// may not have, and windows on any other prompt. +func TestStandingPromptIsStrict(t *testing.T) { + if err := checkPrompt(standingPrompt(), false); err != nil { + t.Fatalf("a good prompt: %v", err) + } + tests := []struct { + name string + change func(*approval.Prompt) + }{ + {"no windows", func(p *approval.Prompt) { p.Windows = nil }}, + {"8 windows", func(p *approval.Prompt) { p.Windows = make([]string, 8) }}, + {"a window with markup", func(p *approval.Prompt) { p.Windows = []string{"daily 03:00-05:00"} }}, + {"a window with a newline", func(p *approval.Prompt) { p.Windows = []string{"daily 03:00-05:00\nA"} }}, + {"a window with a bad day", func(p *approval.Prompt) { p.Windows = []string{"xyz 03:00-05:00"} }}, + {"a window with a bad time", func(p *approval.Prompt) { p.Windows = []string{"daily 25:00-26:00"} }}, + {"a key", func(p *approval.Prompt) { p.Key, p.Hosts, p.RemoteUser = "web", []string{"web-1"}, "deploy" }}, + {"no secrets", func(p *approval.Prompt) { p.Secrets = nil }}, + {"no uses", func(p *approval.Prompt) { p.Uses = 0 }}, + {"no Approve button", func(p *approval.Prompt) { p.Once = false }}, + {"a window button", func(p *approval.Prompt) { p.WindowMinutes = 5 }}, + {"a grant ID", func(p *approval.Prompt) { p.GrantID = testID(1) }}, + {"the end before the start", func(p *approval.Prompt) { p.Until = p.From }}, + } + for _, tt := range tests { + p := standingPrompt() + tt.change(&p) + if checkPrompt(p, false) == nil { + t.Errorf("%s: the prompt passed", tt.name) + } + } + // A notice of a standing grant is not made yet. + if checkPrompt(standingPrompt(), true) == nil { + t.Error("a notice of a standing grant passed") + } + // Windows on a scheduled grant. + p := preapprovePrompt() + p.Windows = []string{"daily 03:00-05:00"} + if checkPrompt(p, false) == nil { + t.Error("windows on a scheduled grant passed") + } +} + +// TestStandingEndLines: the line of the end shows the cause and no uses +// left, which a standing grant does not count. +func TestStandingEndLines(t *testing.T) { + at := time.Date(2026, 10, 30, 3, 0, 0, 0, time.UTC) + for cause, want := range map[audit.EndCause]string{ + audit.EndRevoked: "revoked by you", audit.EndAlarm: "alarm", audit.EndRenewed: "renewed", + audit.EndStopped: "broker stopped", audit.EndEnded: "ended at", + } { + got := resultLine(standingPrompt(), approval.Ending{Answer: audit.AnswerApproveOnce, At: at, Approved: at, GrantEnd: cause}) + if !strings.Contains(got, want) || strings.Contains(got, "use(s) left") || !strings.Contains(got, "in each window") { + t.Errorf("%s: %q", cause, got) + } + } +} diff --git a/internal/telegram/text.go b/internal/telegram/text.go index 7ddf674..494d6b6 100644 --- a/internal/telegram/text.go +++ b/internal/telegram/text.go @@ -57,6 +57,9 @@ const claimedHead = "— claimed by the agent —" const ( noticeLine = `allowed by a "never ask" rule, without a tap` scheduledNoticeLine = `allowed by a scheduled grant, without a tap` + // standingDenyLine is in the message of a standing grant. A Deny of + // another request does not end the grant (DESIGN.md section 8). + standingDenyLine = `A Deny of other requests does not end this grant. Use Revoke to end it.` ) // maxWindowMinutes is the longest "Approve N min" (DESIGN.md section 8). @@ -64,7 +67,10 @@ const maxWindowMinutes = int(config.MaxWindow / time.Minute) // Patterns for the checked fields of a prompt, as in package audit. var ( - namePattern = regexp.MustCompile(`^[A-Za-z0-9][A-Za-z0-9._-]{0,63}$`) + namePattern = regexp.MustCompile(`^[A-Za-z0-9][A-Za-z0-9._-]{0,63}$`) + // windowPattern is the form of a window of a standing grant, as + // the core shows it: "daily 03:00-05:00" or "mon,thu 03:00-05:00". + windowPattern = regexp.MustCompile(`^(daily|(mon|tue|wed|thu|fri|sat|sun)(,(mon|tue|wed|thu|fri|sat|sun)){0,6}) ([01][0-9]|2[0-3]):[0-5][0-9]-([01][0-9]|2[0-3]):[0-5][0-9]$`) remoteUserPattern = regexp.MustCompile(`^[A-Za-z0-9._][A-Za-z0-9._-]{0,31}$`) hostKeyPattern = regexp.MustCompile(`^SHA256:[A-Za-z0-9+/]{43}$`) // shortCodePattern: 4 characters of 23456789abcdefghjkmnpqrstuvwxyz. @@ -240,10 +246,13 @@ func checkPrompt(p approval.Prompt, notice bool) error { case !namePattern.MatchString(p.Client): return bad("the client name") } - if p.Kind != audit.KindPreapprove && (p.Uses != 0 || !p.From.IsZero() || !p.Until.IsZero() || + if !p.Kind.IsPreapprove() && (p.Uses != 0 || !p.From.IsZero() || !p.Until.IsZero() || p.GrantID != "" || p.UsesLeft != 0) { return bad("a span of uses outside a scheduled grant") } + if p.Kind != audit.KindStanding && len(p.Windows) != 0 { + return bad("windows outside a standing grant") + } switch { case notice && p.Via != "" && p.Via != audit.ViaScheduled, !notice && p.Via != "": return bad("the via") @@ -279,6 +288,19 @@ func checkPrompt(p approval.Prompt, notice bool) error { (!notice && (p.GrantID != "" || p.UsesLeft != 0 || !p.Once)) { return bad("the fields of a scheduled grant") } + case audit.KindStanding: + if notice || len(p.Secrets) == 0 || !allNames(p.Secrets) || p.Key != "" || len(p.Hosts) != 0 || + p.RemoteUser != "" || p.HostKey != "" || !p.GrantEnds.IsZero() || p.WindowMinutes != 0 || + p.Uses < 1 || p.From.IsZero() || !p.From.Before(p.Until) || p.Via != "" || + p.GrantID != "" || p.UsesLeft != 0 || !p.Once || len(p.Windows) < 1 || + len(p.Windows) > approval.MaxStandingWindows { + return bad("the fields of a standing grant") + } + for _, w := range p.Windows { + if !windowPattern.MatchString(w) { + return bad("a window") + } + } default: return bad("the kind") } @@ -316,6 +338,8 @@ func title(p approval.Prompt, notice bool) string { icon, name = "🔐", "SSH grant" case audit.KindPreapprove: icon, name = "🗓", "Pre-approval" + case audit.KindStanding: + icon, name = "🗓", "Standing grant" } if notice { icon = "🔔" @@ -393,6 +417,15 @@ func body(p approval.Prompt, notice bool, cv claimedValues, c caps) *text { verified("uses", strconv.Itoa(p.Uses)) verified("from", p.From.Format(dateTimeFormat)) verified("until", p.Until.Format(dateTimeFormat)) + case audit.KindStanding: + verified("secrets", strings.Join(p.Secrets, ", ")) + for _, w := range p.Windows { + verified("window", w) + } + verified("uses in each window", strconv.Itoa(p.Uses)) + verified("from", p.From.Format(dateTimeFormat)) + verified("until", p.Until.Format(dateTimeFormat)) + t.write("\n" + standingDenyLine) } fields := []struct { label, value string @@ -439,7 +472,7 @@ func keyboard(p approval.Prompt) inlineKeyboard { var row []inlineButton if p.Once { label := "Approve once" - if p.Kind == audit.KindPreapprove { + if p.Kind.IsPreapprove() { label = "Approve" } row = append(row, inlineButton{Text: label, CallbackData: dataOnce + p.RequestID}) @@ -488,7 +521,7 @@ func expiryLine(p approval.Prompt) string { // p. The last lines need room: one, or for a scheduled grant two, because // its message ends with the line of its approval and the line of its end. func bodyLimit(p approval.Prompt) int { - if p.Kind == audit.KindPreapprove { + if p.Kind.IsPreapprove() { return maxTextUnits - 2*lastLineRoom } return maxTextUnits - lastLineRoom @@ -532,6 +565,10 @@ func resultLine(p approval.Prompt, e approval.Ending) string { } switch e.Answer { case audit.AnswerApproveOnce: + if p.Kind == audit.KindStanding { + return "👍 approved by you at " + at + ", for " + strconv.Itoa(p.Uses) + " use(s) in each window until " + + p.Until.Format(dateTimeFormat) + } if p.Kind == audit.KindPreapprove { return "👍 approved by you at " + at + ", for " + strconv.Itoa(p.Uses) + " use(s) until " + p.Until.Format(dateTimeFormat) @@ -562,6 +599,9 @@ func grantEndLines(p approval.Prompt, e approval.Ending) string { approved := resultLine(p, approval.Ending{Answer: e.Answer, At: e.Approved}) at := e.At.Format(dateTimeFormat) var end string + if p.Kind == audit.KindStanding { + return approved + "\n" + standingEndLine(e.GrantEnd, at) + } switch e.GrantEnd { case audit.EndUsedUp: end = "✅ all " + strconv.Itoa(p.Uses) + " use(s) used; the grant ended at " + at @@ -586,3 +626,21 @@ func noticeText(p approval.Prompt) (*text, error) { } return fitBody(p, true, maxTextUnits) } + +// standingEndLine says how and when a standing grant ended. It has no +// uses left to show: it counts its uses in each window. +func standingEndLine(cause audit.EndCause, at string) string { + switch cause { + case audit.EndEnded: + return "⌛ ended at " + at + case audit.EndRevoked: + return "⛔ revoked by you at " + at + case audit.EndAlarm: + return "🚨 ended by an alarm at " + at + case audit.EndRenewed: + return "🔁 renewed at " + at + "; a new grant replaced this one" + case audit.EndStopped: + return "⛔ dropped: the broker stopped at " + at + } + return "⛔ ended at " + at +} From 23b2c83c76cb3c92e88e32031d2fdf7050acb971 Mon Sep 17 00:00:00 2001 From: Geno Roupsky <185580+groupsky@users.noreply.github.com> Date: Thu, 29 Oct 2026 07:32:30 +0200 Subject: [PATCH 09/18] feat(audit): grant_alarm and new_token_hash entries (#110) Co-Authored-By: Claude Opus 5.5 --- internal/audit/event.go | 58 ++++++++++++++++++++++++++++++++++-- internal/audit/event_test.go | 17 ++++++++++- 2 files changed, 72 insertions(+), 3 deletions(-) diff --git a/internal/audit/event.go b/internal/audit/event.go index cbdde4e..a40bff0 100644 --- a/internal/audit/event.go +++ b/internal/audit/event.go @@ -229,6 +229,31 @@ type GrantUsed struct { Subject // Via says what allowed it. Via Via `json:"via"` + // NewTokenHash is the SHA-256 hash of the new token of a standing + // grant, in hex, for Via standing only. Never the token itself. + NewTokenHash string `json:"new_token_hash,omitempty"` +} + +// AlarmCause says why a standing grant raised an alarm (DESIGN.md section +// 8). +type AlarmCause string + +// The causes of a grant_alarm entry. +const ( + AlarmOldToken AlarmCause = "old_token" // a token that was replaced came again + AlarmOverUses AlarmCause = "over_uses" // more uses in a window than allowed + AlarmOutsideWindow AlarmCause = "outside_window" // a valid token outside the windows + AlarmUnknownToken AlarmCause = "unknown_token" // a token that no grant of the client has + AlarmMissedWindow AlarmCause = "missed_window" // a window ended with no use +) + +// GrantAlarm records an alarm of a standing grant. Subject is that of the +// request, with the GrantID of the grant. For an unknown token there is no +// grant, so no GrantID. +type GrantAlarm struct { + Subject + // Cause says why. + Cause AlarmCause `json:"cause"` } // GrantEnded records the end of a scheduled grant. Subject is that of the @@ -308,6 +333,7 @@ const ( eventDecision = "decision" eventGrantUsed = "grant_used" eventGrantEnded = "grant_ended" + eventGrantAlarm = "grant_alarm" eventSecretSent = "secret_sent" eventSigned = "signed" eventDenied = "denied" @@ -322,6 +348,7 @@ func (Request) eventName() string { return eventRequest } func (Decision) eventName() string { return eventDecision } func (GrantUsed) eventName() string { return eventGrantUsed } func (GrantEnded) eventName() string { return eventGrantEnded } +func (GrantAlarm) eventName() string { return eventGrantAlarm } func (SecretSent) eventName() string { return eventSecretSent } func (Signed) eventName() string { return eventSigned } func (Denied) eventName() string { return eventDenied } @@ -481,12 +508,39 @@ func (e GrantUsed) validate() error { return err } switch e.Via { - case ViaGrant, ViaNeverRule, ViaScheduled, ViaStanding: + case ViaGrant, ViaNeverRule, ViaScheduled: + if e.NewTokenHash != "" { + return errors.New("new_token_hash is only for via standing") + } + return nil + case ViaStanding: + if !isHex64(e.NewTokenHash) { + return errors.New("a standing use needs new_token_hash, 64 hex digits") + } return nil } return errors.New("via is not known") } +func (e GrantAlarm) validate() error { + if err := e.Subject.validate(true); err != nil { + return err + } + switch e.Cause { + case AlarmUnknownToken: + if e.GrantID != "" { + return errors.New("an unknown token has no grant_id") + } + return nil + case AlarmOldToken, AlarmOverUses, AlarmOutsideWindow, AlarmMissedWindow: + if e.GrantID == "" { + return errors.New("it needs a grant_id") + } + return nil + } + return errors.New("cause is not known") +} + func (e GrantEnded) validate() error { if err := e.Subject.validate(true); err != nil { return err @@ -551,7 +605,7 @@ func encodeEvent(ev Event) (string, []byte, error) { // pointer has the methods as well, and a nil pointer would panic in // them. switch ev.(type) { - case BrokerStarted, BrokerStopped, Request, Decision, GrantUsed, GrantEnded, SecretSent, Signed, Denied, ForeignTap, Failure, + case BrokerStarted, BrokerStopped, Request, Decision, GrantUsed, GrantEnded, GrantAlarm, SecretSent, Signed, Denied, ForeignTap, Failure, logRecovered: default: return "", nil, errors.New("not an event of the audit package; pass an event such as audit.Signed{...} as a value") diff --git a/internal/audit/event_test.go b/internal/audit/event_test.go index 2e08b96..3bf6324 100644 --- a/internal/audit/event_test.go +++ b/internal/audit/event_test.go @@ -208,7 +208,22 @@ func TestStandingEvents(t *testing.T) { ok.Windows = []string{"daily 03:00-05:00", "mon,thu 10:30-11:00"} good("a standing request", Request{Subject: ok}) good("a standing decision", Decision{Subject: ok, UserID: 1, Answer: AnswerApproveOnce}) - good("a standing use", GrantUsed{Subject: ok, Via: ViaStanding}) + h := strings.Repeat("ab", 32) + good("a standing use", GrantUsed{Subject: ok, Via: ViaStanding, NewTokenHash: h}) + bad("a standing use without a hash", GrantUsed{Subject: ok, Via: ViaStanding}) + bad("a standing use with a short hash", GrantUsed{Subject: ok, Via: ViaStanding, NewTokenHash: "ab"}) + bad("a hash outside a standing use", GrantUsed{Subject: ok, Via: ViaScheduled, NewTokenHash: h}) + for _, c := range []AlarmCause{AlarmOldToken, AlarmOverUses, AlarmOutsideWindow, AlarmMissedWindow} { + good("grant_alarm "+string(c), GrantAlarm{Subject: ok, Cause: c}) + noID := ok + noID.GrantID = "" + bad("grant_alarm without a grant "+string(c), GrantAlarm{Subject: noID, Cause: c}) + } + noID := ok + noID.GrantID = "" + 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"}) for _, c := range []EndCause{EndAlarm, EndRenewed, EndRevoked, EndStopped, EndEnded} { good("grant_ended "+string(c), GrantEnded{Subject: ok, Cause: c}) } From c930c57c7aff9ee268e4b5bb1299831651d014ad Mon Sep 17 00:00:00 2001 From: Geno Roupsky <185580+groupsky@users.noreply.github.com> Date: Thu, 29 Oct 2026 07:35:02 +0200 Subject: [PATCH 10/18] feat(broker): use and rotate standing grant tokens, with alarms (#110) Co-Authored-By: Claude Opus 5.5 --- DESIGN.md | 3 +- internal/approval/approvaltest/approver.go | 24 ++ internal/approval/grants.go | 2 + internal/approval/request.go | 18 +- internal/approval/standinguse.go | 253 +++++++++++++++ internal/approval/state.go | 2 + internal/broker/handler.go | 7 + internal/broker/secrets.go | 61 +++- internal/broker/standinguse_test.go | 342 +++++++++++++++++++++ internal/telegram/alarm_test.go | 108 +++++++ internal/telegram/telegram.go | 23 ++ internal/telegram/text.go | 53 +++- 12 files changed, 887 insertions(+), 9 deletions(-) create mode 100644 internal/approval/standinguse.go create mode 100644 internal/broker/standinguse_test.go create mode 100644 internal/telegram/alarm_test.go diff --git a/DESIGN.md b/DESIGN.md index 51bd0e7..cede1ed 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -1544,7 +1544,8 @@ grant ends, to know the old tokens. **Alarms.** An alarm is a Telegram message with sound, and a `grant_alarm` entry with a cause. It counts toward the limit of 20 notices in all. One -grant sends at most one alarm in a window. +grant sends at most one alarm in a window. For a valid token outside the +windows, the limit is one alarm in a calendar day. | case | request | grant | cause | |---|---|---|---| diff --git a/internal/approval/approvaltest/approver.go b/internal/approval/approvaltest/approver.go index 6682d87..d4b075c 100644 --- a/internal/approval/approvaltest/approver.go +++ b/internal/approval/approvaltest/approver.go @@ -49,6 +49,8 @@ type Approver struct { asked []Asked ended []Ended notices []approval.Prompt + alarms []approval.Alarm + alarmErr error askErr error notifyErr error endErr error @@ -245,6 +247,28 @@ func (a *Approver) Ended() []Ended { return append([]Ended(nil), a.ended...) } +// Alarm keeps the alarm, unless FailAlarm set an error. +func (a *Approver) Alarm(ctx context.Context, al approval.Alarm) error { + if err := ctx.Err(); err != nil { + return err + } + a.mu.Lock() + defer a.mu.Unlock() + if a.alarmErr != nil { + return a.alarmErr + } + a.alarms = append(a.alarms, al) + a.changedLocked() + return nil +} + +// Alarms returns the alarms, oldest first. +func (a *Approver) Alarms() []approval.Alarm { + a.mu.Lock() + defer a.mu.Unlock() + return append([]approval.Alarm(nil), a.alarms...) +} + // Notices returns the notices of "never ask" uses, oldest first. func (a *Approver) Notices() []approval.Prompt { a.mu.Lock() diff --git a/internal/approval/grants.go b/internal/approval/grants.go index 387d4e2..2d87b07 100644 --- a/internal/approval/grants.go +++ b/internal/approval/grants.go @@ -72,6 +72,8 @@ type grant struct { // before: one for each use. The grant never holds a token. tokenHash [32]byte oldHashes [][32]byte + // alarmMark is the day of the last alarm outside the windows. + alarmMark string } // grantKind is the kind of a grant (DESIGN.md section 8, "The grant core"). diff --git a/internal/approval/request.go b/internal/approval/request.go index c866337..96a3ff9 100644 --- a/internal/approval/request.go +++ b/internal/approval/request.go @@ -116,6 +116,7 @@ type Result struct { plan *grant // the scheduled grant of an approved KindPreapprove request msg MessageID // with plan: the message of the request, and its prompt prompt Prompt + su *standingUse // the use path of a standing grant usedID string // the scheduled grant that allowed the request ended []audit.GrantEnded // entries of grants that this request ended vias []audit.Via // what allowed each secret or key without a tap @@ -149,6 +150,11 @@ type Result struct { // grant_used and before secret_sent or signed. func (r *Result) Events() []audit.Event { events := r.mainEvents() + if r.su != nil && r.su.alarm != "" { + s := r.Subject + s.GrantID = r.su.grantID + events = append(events, audit.GrantAlarm{Subject: s, Cause: r.su.alarm}) + } for _, ev := range r.ended { events = append(events, ev) } @@ -167,7 +173,7 @@ func (r *Result) mainEvents() []audit.Event { events = append(events, audit.Denied{Subject: r.Subject, Cause: r.Cause}) } case r.Allowed: - for _, via := range []audit.Via{audit.ViaGrant, audit.ViaNeverRule, audit.ViaScheduled} { + for _, via := range []audit.Via{audit.ViaGrant, audit.ViaNeverRule, audit.ViaScheduled, audit.ViaStanding} { used := false var secrets []string for i, v := range r.vias { @@ -186,10 +192,14 @@ func (r *Result) mainEvents() []audit.Event { if s.Kind == audit.KindSecrets { s.Secrets = secrets } - if via == audit.ViaScheduled { - s.GrantID = r.usedID + gu := audit.GrantUsed{Subject: s, Via: via} + if via == audit.ViaScheduled || via == audit.ViaStanding { + gu.GrantID = r.usedID + } + if via == audit.ViaStanding { + gu.NewTokenHash = r.newTokenHash() } - events = append(events, audit.GrantUsed{Subject: s, Via: via}) + events = append(events, gu) } default: if r.Cause == audit.CauseError && r.ErrorCode != "" { diff --git a/internal/approval/standinguse.go b/internal/approval/standinguse.go new file mode 100644 index 0000000..1112bb5 --- /dev/null +++ b/internal/approval/standinguse.go @@ -0,0 +1,253 @@ +package approval + +import ( + "context" + "encoding/hex" + "log/slog" + "slices" + + "github.com/code6x/nodkey/internal/audit" + "github.com/code6x/nodkey/internal/grantkey" +) + +// MaxUnknownTokensPerMinute is the most unknown tokens of one client that +// raise an alarm, and get entries in the audit log, in any 60 seconds +// (DESIGN.md section 8). The others are denied with cause limit. +const MaxUnknownTokensPerMinute = 3 + +// Alarm is what the approver shows for an alarm of a standing grant: a +// message with a sound. It holds no token. +type Alarm struct { + RequestID, ShortCode string + Client string + Secrets []string + // GrantID is "" for an unknown token. + GrantID string + Cause audit.AlarmCause + // Ended says that the alarm ended the grant. + Ended bool +} + +// Alarmer is an Approver that can also send alarms. The core sends an +// alarm only to an approver that has this method. It counts toward the +// same limit of notices as Notify does. +type Alarmer interface { + Alarm(ctx context.Context, a Alarm) error +} + +// StandingRequest is a request for secrets with a standing grant token +// (DESIGN.md section 8). The broker has checked the secret names. +type StandingRequest struct { + Client string + Secrets []string + Claim Claim + // Token is the hash of the token that the client sent, or nil when + // the header was not a valid token. + Token *[32]byte +} + +// standingUse is what a Result of UseStanding keeps. +type standingUse struct { + p *pending + g *grant + prevHash [32]byte + newHash [32]byte + token grantkey.Token + alarm audit.AlarmCause + grantID string // of the alarm + ended bool +} + +// UseStanding decides a request with a token: the use path of a standing +// grant (DESIGN.md section 8). It never asks the person. For an allowed +// request, the token has been replaced already: the broker writes +// Events() first, then calls SendNotice, and hands out nothing if either +// fails (the first by calling Undo itself). For a denied request it writes +// Events(), then calls SendAlarm. +func (c *Core) UseStanding(r StandingRequest) *Result { + p := &pending{start: c.now()} + if err := c.register(p); err != nil { + return &Result{Cause: audit.CauseError, Err: err, at: p.start.wall} + } + defer c.leave(p) + p.deadline = p.start.add(c.timeout(audit.KindSecrets)) + if !c.check(&Request{Client: r.Client, Kind: audit.KindSecrets, Secrets: r.Secrets, Claim: r.Claim}, p) { + return c.denied(p, audit.CauseNotAllowed) + } + tok, err := grantkey.New() + if err != nil { + res := c.denied(p, audit.CauseError) + res.Err = err + return res + } + c.mu.Lock() + defer c.mu.Unlock() + if c.stopped { + return c.failed(p, ErrStopped) + } + now := c.now() + c.sweepGrants(now) + cs := c.client(p.client) + // Every grant and hash is compared, with no early exit. + var cur, old *grant + if r.Token != nil { + for _, g := range c.grants { + if !g.standing() || g.client != p.client { + continue + } + if grantkey.Equal(g.tokenHash, *r.Token) { + cur = g + } + for _, h := range g.oldHashes { + if grantkey.Equal(h, *r.Token) { + old = g + } + } + } + } + deny := func(g *grant, cause audit.AlarmCause, end bool) *Result { + res := c.denied(p, audit.CauseNotAllowed) + su := &standingUse{p: p, alarm: cause} + if g != nil { + su.grantID, res.Subject.GrantID = g.id, g.id + } + if end { + su.ended = true + res.ended = append(res.ended, c.endWhere(func(x *grant) audit.EndCause { + if x == g { + return audit.EndAlarm + } + return "" + })...) + } + res.su = su + return res + } + switch { + case cur == nil && old == nil: + if cs.unknown.recent(now.mono) >= MaxUnknownTokensPerMinute { + return c.denied(p, audit.CauseLimit) + } + cs.unknown = cs.unknown.add(now.mono, MaxUnknownTokensPerMinute) + return deny(nil, audit.AlarmUnknownToken, false) + case cur == nil: + return deny(old, audit.AlarmOldToken, true) + } + g := cur + 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 + } + } + switch g.tryUse(now) { + case useNone: + return c.denied(p, audit.CauseNotAllowed) + case useOutsideWindow: + res := deny(g, audit.AlarmOutsideWindow, false) + if mark := now.wall.Format("2006-01-02"); g.alarmMark == mark { + res.su.alarm = "" // one alarm in a window + } else { + g.alarmMark = mark + } + return res + case useOverUses: + return deny(g, audit.AlarmOverUses, true) + } + if cs.notices.recent(now.mono) >= MaxNoticesPerMinute { + g.giveBackUse() + return c.denied(p, audit.CauseLimit) + } + cs.notices = cs.notices.add(now.mono, MaxNoticesPerMinute) + su := &standingUse{p: p, g: g, prevHash: g.tokenHash, newHash: grantkey.Hash(tok), token: tok} + g.oldHashes = append(g.oldHashes, g.tokenHash) + g.tokenHash = su.newHash + res := c.newResult(p) + res.Allowed = true + res.usedID = g.id + res.su = su + res.vias = make([]audit.Via, len(p.items)) + for i := range res.vias { + res.vias[i] = audit.ViaStanding + } + return res +} + +// NewToken returns the new token of an allowed UseStanding result. The +// broker puts it only into the answer. +func (r *Result) NewToken() (grantkey.Token, bool) { + if r == nil || r.su == nil || !r.Allowed { + return grantkey.Token{}, false + } + return r.su.token, true +} + +// Undo takes back the use and the new token of an allowed UseStanding +// result, because the use did not happen: the old token is valid again. +func (r *Result) Undo() { + if r == nil || r.su == nil || r.su.g == nil || r.core == nil { + return + } + c, su := r.core, r.su + c.mu.Lock() + defer c.mu.Unlock() + if !slices.Contains(c.grants, su.g) || su.g.tokenHash != su.newHash { + return + } + su.g.tokenHash = su.prevHash + if n := len(su.g.oldHashes); n > 0 { + su.g.oldHashes = su.g.oldHashes[:n-1] + } + su.g.giveBackUse() +} + +// SendNotice sends the silent notice of an allowed UseStanding result. If +// it cannot be sent, it undoes the use and returns the error: the request +// is then denied, and the old token stays valid. +func (r *Result) SendNotice(ctx context.Context) error { + c, p := r.core, r.su.p + now := c.now() + pr := p.notice(now) + pr.Via = audit.ViaStanding + nctx, cancel := c.callContext(ctx, now.until(p.deadline)) + err := c.approver.Notify(nctx, pr) + cancel() + if err == nil && ctx.Err() != nil { + err = ctx.Err() + } + if err != nil { + r.Undo() + } + return err +} + +// SendAlarm sends the alarm of a denied UseStanding result, with a sound, +// if there is one. It is best effort: the request is denied anyway. The +// entry of the alarm is in Events. +func (r *Result) SendAlarm(ctx context.Context) { + if r == nil || r.su == nil || r.su.alarm == "" || r.Allowed || r.core == nil { + return + } + c, su := r.core, r.su + al, ok := c.approver.(Alarmer) + if !ok { + return + } + actx, cancel := c.callContext(context.WithoutCancel(ctx), endTimeout) + defer cancel() + err := al.Alarm(actx, Alarm{ + RequestID: r.RequestID, ShortCode: r.ShortCode, Client: r.Subject.Client, + Secrets: slices.Clone(r.Subject.Secrets), GrantID: su.grantID, Cause: su.alarm, Ended: su.ended, + }) + if err != nil { + c.logger.LogAttrs(actx, slog.LevelWarn, "approval: cannot send the alarm of a standing grant", + slog.String("request_id", r.RequestID), slog.String("cause", string(su.alarm)), slog.Any("error", err)) + } +} + +// newTokenHash is the hash of the new token in hex, for the audit log. +func (r *Result) newTokenHash() string { + if r.su == nil { + return "" + } + return hex.EncodeToString(r.su.newHash[:]) +} diff --git a/internal/approval/state.go b/internal/approval/state.go index 685bf4d..64f1b01 100644 --- a/internal/approval/state.go +++ b/internal/approval/state.go @@ -155,6 +155,8 @@ type clientState struct { sent rateLog // notices holds when the newest notices of "never ask" uses were sent. notices rateLog + // unknown holds when the newest alarms of unknown tokens came. + unknown rateLog // queue holds the requests that wait for a place, oldest first. queue []*pending // paused holds when the pause after a Deny ends, for each secret or diff --git a/internal/broker/handler.go b/internal/broker/handler.go index 22ef284..4869b3f 100644 --- a/internal/broker/handler.go +++ b/internal/broker/handler.go @@ -366,6 +366,13 @@ func (rq *request) decide(ctx context.Context, ar *approval.Request) (res *appro rq.inDecide = true res = rq.core.Decide(ctx, ar) rq.inDecide = false + return rq.settle(res) +} + +// settle writes the entries of res, and answers a request that is not +// allowed (see decide). +func (rq *request) settle(res *approval.Result) (*approval.Result, receipt, bool) { + var rc receipt rq.sub = res.Subject if res.Err != nil { // Result.Err goes to the journal, never to the audit log. diff --git a/internal/broker/secrets.go b/internal/broker/secrets.go index 7ec8706..6c835b7 100644 --- a/internal/broker/secrets.go +++ b/internal/broker/secrets.go @@ -10,6 +10,7 @@ import ( "github.com/code6x/nodkey/internal/approval" "github.com/code6x/nodkey/internal/audit" "github.com/code6x/nodkey/internal/claim" + "github.com/code6x/nodkey/internal/grantkey" "github.com/code6x/nodkey/internal/store" "github.com/code6x/nodkey/internal/transport" ) @@ -40,12 +41,28 @@ func (rq *request) secrets(ctx context.Context, r *http.Request) { rq.logDeniedVar(req.Secrets) return } - res, rc, ok := rq.decide(ctx, &approval.Request{ - Client: rq.client, Kind: audit.KindSecrets, Secrets: names, Claim: cl, - }) + var res *approval.Result + var rc receipt + if tokens := r.Header.Values(api.GrantTokenHeader); len(tokens) > 0 { + // A request with a token never asks the person (DESIGN.md section 8). + res, rc, ok = rq.decideStanding(ctx, names, cl, tokens) + } else { + res, rc, ok = rq.decide(ctx, &approval.Request{ + Client: rq.client, Kind: audit.KindSecrets, Secrets: names, Claim: cl, + }) + } if !ok { return } + // A standing grant has a new token now. If nothing is answered, the + // client never gets it, so the use is undone and the old token stays + // valid (DESIGN.md section 8). + handed := false + defer func() { + if !handed { + res.Undo() + } + }() // The decision entry is on disk: the grants of an "Approve N min" tap // may start, before anything is handed out. res.StartGrants() @@ -54,6 +71,9 @@ func (rq *request) secrets(ctx context.Context, r *http.Request) { return } ans := api.SecretsAnswer{Secrets: make([]api.SecretValue, len(req.Secrets))} + if tok, rotated := res.NewToken(); rotated { + ans.GrantToken = tok.Reveal() + } for i, ref := range req.Secrets { // The value goes from Bytes into the answer, without a copy. ans.Secrets[i] = api.SecretValue{Var: ref.Var, Value: values[ref.Name].Bytes()} @@ -77,6 +97,7 @@ func (rq *request) secrets(ctx context.Context, r *http.Request) { return } rq.answered = true + handed = true if err := transport.WriteJSONBody(rq.w, http.StatusOK, body); err != nil { rq.s.log.Warn("broker: cannot write the answer; the client may not have it", slog.String("request_id", rq.sub.RequestID), slog.String("client", rq.client)) @@ -182,3 +203,37 @@ func (rq *request) fetch(ctx context.Context, rc receipt, names []string) (map[s } return values, true } + +// decideStanding decides a request that has the header of a standing grant +// token (DESIGN.md section 8). The core never asks the person. It writes +// the entries, sends the alarm of a denied request, and sends the silent +// notice of an allowed one. It returns ok only when every step worked; the +// request has its answer otherwise. If a step fails after the new token +// was made, the use is undone, so the old token stays valid. +func (rq *request) decideStanding(ctx context.Context, names []string, cl approval.Claim, tokens []string) (*approval.Result, receipt, bool) { + var hash *[32]byte + if len(tokens) == 1 { + if t, err := grantkey.Parse(tokens[0]); err == nil { + h := grantkey.Hash(t) + hash = &h + } + } + rq.failCode = audit.CodeApproverFailed + res := rq.core.UseStanding(approval.StandingRequest{Client: rq.client, Secrets: names, Claim: cl, Token: hash}) + res, rc, ok := rq.settle(res) + if !ok { + if res.Allowed { + res.Undo() // the entry could not be written, or the broker stops + } + res.SendAlarm(ctx) + return res, rc, false + } + if err := res.SendNotice(ctx); err != nil { + rq.s.log.Warn("broker: cannot send the notice of a standing grant use; the request is denied", + slog.String("request_id", rq.sub.RequestID), slog.String("client", rq.client), + slog.String("error", errorText(err))) + rq.fail(audit.CodeApproverFailed) + return res, rc, false + } + return res, rc, true +} diff --git a/internal/broker/standinguse_test.go b/internal/broker/standinguse_test.go new file mode 100644 index 0000000..bcd4c8d --- /dev/null +++ b/internal/broker/standinguse_test.go @@ -0,0 +1,342 @@ +package broker + +import ( + "bytes" + "context" + "crypto/sha256" + "encoding/hex" + "encoding/json" + "errors" + "net/http" + "strings" + "testing" + "time" + + "github.com/code6x/nodkey/internal/api" + "github.com/code6x/nodkey/internal/audit" + "github.com/code6x/nodkey/internal/grantkey" +) + +// useEnv is an environment with one standing grant of laptop for +// github-token, daily 11:00-13:00 (the clock is at 12:00), 2 uses in each +// window. tok is its token. +func useEnv(t *testing.T, names ...string) (*env, string, string) { + t.Helper() + e := newEnv(t, envOptions{config: standingOn}) + if len(names) == 0 { + names = []string{"github-token"} + } + r := standReq(names...) + r.Windows = []api.Window{{Kind: api.WindowDaily, From: "11:00", To: "13:00"}} + ans := e.preapprove(e.httpClient("laptop"), r, 1) + return e, ans.GrantToken, ans.ID +} + +// useToken sends a request for github-token with the header, as client. +func (e *env) useToken(client, token string, names ...string) (*result, *api.SecretsAnswer) { + e.t.Helper() + req := secretsRequest("GH_TOKEN", "github-token") + if client == "ci" { + req = secretsRequest("GH_TOKEN", "backup-token") + } + for i, n := range names { + req.Secrets = append(req.Secrets, api.SecretRef{Var: "V" + string(rune('A'+i)), Name: n}) + } + data, _ := json.Marshal(req) + hr, err := http.NewRequestWithContext(context.Background(), http.MethodPost, "https://"+e.addr+api.PathSecrets, bytes.NewReader(data)) + if err != nil { + e.t.Fatal(err) + } + hr.Header.Set("Content-Type", api.ContentType) + hr.Header.Set(api.GrantTokenHeader, token) + resp, err := e.httpClient(client).Do(hr) + if err != nil { + e.t.Fatal(err) + } + defer func() { _ = resp.Body.Close() }() + res := &result{status: resp.StatusCode} + buf := new(bytes.Buffer) + _, _ = buf.ReadFrom(resp.Body) + res.body = buf.Bytes() + if resp.StatusCode != http.StatusOK { + return res, nil + } + ans, err := api.ReadAnswer[api.SecretsAnswer](bytes.NewReader(res.body)) + if err != nil { + e.t.Fatalf("the answer: %v", err) + } + return res, ans +} + +func hashHex(token string) string { + h := sha256.Sum256([]byte(token)) + return hex.EncodeToString(h[:]) +} + +// noToken fails if the token is in any text the broker wrote or sent. +func (e *env) noToken(tokens ...string) { + e.t.Helper() + alarms := "" + for _, a := range e.approver.Alarms() { + alarms += a.Client + a.GrantID + string(a.Cause) + strings.Join(a.Secrets, ",") + } + for name, text := range map[string]string{ + "the audit log": encodeEvents(e.t, e.audit.all()), "the approver": e.approverTexts() + alarms, + "the logs": e.logs.String(), "the notices": strings.Join(e.notices.wentOut(), "\n"), + } { + for _, tok := range tokens { + if strings.Contains(text, tok) { + e.t.Errorf("a token is in %s", name) + } + } + } +} + +func (e *env) alarmEntries() (out []audit.GrantAlarm) { + for _, ev := range e.audit.all() { + if a, ok := ev.(audit.GrantAlarm); ok { + out = append(out, a) + } + } + return out +} + +// TestStandingUseRotates: the way of a good use, then the second use in the +// window, then the use over the limit that ends the grant. +func TestStandingUseRotates(t *testing.T) { + e, tok0, id := useEnv(t) + noticesBefore := len(e.approver.Notices()) + res, ans := e.useToken("laptop", tok0) + mustStatus(t, res, http.StatusOK) + tok1 := ans.GrantToken + if _, err := grantkey.Parse(tok1); err != nil || tok1 == tok0 || len(ans.Secrets) != 1 { + t.Fatalf("the answer: token ok=%v, secrets %d", err == nil, len(ans.Secrets)) + } + var used *audit.GrantUsed + for _, ev := range e.audit.all() { + if u, ok := ev.(audit.GrantUsed); ok { + used = &u + } + } + if used == nil || used.Via != audit.ViaStanding || used.GrantID != id || used.NewTokenHash != hashHex(tok1) { + t.Fatalf("grant_used is %+v", used) + } + ns := e.approver.Notices() + if len(ns) != noticesBefore+1 || ns[len(ns)-1].Via != audit.ViaStanding { + t.Errorf("notices: %+v", ns) + } + if len(e.approver.Asked()) != 1 { + t.Error("the person was asked for a use") + } + // The second use in the window, with the new token. + res, ans = e.useToken("laptop", tok1) + mustStatus(t, res, http.StatusOK) + tok2 := ans.GrantToken + // The third is over the 2 uses: a deny, an alarm, the end. + res, _ = e.useToken("laptop", tok2) + mustStatus(t, res, http.StatusForbidden) + al := e.alarmEntries() + if len(al) != 1 || al[0].Cause != audit.AlarmOverUses || al[0].GrantID != id { + t.Fatalf("alarms: %+v", al) + } + if as := e.approver.Alarms(); len(as) != 1 || !as[0].Ended || as[0].Cause != audit.AlarmOverUses { + t.Errorf("the alarm: %+v", as) + } + var ended bool + for _, ev := range e.audit.all() { + if g, ok := ev.(audit.GrantEnded); ok && g.GrantID == id && g.Cause == audit.EndAlarm { + ended = true + } + } + if !ended { + t.Error("no grant_ended:alarm") + } + // The grant is gone: the token is now unknown. + res, _ = e.useToken("laptop", tok2) + mustStatus(t, res, http.StatusForbidden) + if len(e.approver.Asked()) != 1 { + t.Error("the person was asked after a token was sent") + } + e.noToken(tok0, tok1, tok2) +} + +// TestStandingOldToken: an old token is denied and ends the grant. +func TestStandingOldToken(t *testing.T) { + e, tok0, id := useEnv(t) + _, ans := e.useToken("laptop", tok0) + tok1 := ans.GrantToken + res, _ := e.useToken("laptop", tok0) + mustStatus(t, res, http.StatusForbidden) + al := e.alarmEntries() + if len(al) != 1 || al[0].Cause != audit.AlarmOldToken || al[0].GrantID != id { + t.Fatalf("alarms: %+v", al) + } + if as := e.approver.Alarms(); len(as) != 1 || !as[0].Ended { + t.Errorf("the alarm: %+v", as) + } + // The new token does not work any more: the grant ended. + res, _ = e.useToken("laptop", tok1) + mustStatus(t, res, http.StatusForbidden) + if len(e.approver.Asked()) != 1 { + t.Error("the person was asked") + } + e.noToken(tok0, tok1) +} + +// TestStandingOutsideWindow: a denied use, one alarm in the window, the +// grant stays and works in the next window. +func TestStandingOutsideWindow(t *testing.T) { + e, tok0, id := useEnv(t) + e.clock.Advance(3 * time.Hour) // 15:00 + for range 2 { + res, _ := e.useToken("laptop", tok0) + mustStatus(t, res, http.StatusForbidden) + } + al := e.alarmEntries() + if len(al) != 1 || al[0].Cause != audit.AlarmOutsideWindow || al[0].GrantID != id { + t.Fatalf("alarms: %+v", al) + } + if as := e.approver.Alarms(); len(as) != 1 || as[0].Ended { + t.Errorf("the alarms: %+v", as) + } + e.clock.Advance(20 * time.Hour) // 11:00 the next day + res, _ := e.useToken("laptop", tok0) + mustStatus(t, res, http.StatusOK) + for _, ev := range e.audit.all() { + if g, ok := ev.(audit.GrantEnded); ok && g.GrantID == id { + t.Error("the grant ended") + } + } +} + +// TestStandingUnknownToken: no grant, an alarm with sound, rate limited; +// a bad format is the same; a token of another client is the same. +func TestStandingUnknownToken(t *testing.T) { + e, tok0, _ := useEnv(t) + other, err := grantkey.New() + if err != nil { + t.Fatal(err) + } + res, _ := e.useToken("laptop", other.Reveal()) + mustStatus(t, res, http.StatusForbidden) + res, _ = e.useToken("laptop", "not a token") + mustStatus(t, res, http.StatusForbidden) + // The token of laptop's grant, sent by ci. + res, _ = e.useToken("ci", tok0) + mustStatus(t, res, http.StatusForbidden) + al := e.alarmEntries() + if len(al) != 3 || al[2].Cause != audit.AlarmUnknownToken || al[2].Client != "ci" || al[0].Cause != audit.AlarmUnknownToken || al[0].GrantID != "" { + t.Fatalf("alarms: %+v", al) + } + if len(e.approver.Alarms()) != 3 { + t.Errorf("the alarms: %+v", e.approver.Alarms()) + } + // Laptop's grant is not touched. + res, _ = e.useToken("laptop", tok0) + mustStatus(t, res, http.StatusOK) + // More than 3 a minute for one client: no more alarms. + for range 6 { + _, _ = e.useToken("laptop", other.Reveal()) + } + if n := len(e.approver.Alarms()); n > 1+MaxUnknownPerMinuteForTest { + t.Errorf("%d alarms", n) + } + e.noToken(tok0, other.Reveal()) +} + +// MaxUnknownPerMinuteForTest is the limit of the core. +const MaxUnknownPerMinuteForTest = 3 + +// TestStandingNotAllSecrets: a token for a grant that does not cover the +// whole request is denied; it takes no use and raises no alarm. +func TestStandingNotAllSecrets(t *testing.T) { + e, tok0, _ := useEnv(t) + res, _ := e.useToken("laptop", tok0, "npm-token") + mustStatus(t, res, http.StatusForbidden) + if len(e.alarmEntries()) != 0 || len(e.approver.Alarms()) != 0 { + t.Error("an alarm for a request that the grant does not cover") + } + if len(e.approver.Asked()) != 1 { + t.Error("the person was asked") + } + for range 2 { // both uses are still there + res, ans := e.useToken("laptop", tok0) + mustStatus(t, res, http.StatusOK) + tok0 = ans.GrantToken + } +} + +// TestStandingAuditFailure: if grant_used cannot be written, the request is +// denied, the use is given back, and the token is not replaced. +func TestStandingAuditFailure(t *testing.T) { + e, tok0, _ := useEnv(t) + e.audit.setFail(func(ev audit.Event) error { + if u, ok := ev.(audit.GrantUsed); ok && u.Via == audit.ViaStanding { + return errors.New("disk full") + } + return nil + }) + res, _ := e.useToken("laptop", tok0) + mustStatus(t, res, http.StatusServiceUnavailable) + if strings.Contains(string(res.body), "nkg_") { + t.Error("a token in a failed answer") + } + e.audit.setFail(nil) + for range 2 { + res, ans := e.useToken("laptop", tok0) + mustStatus(t, res, http.StatusOK) + tok0 = ans.GrantToken + } + if len(e.alarmEntries()) != 0 { + t.Error("an alarm after a failed write") + } +} + +// TestStandingNoticeFailure: if the notice cannot be sent, the request is +// denied and the old token stays valid. +func TestStandingNoticeFailure(t *testing.T) { + e, tok0, _ := useEnv(t) + e.approver.FailNotify(errors.New("telegram is down")) + res, _ := e.useToken("laptop", tok0) + mustStatus(t, res, http.StatusServiceUnavailable) + e.approver.FailNotify(nil) + res, ans := e.useToken("laptop", tok0) + mustStatus(t, res, http.StatusOK) + if ans.GrantToken == tok0 { + t.Error("the token did not change") + } + if len(e.alarmEntries()) != 0 { + t.Error("an alarm for a failed notice") + } + e.noToken(tok0, ans.GrantToken) +} + +// TestStandingNoTokenAsksAsBefore: a request without the header is unchanged. +func TestStandingNoTokenAsksAsBefore(t *testing.T) { + e, _, _ := useEnv(t) + p := e.start(e.httpClient("laptop"), api.PathSecrets, secretsRequest("GH_TOKEN", "github-token")) + pr := e.waitAsked(2)[1].Prompt + if pr.Kind != audit.KindSecrets { + t.Errorf("kind %q", pr.Kind) + } + e.tap(pr.RequestID, audit.AnswerDeny) + _ = p.wait(t) +} + +// TestStandingStoreFailure: if the secret cannot be fetched after the +// token was replaced, the client has no new token, so the old one must stay +// valid and no alarm comes. +func TestStandingStoreFailure(t *testing.T) { + e, tok0, _ := useEnv(t) + e.main.set(func(f *fakeStore) { f.err = errors.New("store down") }) + res, _ := e.useToken("laptop", tok0) + mustStatus(t, res, http.StatusServiceUnavailable) + e.main.set(func(f *fakeStore) { f.err = nil }) + res, ans := e.useToken("laptop", tok0) + mustStatus(t, res, http.StatusOK) + res, _ = e.useToken("laptop", ans.GrantToken) + mustStatus(t, res, http.StatusOK) // the failed use did not count + if len(e.alarmEntries()) != 0 { + t.Error("an alarm after a failed fetch") + } +} diff --git a/internal/telegram/alarm_test.go b/internal/telegram/alarm_test.go new file mode 100644 index 0000000..aafc05a --- /dev/null +++ b/internal/telegram/alarm_test.go @@ -0,0 +1,108 @@ +package telegram + +import ( + "context" + "errors" + "strings" + "testing" + + "github.com/code6x/nodkey/internal/approval" + "github.com/code6x/nodkey/internal/audit" +) + +// TestStandingNotice: the notice of a standing use says so, and checkPrompt +// accepts the via only for a secrets notice. +func TestStandingNotice(t *testing.T) { + p := approval.Prompt{ + RequestID: testID(4), ShortCode: "k3f9", Kind: audit.KindSecrets, Client: "laptop", + Secrets: []string{"backup-token"}, Via: audit.ViaStanding, + } + tx, err := noticeText(p) + if err != nil { + t.Fatal(err) + } + if got := tx.plain.String(); !strings.Contains(got, "\n"+standingNoticeLine+"\n") { + t.Errorf("the notice is %q", got) + } + if err := checkPrompt(p, false); err == nil { + t.Error("a standing via in a message with buttons was accepted") + } + k := p + k.Kind, k.Secrets, k.Key, k.Hosts, k.HostKey, k.RemoteUser = audit.KindSSHLogin, nil, "web", []string{"web-1"}, + "SHA256:abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQ", "deploy" + if _, err := noticeText(k); err == nil { + t.Error("a standing via in an SSH notice was accepted") + } +} + +func alarmOf(cause audit.AlarmCause) approval.Alarm { + a := approval.Alarm{RequestID: testID(5), ShortCode: "k3f9", Client: "laptop", + Secrets: []string{"github-token"}, GrantID: testID(6), Cause: cause} + switch cause { + case audit.AlarmUnknownToken: + a.GrantID = "" + case audit.AlarmOldToken, audit.AlarmOverUses: + a.Ended = true + } + return a +} + +// TestAlarm: an alarm goes with a sound, with fixed text for the cause, and +// 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} { + 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 { + t.Fatalf("%d messages", len(calls)) + } + for _, c := range calls { + if c.field("disable_notification") == "true" || c.num("chat_id") != noticeChatID || c.has("reply_markup") || + !strings.Contains(c.str("text"), "🚨") || !strings.Contains(c.str("text"), "laptop") { + t.Errorf("alarm %s", c.Raw) + } + } + if !strings.Contains(calls[0].str("text"), "old token") || strings.Contains(calls[3].str("text"), "grant:") { + 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++ { + if err := e.a.Alarm(context.Background(), alarmOf(audit.AlarmOldToken)); err != nil { + t.Fatal(err) + } + } + if err := e.a.Alarm(context.Background(), alarmOf(audit.AlarmOldToken)); !errors.Is(err, ErrNoticeLimit) { + t.Errorf("over the limit: %v", err) + } + if err := e.a.Notify(context.Background(), approval.Prompt{}); err == nil { + t.Error("Notify sent a notice after the budget was used") + } +} + +// TestAlarmRefusals: an alarm that is not made by the core is not sent. +func TestAlarmRefusals(t *testing.T) { + e := newEnv(t) + for name, change := range map[string]func(*approval.Alarm){ + "a bad request ID": func(a *approval.Alarm) { a.RequestID = "x" }, + "a bad short code": func(a *approval.Alarm) { a.ShortCode = "" }, + "a bad client": func(a *approval.Alarm) { a.Client = "a b" }, + "a bad secret": func(a *approval.Alarm) { a.Secrets = []string{""} }, + "unknown with a grant": func(a *approval.Alarm) { a.Cause, a.Ended = audit.AlarmUnknownToken, false }, + "no grant": func(a *approval.Alarm) { a.GrantID = "" }, + "an unknown cause": func(a *approval.Alarm) { a.Cause = "x" }, + "ended, but not ending": func(a *approval.Alarm) { a.Cause = audit.AlarmOutsideWindow }, + } { + a := alarmOf(audit.AlarmOldToken) + change(&a) + if err := e.a.Alarm(context.Background(), a); err == nil { + t.Errorf("%s: sent", name) + } + } + if n := len(e.f.allCalls()); n != 0 { + t.Errorf("%d calls", n) + } +} diff --git a/internal/telegram/telegram.go b/internal/telegram/telegram.go index 45cd9ca..fa44e16 100644 --- a/internal/telegram/telegram.go +++ b/internal/telegram/telegram.go @@ -395,6 +395,29 @@ func (a *Approver) Notify(ctx context.Context, p approval.Prompt) error { }, nil) } +// Alarm sends the alarm of a standing grant to the notice chat, with a +// sound. It counts toward the notices per minute of Notify (DESIGN.md +// section 8): when the limit is reached it sends nothing and returns +// ErrNoticeLimit. +func (a *Approver) Alarm(ctx context.Context, al approval.Alarm) error { + if err := ctx.Err(); err != nil { + return err + } + t, err := alarmText(al) + if err != nil { + return err + } + if !a.takeNotice() { + return ErrNoticeLimit + } + return a.api.call(ctx, "sendMessage", sendMessageParams{ + ChatID: a.noticeChat, + Text: t.html.String(), + ParseMode: "HTML", + LinkPreviewOptions: noPreview, + }, nil) +} + // Alert sends msg to the approver's private chat, with a sound, for a // problem the person must see, such as a polling conflict or a torn line // in the audit log. msg is the broker's own text, never claimed text. It diff --git a/internal/telegram/text.go b/internal/telegram/text.go index 494d6b6..1087529 100644 --- a/internal/telegram/text.go +++ b/internal/telegram/text.go @@ -57,6 +57,7 @@ const claimedHead = "— claimed by the agent —" const ( noticeLine = `allowed by a "never ask" rule, without a tap` scheduledNoticeLine = `allowed by a scheduled grant, without a tap` + standingNoticeLine = `allowed by a standing grant, without a tap` // standingDenyLine is in the message of a standing grant. A Deny of // another request does not end the grant (DESIGN.md section 8). standingDenyLine = `A Deny of other requests does not end this grant. Use Revoke to end it.` @@ -254,9 +255,12 @@ func checkPrompt(p approval.Prompt, notice bool) error { return bad("windows outside a standing grant") } switch { - case notice && p.Via != "" && p.Via != audit.ViaScheduled, !notice && p.Via != "": + case notice && p.Via != "" && p.Via != audit.ViaScheduled && p.Via != audit.ViaStanding, !notice && p.Via != "": return bad("the via") } + if p.Via == audit.ViaStanding && p.Kind != audit.KindSecrets { + return bad("a standing use outside a secrets request") + } switch p.Kind { case audit.KindSecrets: if len(p.Secrets) == 0 || !allNames(p.Secrets) || p.Key != "" || len(p.Hosts) != 0 || p.HostKey != "" || @@ -386,6 +390,8 @@ func body(p approval.Prompt, notice bool, cv claimedValues, c caps) *text { if p.Kind == audit.KindPreapprove { t.write("\nnot used: grant " + p.GrantID + " ended with " + strconv.Itoa(p.UsesLeft) + " of " + strconv.Itoa(p.Uses) + " uses left") + } else if p.Via == audit.ViaStanding { + t.write("\n" + standingNoticeLine) } else if p.Via == audit.ViaScheduled { t.write("\n" + scheduledNoticeLine) } else { @@ -644,3 +650,48 @@ func standingEndLine(cause audit.EndCause, at string) string { } return "⛔ ended at " + at } + +// alarmText returns the text of the alarm of a standing grant. It holds +// the client, the secret names, the grant ID and a fixed text for the +// cause; no token and no claimed text. +func alarmText(a approval.Alarm) (*text, error) { + bad := func(what string) error { return errors.New("telegram: the alarm is not valid: " + what) } + switch { + case !validRequestID(a.RequestID): + return nil, bad("the request ID") + case !shortCodePattern.MatchString(a.ShortCode): + return nil, bad("the short code") + case !namePattern.MatchString(a.Client): + return nil, bad("the client name") + case len(a.Secrets) > 0 && !allNames(a.Secrets): + return nil, bad("the secrets") + case (a.GrantID == "") != (a.Cause == audit.AlarmUnknownToken) || (a.GrantID != "" && !validRequestID(a.GrantID)): + return nil, bad("the grant ID") + } + var what string + switch a.Cause { + case audit.AlarmOldToken: + what = "an old token was used again; it may be stolen. The grant is ended." + case audit.AlarmOverUses: + what = "more uses than allowed in one window. The grant is ended." + case audit.AlarmOutsideWindow: + 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." + default: + return nil, bad("the cause") + } + if a.Ended != (a.Cause == audit.AlarmOldToken || a.Cause == audit.AlarmOverUses) { + return nil, bad("the end") + } + t := &text{} + t.write("🚨 Standing grant alarm · request " + a.ShortCode + "\n" + what) + t.write("\n✅ machine: " + a.Client) + if len(a.Secrets) > 0 { + t.write("\n✅ secrets: " + strings.Join(a.Secrets, ", ")) + } + if a.GrantID != "" { + t.write("\n✅ grant: " + a.GrantID) + } + return t, nil +} From ae944afd2b1af928ecd98bddfce7779fe1633492 Mon Sep 17 00:00:00 2001 From: Geno Roupsky <185580+groupsky@users.noreply.github.com> Date: Thu, 29 Oct 2026 12:11:04 +0200 Subject: [PATCH 11/18] test: alarms through Telegram, failed audit write, cancel after rotation (#110) Co-Authored-By: Claude Opus 5.5 --- cmd/nodkey/broker_standing_linux_test.go | 89 ++++++++++++++++++++++++ cmd/nodkey/brokerenv_linux_test.go | 14 ++++ internal/api/types.go | 2 +- internal/broker/standinguse_test.go | 68 ++++++++++++++++++ internal/client/run.go | 2 +- internal/client/run_grant_test.go | 62 +++++++++++++++-- 6 files changed, 230 insertions(+), 7 deletions(-) create mode 100644 cmd/nodkey/broker_standing_linux_test.go diff --git a/cmd/nodkey/broker_standing_linux_test.go b/cmd/nodkey/broker_standing_linux_test.go new file mode 100644 index 0000000..995849a --- /dev/null +++ b/cmd/nodkey/broker_standing_linux_test.go @@ -0,0 +1,89 @@ +package main + +import ( + "net/http" + "slices" + "strings" + "syscall" + "testing" + "time" + + "github.com/code6x/nodkey/internal/api" + "github.com/code6x/nodkey/internal/grantkey" +) + +// TestBrokerStandingAlarmThroughTelegram: the alarm of an old token reaches +// the fake Telegram as a message with no buttons, the request is denied, +// and the grant ends: even the new token is refused afterwards. +func TestBrokerStandingAlarmThroughTelegram(t *testing.T) { + t.Parallel() + e := startListening(t, brokerOptions{standing: true}) + e.stderr.wait(t, lineListening) + c := e.httpClient() + + // Three windows that cover the whole day, in the broker's local time. + grant := api.PreapproveRequest{ + Secrets: []string{"github-token"}, Uses: 1, Reason: "job nightly-backup", + From: time.Now().Add(-time.Minute).UTC().Format(time.RFC3339), + Until: time.Now().AddDate(0, 0, 30).UTC().Format(time.RFC3339), + Windows: []api.Window{ + {Kind: api.WindowDaily, From: "00:00", To: "07:59"}, + {Kind: api.WindowDaily, From: "08:00", To: "15:59"}, + {Kind: api.WindowDaily, From: "16:00", To: "23:59"}, + }, + } + a := e.approve(c, api.PathPreapprove, grant) + mustStatus(t, a, http.StatusOK) + made, err := api.ReadAnswer[api.PreapproveAnswer](strings.NewReader(string(a.body))) + if err != nil { + t.Fatal(err) + } + tok0 := made.GrantToken + if _, err := grantkey.Parse(tok0); err != nil { + t.Fatalf("the token: %v", err) + } + + use := func(token string) brokerAnswer { + return e.postWithToken(c, api.PathSecrets, api.SecretsRequest{ + Secrets: []api.SecretRef{{Var: "GH_TOKEN", Name: "github-token"}}, Reason: "job nightly-backup", + }, token) + } + // A good use: no tap, a new token. + first := use(tok0) + mustStatus(t, first, http.StatusOK) + rotated, err := api.ReadAnswer[api.SecretsAnswer](strings.NewReader(string(first.body))) + if err != nil || rotated.GrantToken == "" || rotated.GrantToken == tok0 { + t.Fatalf("no new token in the answer: %v", err) + } + if e.tg.alertCount("Standing grant alarm") != 0 { + t.Fatal("an alarm for a good use") + } + asked := e.tg.asked() + + // The old token again: denied, one alarm with the fixed text. + mustStatus(t, use(tok0), http.StatusForbidden) + body := e.tg.waitAlert("Standing grant alarm") + if !strings.Contains(body, "an old token was used again") || !strings.Contains(body, "github-token") { + t.Errorf("the alarm: %s", body) + } + // The grant is over: the new token is refused, and nobody is asked. + mustStatus(t, use(rotated.GrantToken), http.StatusForbidden) + if e.tg.asked() != asked { + t.Error("the person was asked for a request with a token") + } + + e.signal(syscall.SIGINT) + e.mustExit(exitOK) + texts := e.tg.bodies() + e.stderr.text() + strings.Join(eventNames(e.auditEntries()), " ") + for _, tok := range []string{tok0, rotated.GrantToken} { + if strings.Contains(texts, tok) { + t.Error("a grant token is in the calls to Telegram, the journal or the audit log") + } + } + got := eventNames(e.auditEntries()) + for _, want := range []string{"grant_alarm:old_token", "grant_ended:alarm"} { + if !slices.Contains(got, want) { + t.Errorf("no %s entry in %q", want, got) + } + } +} diff --git a/cmd/nodkey/brokerenv_linux_test.go b/cmd/nodkey/brokerenv_linux_test.go index d14a288..ad0846b 100644 --- a/cmd/nodkey/brokerenv_linux_test.go +++ b/cmd/nodkey/brokerenv_linux_test.go @@ -331,6 +331,8 @@ type brokerOptions struct { newestRelease string // updatesOff adds [updates] check = false to the config. updatesOff bool + // standing adds [standing] and standing = true for github-token. + standing bool } // brokerEnv is nodkey broker in a child process, with its files and the @@ -494,6 +496,9 @@ ask = "always" hosts = ["db-1"] ` } + if opts.standing { + cfg = strings.Replace(cfg, "[secret.github-token]\n", "[secret.github-token]\nstanding = true\n", 1) + "\n[standing]\n" + } if opts.updatesOff { cfg += "\n[updates]\ncheck = false\n" } @@ -770,6 +775,12 @@ type brokerAnswer struct { // post sends body to path as JSON, and returns the answer. It does not // fail the test, so it may run in its own goroutine. func (e *brokerEnv) post(c *http.Client, path string, body any) brokerAnswer { + return e.postWithToken(c, path, body, "") +} + +// postWithToken is post with the header of a standing grant token, if +// token is not empty. +func (e *brokerEnv) postWithToken(c *http.Client, path string, body any, token string) brokerAnswer { data, err := json.Marshal(body) if err != nil { return brokerAnswer{err: err} @@ -781,6 +792,9 @@ func (e *brokerEnv) post(c *http.Client, path string, body any) brokerAnswer { return brokerAnswer{err: err} } req.Header.Set("Content-Type", api.ContentType) + if token != "" { + req.Header.Set(api.GrantTokenHeader, token) + } resp, err := c.Do(req) if err != nil { return brokerAnswer{err: err} diff --git a/internal/api/types.go b/internal/api/types.go index fc8156d..6e180e4 100644 --- a/internal/api/types.go +++ b/internal/api/types.go @@ -237,7 +237,7 @@ func (a *SecretsAnswer) Encode() ([]byte, error) { tail = `"}` end = `]}` ) - const tokenKey = `],"grant_token":"` + const tokenKey = `],"grant_token":"` //nolint:gosec // G101: a JSON key, not a credential. n := len(head) + len(end) if a.GrantToken != "" { for _, c := range []byte(a.GrantToken) { diff --git a/internal/broker/standinguse_test.go b/internal/broker/standinguse_test.go index bcd4c8d..5284f7d 100644 --- a/internal/broker/standinguse_test.go +++ b/internal/broker/standinguse_test.go @@ -340,3 +340,71 @@ func TestStandingStoreFailure(t *testing.T) { t.Error("an alarm after a failed fetch") } } + +// TestStandingSecretSentFailure: if secret_sent cannot be written, no value +// and no new token are sent, and the use is undone: the old token is valid +// again, with all its uses. +func TestStandingSecretSentFailure(t *testing.T) { + e, tok0, _ := useEnv(t) + e.audit.setFail(func(ev audit.Event) error { + if _, ok := ev.(audit.SecretSent); ok { + return errors.New("disk full") + } + return nil + }) + res, _ := e.useToken("laptop", tok0) + mustStatus(t, res, http.StatusServiceUnavailable) + if strings.Contains(string(res.body), "nkg_") || strings.Contains(string(res.body), e.values["github-token"]) { + t.Error("a token or a value in a failed answer") + } + e.audit.setFail(nil) + for range 2 { // both uses of the window are still there + res, ans := e.useToken("laptop", tok0) + mustStatus(t, res, http.StatusOK) + tok0 = ans.GrantToken + } + if len(e.alarmEntries()) != 0 || len(e.approver.Alarms()) != 0 { + t.Error("an alarm after a failed write") + } +} + +// TestStandingCancelAfterRotation: the client hangs up after the broker made +// the new token, so it never gets it. Nothing is sent, and the old token +// stays valid with all its uses (DESIGN.md section 8). +func TestStandingCancelAfterRotation(t *testing.T) { + e, tok0, _ := useEnv(t) + e.main.set(func(f *fakeStore) { f.block = true }) // the fetch comes after the rotation + ctx, cancel := context.WithCancel(t.Context()) + defer cancel() + data, _ := json.Marshal(secretsRequest("GH_TOKEN", "github-token")) + hr, err := http.NewRequestWithContext(ctx, http.MethodPost, "https://"+e.addr+api.PathSecrets, bytes.NewReader(data)) + if err != nil { + t.Fatal(err) + } + hr.Header.Set("Content-Type", api.ContentType) + hr.Header.Set(api.GrantTokenHeader, tok0) + done := make(chan struct{}) + go func() { + defer close(done) + if resp, err := e.httpClient("laptop").Do(hr); err == nil { + _ = resp.Body.Close() + } + }() + select { + case <-e.main.started: + case <-time.After(waitLimit): + t.Fatal("no fetch") + } + cancel() + <-done + e.svc.waitIdle(t, "laptop") + e.main.set(func(f *fakeStore) { f.block = false }) + for range 2 { // the cancelled use did not count + res, ans := e.useToken("laptop", tok0) + mustStatus(t, res, http.StatusOK) + tok0 = ans.GrantToken + } + if len(e.alarmEntries()) != 0 || len(e.approver.Alarms()) != 0 { + t.Error("an alarm after a cancel") + } +} diff --git a/internal/client/run.go b/internal/client/run.go index 55b6407..7466d80 100644 --- a/internal/client/run.go +++ b/internal/client/run.go @@ -673,7 +673,7 @@ func readGrantToken(path string) (grantkey.Token, error) { if err := CheckGrantTokenFile(path); err != nil { return grantkey.Token{}, err } - data, err := os.ReadFile(path) //nolint:gosec // G304: the path is the one that the person gave. + data, err := os.ReadFile(path) //nolint:gosec,forbidigo // G304: the path is the person's; CheckGrantTokenFile checked owner and mode above. if err != nil { return grantkey.Token{}, fmt.Errorf("%w: cannot read %q: %s", ErrTokenFile, path, errText(err)) } diff --git a/internal/client/run_grant_test.go b/internal/client/run_grant_test.go index 40c8f97..decb2b3 100644 --- a/internal/client/run_grant_test.go +++ b/internal/client/run_grant_test.go @@ -1,12 +1,15 @@ package client import ( + "context" "net/http" "os" "path/filepath" "strings" "sync" + "syscall" "testing" + "time" "github.com/code6x/nodkey/internal/api" ) @@ -73,7 +76,7 @@ func TestRunGrantToken(t *testing.T) { var inFile string r := testRunner(gb.b, nil, fx) r.exec = func(path string, argv, envv []string) error { - data, _ := os.ReadFile(f) + data, _ := os.ReadFile(f) //nolint:gosec // G304: the test's own file. inFile = string(data) return fx.exec(path, argv, envv) } @@ -108,10 +111,10 @@ func TestRunGrantTokenDeny(t *testing.T) { {"bad new token", func() string { return "nkg_" + strings.Repeat("a", 10) }, nil, exitUnavailable}, {"save fails", func() string { return newToken(t).Reveal() }, func(t *testing.T, dir, f string) { // The directory cannot take a new file; reading still works. - if err := os.Chmod(dir, 0o500); err != nil { + if err := os.Chmod(dir, 0o500); err != nil { //nolint:gosec // G302: a directory that cannot take a new file. t.Fatal(err) } - t.Cleanup(func() { _ = os.Chmod(dir, 0o700) }) + t.Cleanup(func() { _ = os.Chmod(dir, 0o700) }) //nolint:gosec // G302: a directory. }, exitDenied}, } { t.Run(tt.name, func(t *testing.T) { @@ -138,7 +141,7 @@ func TestRunGrantTokenDeny(t *testing.T) { if nt != "" { mustNotLeak(t, nt, stderr) } - if data, _ := os.ReadFile(f); string(data) != oldTok+"\n" { + if data, _ := os.ReadFile(f); string(data) != oldTok+"\n" { //nolint:gosec // G304: the test's own file. t.Error("the token file changed") } }) @@ -153,7 +156,7 @@ func TestRunGrantTokenBadFile(t *testing.T) { if err := os.WriteFile(open, []byte(newToken(t).Reveal()+"\n"), 0o600); err != nil { t.Fatal(err) } - if err := os.Chmod(open, 0o640); err != nil { + if err := os.Chmod(open, 0o640); err != nil { //nolint:gosec // G302: the test needs a file that the group can read. t.Fatal(err) } junk := writeToken(t, dir, "not a token") @@ -188,3 +191,52 @@ func TestRunTokenWithoutFlag(t *testing.T) { t.Error("the request had a token header, or was sent again") } } + +// TestRunGrantTokenCancel: a SIGINT while the broker works on a request with +// a token. The broker may have rotated the token already, but the client +// has no answer, so it keeps its token file as it is, starts no command, +// and never sends the request again (the broker gives the use back when the +// client hangs up). +func TestRunGrantTokenCancel(t *testing.T) { + t.Parallel() + oldTok := newToken(t).Reveal() + f := writeToken(t, t.TempDir(), oldTok) + entered := make(chan struct{}, 1) + fb := newFakeBroker(t, func(w http.ResponseWriter, r *http.Request) { + select { + case entered <- struct{}{}: + default: + } + <-r.Context().Done() // the broker works until the client hangs up + }) + ctx, cancel := context.WithCancelCause(t.Context()) + defer cancel(nil) + fx := &fakeExec{} + done := make(chan int, 1) + var stderr string + go func() { + var code int + code, stderr = runIn(ctx, testRunner(fb.b, nil, fx), + "--secret", "GH_TOKEN=github-token", "--grant-token", f, "--", self(t)) + done <- code + }() + select { + case <-entered: + case <-time.After(waitLimit): + t.Fatal("the broker got no request") + } + cancel(interrupted{sig: syscall.SIGINT}) + if code := <-done; code != 130 || lastLine(stderr) != "nodkey: cancelled by SIGINT (exit 130)" { + t.Errorf("exit code %d, stderr %q", code, safe(stderr)) + } + if len(fx.ran()) != 0 { + t.Error("the command started") + } + if n := fb.requests.Load(); n != 1 { + t.Errorf("%d requests, want 1 (no retry)", n) + } + if data, _ := os.ReadFile(f); string(data) != oldTok+"\n" { //nolint:gosec // G304: the test's own file. + t.Error("the token file changed") + } + mustNotLeak(t, oldTok, stderr) +} From 136a570d68ed7b7ce498a99175be9d91540a37d5 Mon Sep 17 00:00:00 2001 From: Geno Roupsky <185580+groupsky@users.noreply.github.com> Date: Thu, 29 Oct 2026 12:11:45 +0200 Subject: [PATCH 12/18] test: no gap in the windows of the alarm test (#110) Co-Authored-By: Claude Opus 5.5 --- cmd/nodkey/broker_standing_linux_test.go | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/cmd/nodkey/broker_standing_linux_test.go b/cmd/nodkey/broker_standing_linux_test.go index 995849a..49ca89b 100644 --- a/cmd/nodkey/broker_standing_linux_test.go +++ b/cmd/nodkey/broker_standing_linux_test.go @@ -21,14 +21,18 @@ func TestBrokerStandingAlarmThroughTelegram(t *testing.T) { e.stderr.wait(t, lineListening) c := e.httpClient() - // Three windows that cover the whole day, in the broker's local time. + // Three windows that cover the whole day, in the broker's local time, + // but the last minute: To is at most 23:59 and not part of the window. + if now := time.Now(); now.Hour() == 23 && now.Minute() >= 58 { + t.Skip("too close to midnight, outside the windows") + } grant := api.PreapproveRequest{ Secrets: []string{"github-token"}, Uses: 1, Reason: "job nightly-backup", From: time.Now().Add(-time.Minute).UTC().Format(time.RFC3339), Until: time.Now().AddDate(0, 0, 30).UTC().Format(time.RFC3339), Windows: []api.Window{ - {Kind: api.WindowDaily, From: "00:00", To: "07:59"}, - {Kind: api.WindowDaily, From: "08:00", To: "15:59"}, + {Kind: api.WindowDaily, From: "00:00", To: "08:00"}, + {Kind: api.WindowDaily, From: "08:00", To: "16:00"}, {Kind: api.WindowDaily, From: "16:00", To: "23:59"}, }, } From 4c2254ec5af75fc33439b92354dd1aa5ecae77dd Mon Sep 17 00:00:00 2001 From: Geno Roupsky <185580+groupsky@users.noreply.github.com> Date: Thu, 29 Oct 2026 12:17:13 +0200 Subject: [PATCH 13/18] fix: let a client cancel its standing grants (#110) Co-Authored-By: Claude Opus 5.5 --- DESIGN.md | 4 ++- internal/approval/grants.go | 4 +-- internal/broker/standing_test.go | 56 ++++++++++++++++++++++++++++++-- 3 files changed, 58 insertions(+), 6 deletions(-) diff --git a/DESIGN.md b/DESIGN.md index cede1ed..40cd3eb 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -1570,7 +1570,9 @@ client, the secrets, the windows, the last use and the end date. The bot command `/grants` from the approver shows the same list. **End.** A standing grant ends when its end date comes, on Revoke, on an -alarm that ends it, on a renewal, or at a broker stop. A **Deny** of +alarm that ends it, on a renewal, on a cancel by its own client (`nodkey +preapprove --cancel`, or the broker, when the answer with the token cannot +be written; cause `cancelled`), or at a broker stop. A **Deny** of another request of the same client and secret does **not** end it: an "Approve once" Deny at noon must not break the night job. The Deny message says "standing grants stay; use Revoke to end them". At a stop the grants diff --git a/internal/approval/grants.go b/internal/approval/grants.go index 2d87b07..93830c1 100644 --- a/internal/approval/grants.go +++ b/internal/approval/grants.go @@ -508,7 +508,7 @@ func (c *Core) revoke(a Answer) error { return nil } -// CancelGrant ends the scheduled grant id of client at the client's +// CancelGrant ends the scheduled or standing grant id of client at the client's // request, without a tap. It returns the audit entry, which the caller // writes. It reports false for an ID that no grant of client has, and the // answer is the same for the ID of another client: a client learns @@ -518,7 +518,7 @@ func (c *Core) CancelGrant(client, id string) (audit.GrantEnded, bool) { defer c.mu.Unlock() c.sweepGrants(c.now()) // a grant past its end is gone ended := c.endWhere(func(g *grant) audit.EndCause { - if g.scheduled() && g.client == client && g.id == id { + if g.audited() && g.client == client && g.id == id { return audit.EndCancelled } return "" diff --git a/internal/broker/standing_test.go b/internal/broker/standing_test.go index 6529840..4d2c756 100644 --- a/internal/broker/standing_test.go +++ b/internal/broker/standing_test.go @@ -79,9 +79,12 @@ func TestStandingGrantIsMade(t *testing.T) { if n := e.main.fetched(); n != 0 { t.Errorf("a standing grant fetched %d values", n) } - // The scheduled grants are not touched: this grant is no scheduled one. - if _, ok := e.core.CancelGrant("laptop", ans.ID); ok { - t.Error("the cancel of a scheduled grant ended a standing grant") + // The client can cancel its own standing grant, but not that of another. + if _, ok := e.core.CancelGrant("ci", ans.ID); ok { + t.Error("another client cancelled the grant") + } + if end, ok := e.core.CancelGrant("laptop", ans.ID); !ok || end.Cause != audit.EndCancelled { + t.Errorf("the cancel of a standing grant: %+v, %v", end, ok) } } @@ -210,3 +213,50 @@ func TestStandingRevokeAndStop(t *testing.T) { t.Error("no grant_ended:stopped for the standing grant") } } + +// TestStandingCancel: the client can cancel its own standing grant, the +// token stops working, and another client or an unknown ID finds nothing. +func TestStandingCancel(t *testing.T) { + e, tok, id := useEnv(t) + cancel := func(client, id string) *result { + return e.post(t.Context(), e.httpClient(client), api.PathPreapproveCancel, api.PreapproveCancelRequest{ID: id}) + } + mustStatus(t, cancel("ci", id), http.StatusForbidden) + mustStatus(t, cancel("laptop", "AAAAAAAAAAAAAAAAAAAAAA"), http.StatusForbidden) + before := len(e.audit.all()) + mustStatus(t, cancel("laptop", id), http.StatusOK) + events := e.audit.waitFor(t, before+1)[before:] + mustEvents(t, events, "grant_ended:cancelled") + if end := events[0].(audit.GrantEnded); end.GrantID != id || end.Kind != audit.KindStanding { + t.Errorf("entry %+v", end) + } + mustStatus(t, cancel("laptop", id), http.StatusForbidden) + // The token is unknown now: denied, and no use. + res, _ := e.useToken("laptop", tok) + mustStatus(t, res, http.StatusForbidden) +} + +// TestStandingAnswerNotWritten: the answer with the token cannot be +// written, so nobody has the token. The broker cancels the grant. +func TestStandingAnswerNotWritten(t *testing.T) { + wrap := func(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path == api.PathPreapprove { + w = failWriter{w} + } + next.ServeHTTP(w, r) + }) + } + e := newEnv(t, envOptions{wrap: wrap, config: standingOn}) + c := e.httpClient("laptop") + p := e.start(c, api.PathPreapprove, standReq("github-token")) + id := e.waitAsked(1)[0].Prompt.RequestID + e.tap(id, audit.AnswerApproveOnce) + p.wait(t) + events := e.audit.waitFor(t, 3) + mustEvents(t, events, "request decision:approve_once grant_ended:cancelled") + e.clock.Advance(time.Hour) + if n := len(e.alarmEntries()); n != 0 { + t.Errorf("%d alarms", n) + } +} From 690ccf85a35d8c28fcf41959f7f08fef00cfeb43 Mon Sep 17 00:00:00 2001 From: Geno Roupsky <185580+groupsky@users.noreply.github.com> Date: Thu, 29 Oct 2026 12:17:41 +0200 Subject: [PATCH 14/18] fix: read the grant token file without following links (#110) Co-Authored-By: Claude Opus 5.5 --- internal/client/run.go | 16 +++++++++++++++- internal/client/run_grant_test.go | 26 ++++++++++++++++++++++++++ 2 files changed, 41 insertions(+), 1 deletion(-) diff --git a/internal/client/run.go b/internal/client/run.go index 7466d80..73049f6 100644 --- a/internal/client/run.go +++ b/internal/client/run.go @@ -673,7 +673,21 @@ func readGrantToken(path string) (grantkey.Token, error) { if err := CheckGrantTokenFile(path); err != nil { return grantkey.Token{}, err } - data, err := os.ReadFile(path) //nolint:gosec,forbidigo // G304: the path is the person's; CheckGrantTokenFile checked owner and mode above. + // Open without following a link, then check the open file, so a link put + // in place after the check above is not read. + f, err := os.OpenFile(path, os.O_RDONLY|noFollow, 0) //nolint:gosec,forbidigo // G304: the path is the person's; the open file is checked below. + if err != nil { + return grantkey.Token{}, fmt.Errorf("%w: cannot read %q: %s", ErrTokenFile, path, errText(err)) + } + defer func() { _ = f.Close() }() + fi, err := f.Stat() + if err != nil { + return grantkey.Token{}, fmt.Errorf("%w: cannot look at %q: %s", ErrTokenFile, path, errText(err)) + } + if err := checkTokenFileInfo(path, fi); err != nil { + return grantkey.Token{}, err + } + data, err := io.ReadAll(io.LimitReader(f, 4096)) if err != nil { return grantkey.Token{}, fmt.Errorf("%w: cannot read %q: %s", ErrTokenFile, path, errText(err)) } diff --git a/internal/client/run_grant_test.go b/internal/client/run_grant_test.go index decb2b3..6df3cb2 100644 --- a/internal/client/run_grant_test.go +++ b/internal/client/run_grant_test.go @@ -240,3 +240,29 @@ func TestRunGrantTokenCancel(t *testing.T) { } mustNotLeak(t, oldTok, stderr) } + +// TestRunGrantTokenSymlink: a symbolic link is not read, even when the file +// it points to is a good token file. Nothing is sent and no command runs. +func TestRunGrantTokenSymlink(t *testing.T) { + t.Parallel() + dir := t.TempDir() + target := writeToken(t, dir, newToken(t).Reveal()) + link := filepath.Join(dir, "link") + if err := os.Symlink(target, link); err != nil { + t.Fatal(err) + } + gb := newGrantRunBroker(t, newToken(t).Reveal()) + fx := &fakeExec{} + code, stderr := runGrant(t, gb, fx, "--grant-token", link) + if code == exitOK || len(fx.ran()) != 0 || len(gb.seen()) != 0 { + t.Errorf("exit %d, %d requests, %d commands; stderr %q", code, len(gb.seen()), len(fx.ran()), safe(stderr)) + } + if _, err := readGrantToken(link); err == nil { + t.Error("readGrantToken read through a link") + } + // The open itself refuses a link, with no check before it. + if f, err := os.OpenFile(link, os.O_RDONLY|noFollow, 0); err == nil { //nolint:gosec,forbidigo // G304: the test's own link. + _ = f.Close() + t.Error("the open followed a link") + } +} From 3851b8d9fb623a3a36937d3aa37238a2d6ee5712 Mon Sep 17 00:00:00 2001 From: Geno Roupsky <185580+groupsky@users.noreply.github.com> Date: Thu, 29 Oct 2026 12:19:25 +0200 Subject: [PATCH 15/18] fix: end a grant whose token another client sends; mark outside_window only after a sent alarm (#110) Co-Authored-By: Claude Opus 5.5 --- DESIGN.md | 7 +- THREAT-MODEL.md | 5 +- internal/approval/approvaltest/approver.go | 7 ++ internal/approval/grants.go | 5 +- internal/approval/standinguse.go | 48 +++++++++++-- internal/approval/window_test.go | 4 +- internal/broker/standinguse_test.go | 82 +++++++++++++++++++++- 7 files changed, 144 insertions(+), 14 deletions(-) diff --git a/DESIGN.md b/DESIGN.md index 40cd3eb..ecefe28 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -1545,14 +1545,17 @@ grant ends, to know the old tokens. **Alarms.** An alarm is a Telegram message with sound, and a `grant_alarm` entry with a cause. It counts toward the limit of 20 notices in all. One grant sends at most one alarm in a window. For a valid token outside the -windows, the limit is one alarm in a calendar day. +windows (also before the start of the grant), the limit is one alarm in a +calendar day. The day counts only when the alarm was sent: after a failed +send, the next request tries again. | case | request | grant | cause | |---|---|---|---| | an old token | denied | **ends** (`alarm`) | `old_token` | | more uses in a window than allowed | denied | **ends** (`alarm`) | `over_uses` | | a valid token outside the windows | denied | stays | `outside_window` | -| an unknown token, or the token of another client | denied | none | `unknown_token` (rate limited) | +| a token of another client's grant, current or old | denied | **ends that grant** (`alarm`) | `old_token`, with the ID of that grant | +| an unknown token | denied | none | `unknown_token` (rate limited) | | a window ended with no use | none | stays | `missed_window` | An old token is taken as stolen. A retry after a lost answer looks the diff --git a/THREAT-MODEL.md b/THREAT-MODEL.md index 2190883..5b6dbcd 100644 --- a/THREAT-MODEL.md +++ b/THREAT-MODEL.md @@ -1046,7 +1046,10 @@ each use. The token is in a file on the client. the client saves the new token before it uses the secrets and never retries by itself. A new grant is one tap, so there is no pause state. - A valid token outside the windows is denied with an alarm, and the grant - stays. An unknown token, or the token of another client, is denied with + stays; this holds also before the start of the grant, and the day's + alarm counts only if it was sent. The current or old token of another + client's grant has left its machine: the request is denied and that + grant ends, with an alarm that names it. An unknown token is denied with a rate-limited alarm; it may be a guess. Alarms have sound, count toward the 20 notices in all, and one grant sends at most one in a window. - Run the job as its own system user, so an agent that runs as another diff --git a/internal/approval/approvaltest/approver.go b/internal/approval/approvaltest/approver.go index d4b075c..9150539 100644 --- a/internal/approval/approvaltest/approver.go +++ b/internal/approval/approvaltest/approver.go @@ -182,6 +182,13 @@ func (a *Approver) FailNotify(err error) { a.notifyErr = err } +// FailAlarm makes Alarm fail with err. nil makes it work again. +func (a *Approver) FailAlarm(err error) { + a.mu.Lock() + defer a.mu.Unlock() + a.alarmErr = err +} + // FailEnd makes End return err. nil makes it work again. func (a *Approver) FailEnd(err error) { a.mu.Lock() diff --git a/internal/approval/grants.go b/internal/approval/grants.go index 93830c1..aa43fc0 100644 --- a/internal/approval/grants.go +++ b/internal/approval/grants.go @@ -147,9 +147,12 @@ func (g *grant) live(now moment) bool { // this window are spent, for the alarms. A standing grant without a valid // window cannot be used. func (g *grant) tryUse(now moment) useResult { - if !g.standing() || len(g.windows) == 0 || now.wall.Before(g.start) || !now.before(g.end) { + if !g.standing() || len(g.windows) == 0 || !now.before(g.end) { return useNone } + if now.wall.Before(g.start) { + return useOutsideWindow // a valid token before the start is outside the windows + } key, _, in := windowAt(g.windows, now.wall) if !in { return useOutsideWindow diff --git a/internal/approval/standinguse.go b/internal/approval/standinguse.go index 1112bb5..b33c61e 100644 --- a/internal/approval/standinguse.go +++ b/internal/approval/standinguse.go @@ -56,6 +56,12 @@ type standingUse struct { alarm audit.AlarmCause grantID string // of the alarm ended bool + // markG is the grant whose daily outside_window mark this use took, and + // markPrev the mark before. SendAlarm gives the mark back if the alarm + // is not sent. + markG *grant + markPrev string + markNow string } // UseStanding decides a request with a token: the use path of a standing @@ -89,18 +95,29 @@ func (c *Core) UseStanding(r StandingRequest) *Result { c.sweepGrants(now) cs := c.client(p.client) // Every grant and hash is compared, with no early exit. - var cur, old *grant + // A token of a grant of another client has left its machine: that grant + // is the one that is named and ended (foreign). + var cur, old, foreign *grant if r.Token != nil { for _, g := range c.grants { - if !g.standing() || g.client != p.client { + if !g.standing() { continue } + mine := g.client == p.client if grantkey.Equal(g.tokenHash, *r.Token) { - cur = g + if mine { + cur = g + } else { + foreign = g + } } for _, h := range g.oldHashes { if grantkey.Equal(h, *r.Token) { - old = g + if mine { + old = g + } else { + foreign = g + } } } } @@ -124,6 +141,8 @@ func (c *Core) UseStanding(r StandingRequest) *Result { return res } switch { + case cur == nil && old == nil && foreign != nil: + return deny(foreign, audit.AlarmOldToken, true) case cur == nil && old == nil: if cs.unknown.recent(now.mono) >= MaxUnknownTokensPerMinute { return c.denied(p, audit.CauseLimit) @@ -145,8 +164,11 @@ func (c *Core) UseStanding(r StandingRequest) *Result { case useOutsideWindow: res := deny(g, audit.AlarmOutsideWindow, false) if mark := now.wall.Format("2006-01-02"); g.alarmMark == mark { - res.su.alarm = "" // one alarm in a window + res.su.alarm = "" // one alarm in a day } else { + // The mark is taken now, so a second request does not send a + // second alarm. SendAlarm gives it back if the alarm is not sent. + res.su.markG, res.su.markPrev, res.su.markNow = g, g.alarmMark, mark g.alarmMark = mark } return res @@ -230,6 +252,7 @@ func (r *Result) SendAlarm(ctx context.Context) { c, su := r.core, r.su al, ok := c.approver.(Alarmer) if !ok { + c.giveBackMark(su) return } actx, cancel := c.callContext(context.WithoutCancel(ctx), endTimeout) @@ -239,11 +262,26 @@ func (r *Result) SendAlarm(ctx context.Context) { Secrets: slices.Clone(r.Subject.Secrets), GrantID: su.grantID, Cause: su.alarm, Ended: su.ended, }) if err != nil { + c.giveBackMark(su) c.logger.LogAttrs(actx, slog.LevelWarn, "approval: cannot send the alarm of a standing grant", slog.String("request_id", r.RequestID), slog.String("cause", string(su.alarm)), slog.Any("error", err)) } } +// giveBackMark takes back the daily outside_window mark that su took, because +// its alarm was not sent: the next request may send one. A later mark of the +// same grant is left alone. +func (c *Core) giveBackMark(su *standingUse) { + if su.markG == nil { + return + } + c.mu.Lock() + defer c.mu.Unlock() + if su.markG.alarmMark == su.markNow { + su.markG.alarmMark = su.markPrev + } +} + // newTokenHash is the hash of the new token in hex, for the audit log. func (r *Result) newTokenHash() string { if r.su == nil { diff --git a/internal/approval/window_test.go b/internal/approval/window_test.go index ff8be23..055f886 100644 --- a/internal/approval/window_test.go +++ b/internal/approval/window_test.go @@ -181,8 +181,8 @@ func TestStandingUseNotYetStartedAndNoWindows(t *testing.T) { w := []window{{days: anyDay, from: 0, to: 24 * time.Hour}} g := standingGrant(w, 1, end) g.start = time.Date(2026, 11, 10, 0, 0, 0, 0, utc) - if g.live(at(time.Date(2026, 11, 9, 12, 0, 0, 0, utc))) || g.tryUse(at(time.Date(2026, 11, 9, 12, 0, 0, 0, utc))) != useNone { - t.Error("a grant before its start must not be live or usable") + if g.live(at(time.Date(2026, 11, 9, 12, 0, 0, 0, utc))) || g.tryUse(at(time.Date(2026, 11, 9, 12, 0, 0, 0, utc))) != useOutsideWindow { + t.Error("a grant before its start must not be live, and a use is outside the windows") } // No windows: live, but never usable as a standing grant (fail closed). n := standingGrant(nil, 1, end) diff --git a/internal/broker/standinguse_test.go b/internal/broker/standinguse_test.go index 5284f7d..081986c 100644 --- a/internal/broker/standinguse_test.go +++ b/internal/broker/standinguse_test.go @@ -210,7 +210,7 @@ func TestStandingOutsideWindow(t *testing.T) { } // TestStandingUnknownToken: no grant, an alarm with sound, rate limited; -// a bad format is the same; a token of another client is the same. +// a bad format is the same. func TestStandingUnknownToken(t *testing.T) { e, tok0, _ := useEnv(t) other, err := grantkey.New() @@ -221,8 +221,8 @@ func TestStandingUnknownToken(t *testing.T) { mustStatus(t, res, http.StatusForbidden) res, _ = e.useToken("laptop", "not a token") mustStatus(t, res, http.StatusForbidden) - // The token of laptop's grant, sent by ci. - res, _ = e.useToken("ci", tok0) + // A token that no grant has, sent by ci. + res, _ = e.useToken("ci", other.Reveal()) mustStatus(t, res, http.StatusForbidden) al := e.alarmEntries() if len(al) != 3 || al[2].Cause != audit.AlarmUnknownToken || al[2].Client != "ci" || al[0].Cause != audit.AlarmUnknownToken || al[0].GrantID != "" { @@ -408,3 +408,79 @@ func TestStandingCancelAfterRotation(t *testing.T) { t.Error("an alarm after a cancel") } } + +// TestStandingForeignToken: the token of a grant of another client, current +// or old, has left its machine. The request is denied and the grant ends, +// with an alarm that names it. +func TestStandingForeignToken(t *testing.T) { + for _, old := range []bool{false, true} { + e, tok0, id := useEnv(t) + tok := tok0 + if old { + _, ans := e.useToken("laptop", tok0) + tok = tok0 + tok0 = ans.GrantToken + } + res, _ := e.useToken("ci", tok) + mustStatus(t, res, http.StatusForbidden) + al := e.alarmEntries() + if len(al) != 1 || al[0].Cause != audit.AlarmOldToken || al[0].GrantID != id { + t.Fatalf("old=%v alarms: %+v", old, al) + } + if as := e.approver.Alarms(); len(as) != 1 || !as[0].Ended || as[0].GrantID != id { + t.Errorf("old=%v the alarm: %+v", old, as) + } + var ended bool + for _, ev := range e.audit.all() { + if g, ok := ev.(audit.GrantEnded); ok && g.GrantID == id && g.Cause == audit.EndAlarm { + ended = true + } + } + if !ended { + t.Errorf("old=%v: no grant_ended:alarm", old) + } + res, _ = e.useToken("laptop", tok0) + mustStatus(t, res, http.StatusForbidden) + e.noToken(tok, tok0) + } +} + +// TestStandingOutsideWindowAlarmFails: the daily mark is set only by an +// alarm that went out. After a failed send the next request tries again. +func TestStandingOutsideWindowAlarmFails(t *testing.T) { + e, tok0, id := useEnv(t) + e.clock.Advance(3 * time.Hour) // 15:00 + e.approver.FailAlarm(errors.New("made-up: the alarm cannot be sent")) + res, _ := e.useToken("laptop", tok0) + mustStatus(t, res, http.StatusForbidden) + if n := len(e.approver.Alarms()); n != 0 { + t.Fatalf("%d alarms went out", n) + } + e.approver.FailAlarm(nil) + for range 2 { + res, _ = e.useToken("laptop", tok0) + mustStatus(t, res, http.StatusForbidden) + } + if as := e.approver.Alarms(); len(as) != 1 || as[0].Cause != audit.AlarmOutsideWindow || as[0].GrantID != id { + t.Errorf("the alarms: %+v", as) + } +} + +// TestStandingBeforeStart: a valid token before the start of the grant is +// denied, with an outside_window alarm, and the grant stays. +func TestStandingBeforeStart(t *testing.T) { + e := newEnv(t, envOptions{config: standingOn}) + r := standReq("github-token") + r.From = "2026-10-14T20:00:00Z" + r.Windows = []api.Window{{Kind: api.WindowDaily, From: "11:00", To: "13:00"}} + ans := e.preapprove(e.httpClient("laptop"), r, 1) + res, _ := e.useToken("laptop", ans.GrantToken) + mustStatus(t, res, http.StatusForbidden) + al := e.alarmEntries() + if len(al) != 1 || al[0].Cause != audit.AlarmOutsideWindow || al[0].GrantID != ans.ID { + t.Fatalf("alarms: %+v", al) + } + if as := e.approver.Alarms(); len(as) != 1 || as[0].Ended { + t.Errorf("the alarms: %+v", as) + } +} From ba51f50320ce01849f95a5a51ad51032d9332723 Mon Sep 17 00:00:00 2001 From: Geno Roupsky <185580+groupsky@users.noreply.github.com> Date: Thu, 29 Oct 2026 12:19:32 +0200 Subject: [PATCH 16/18] docs: keep the grant token file in a private folder (#110) Co-Authored-By: Claude Opus 5.5 --- docs/setup.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/docs/setup.md b/docs/setup.md index b415c10..adfa1da 100644 --- a/docs/setup.md +++ b/docs/setup.md @@ -1374,6 +1374,10 @@ 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 From 272ffe45e6f45168df1b6d0f78762af422010b9e Mon Sep 17 00:00:00 2001 From: Geno Roupsky <185580+groupsky@users.noreply.github.com> Date: Thu, 29 Oct 2026 12:20:16 +0200 Subject: [PATCH 17/18] chore: drop unused nolint (#110) Co-Authored-By: Claude Opus 5.5 --- internal/client/run.go | 2 +- internal/client/run_grant_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/client/run.go b/internal/client/run.go index 73049f6..bac0c2a 100644 --- a/internal/client/run.go +++ b/internal/client/run.go @@ -675,7 +675,7 @@ func readGrantToken(path string) (grantkey.Token, error) { } // Open without following a link, then check the open file, so a link put // in place after the check above is not read. - f, err := os.OpenFile(path, os.O_RDONLY|noFollow, 0) //nolint:gosec,forbidigo // G304: the path is the person's; the open file is checked below. + f, err := os.OpenFile(path, os.O_RDONLY|noFollow, 0) //nolint:gosec // G304: the path is the person's; the open file is checked below. if err != nil { return grantkey.Token{}, fmt.Errorf("%w: cannot read %q: %s", ErrTokenFile, path, errText(err)) } diff --git a/internal/client/run_grant_test.go b/internal/client/run_grant_test.go index 6df3cb2..ab83c20 100644 --- a/internal/client/run_grant_test.go +++ b/internal/client/run_grant_test.go @@ -261,7 +261,7 @@ func TestRunGrantTokenSymlink(t *testing.T) { t.Error("readGrantToken read through a link") } // The open itself refuses a link, with no check before it. - if f, err := os.OpenFile(link, os.O_RDONLY|noFollow, 0); err == nil { //nolint:gosec,forbidigo // G304: the test's own link. + if f, err := os.OpenFile(link, os.O_RDONLY|noFollow, 0); err == nil { //nolint:gosec // G304: the test's own link. _ = f.Close() t.Error("the open followed a link") } From 464e7863959c3c6fe1dd97871043917ff04c7b08 Mon Sep 17 00:00:00 2001 From: Geno Roupsky <185580+groupsky@users.noreply.github.com> Date: Thu, 29 Oct 2026 12:23:01 +0200 Subject: [PATCH 18/18] fix: name the owner in a foreign token alarm; do not wait on a pipe (#110) Co-Authored-By: Claude Opus 5.5 --- DESIGN.md | 2 +- internal/approval/standinguse.go | 11 ++++++-- internal/broker/standinguse_test.go | 2 +- internal/client/tokenfile_unix.go | 5 ++-- internal/client/tokenfile_unix_test.go | 32 ++++++++++++++++++++++ internal/telegram/alarm_test.go | 38 ++++++++++++++++++++------ internal/telegram/text.go | 5 ++++ 7 files changed, 81 insertions(+), 14 deletions(-) create mode 100644 internal/client/tokenfile_unix_test.go diff --git a/DESIGN.md b/DESIGN.md index ecefe28..4bb3c4c 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -1554,7 +1554,7 @@ send, the next request tries again. | an old token | denied | **ends** (`alarm`) | `old_token` | | more uses in a window than allowed | denied | **ends** (`alarm`) | `over_uses` | | a valid token outside the windows | denied | stays | `outside_window` | -| a token of another client's grant, current or old | denied | **ends that grant** (`alarm`) | `old_token`, with the ID of that grant | +| a token of another client's grant, current or old | denied | **ends that grant** (`alarm`) | `old_token`, with the ID of that grant; the message names the machine of that grant | | an unknown token | denied | none | `unknown_token` (rate limited) | | a window ended with no use | none | stays | `missed_window` | diff --git a/internal/approval/standinguse.go b/internal/approval/standinguse.go index b33c61e..6aa6951 100644 --- a/internal/approval/standinguse.go +++ b/internal/approval/standinguse.go @@ -23,7 +23,10 @@ type Alarm struct { Secrets []string // GrantID is "" for an unknown token. GrantID string - Cause audit.AlarmCause + // Owner is the machine of the grant when it is not Client: Client sent + // a token of another machine's grant. + Owner string + Cause audit.AlarmCause // Ended says that the alarm ended the grant. Ended bool } @@ -55,6 +58,7 @@ type standingUse struct { token grantkey.Token alarm audit.AlarmCause grantID string // of the alarm + owner string // of the grant, when it is not the client's ended bool // markG is the grant whose daily outside_window mark this use took, and // markPrev the mark before. SendAlarm gives the mark back if the alarm @@ -127,6 +131,9 @@ func (c *Core) UseStanding(r StandingRequest) *Result { su := &standingUse{p: p, alarm: cause} if g != nil { su.grantID, res.Subject.GrantID = g.id, g.id + if g.client != p.client { + su.owner = g.client + } } if end { su.ended = true @@ -259,7 +266,7 @@ func (r *Result) SendAlarm(ctx context.Context) { defer cancel() err := al.Alarm(actx, Alarm{ RequestID: r.RequestID, ShortCode: r.ShortCode, Client: r.Subject.Client, - Secrets: slices.Clone(r.Subject.Secrets), GrantID: su.grantID, Cause: su.alarm, Ended: su.ended, + Secrets: slices.Clone(r.Subject.Secrets), GrantID: su.grantID, Owner: su.owner, Cause: su.alarm, Ended: su.ended, }) if err != nil { c.giveBackMark(su) diff --git a/internal/broker/standinguse_test.go b/internal/broker/standinguse_test.go index 081986c..c86eab4 100644 --- a/internal/broker/standinguse_test.go +++ b/internal/broker/standinguse_test.go @@ -427,7 +427,7 @@ func TestStandingForeignToken(t *testing.T) { if len(al) != 1 || al[0].Cause != audit.AlarmOldToken || al[0].GrantID != id { t.Fatalf("old=%v alarms: %+v", old, al) } - if as := e.approver.Alarms(); len(as) != 1 || !as[0].Ended || as[0].GrantID != id { + if as := e.approver.Alarms(); len(as) != 1 || !as[0].Ended || as[0].GrantID != id || as[0].Owner != "laptop" { t.Errorf("old=%v the alarm: %+v", old, as) } var ended bool diff --git a/internal/client/tokenfile_unix.go b/internal/client/tokenfile_unix.go index d40c21b..79037ab 100644 --- a/internal/client/tokenfile_unix.go +++ b/internal/client/tokenfile_unix.go @@ -14,5 +14,6 @@ func ownedByUser(fi fs.FileInfo) bool { return ok && int(st.Uid) == os.Geteuid() } -// noFollow makes open refuse a symbolic link. -const noFollow = syscall.O_NOFOLLOW +// noFollow makes open refuse a symbolic link, and not wait on a named +// pipe that was put in place of the file. +const noFollow = syscall.O_NOFOLLOW | syscall.O_NONBLOCK diff --git a/internal/client/tokenfile_unix_test.go b/internal/client/tokenfile_unix_test.go new file mode 100644 index 0000000..2dbcc8c --- /dev/null +++ b/internal/client/tokenfile_unix_test.go @@ -0,0 +1,32 @@ +//go:build unix + +package client + +import ( + "os" + "path/filepath" + "syscall" + "testing" +) + +// TestNoFollowFIFO: the open of the token file does not wait on a named +// pipe that was put in place of the file. +func TestNoFollowFIFO(t *testing.T) { + t.Parallel() + p := filepath.Join(t.TempDir(), "fifo") + if err := syscall.Mkfifo(p, 0o600); err != nil { + t.Skipf("no named pipe here: %v", err) + } + f, err := os.OpenFile(p, os.O_RDONLY|noFollow, 0) //nolint:gosec // G304: the test's own pipe. + if err != nil { + t.Fatalf("the open: %v", err) + } + defer func() { _ = f.Close() }() + fi, err := f.Stat() + if err != nil { + t.Fatal(err) + } + if err := checkTokenFileInfo(p, fi); err == nil { + t.Error("a named pipe passed the check") + } +} diff --git a/internal/telegram/alarm_test.go b/internal/telegram/alarm_test.go index aafc05a..5e6c214 100644 --- a/internal/telegram/alarm_test.go +++ b/internal/telegram/alarm_test.go @@ -87,14 +87,19 @@ func TestAlarm(t *testing.T) { func TestAlarmRefusals(t *testing.T) { e := newEnv(t) for name, change := range map[string]func(*approval.Alarm){ - "a bad request ID": func(a *approval.Alarm) { a.RequestID = "x" }, - "a bad short code": func(a *approval.Alarm) { a.ShortCode = "" }, - "a bad client": func(a *approval.Alarm) { a.Client = "a b" }, - "a bad secret": func(a *approval.Alarm) { a.Secrets = []string{""} }, - "unknown with a grant": func(a *approval.Alarm) { a.Cause, a.Ended = audit.AlarmUnknownToken, false }, - "no grant": func(a *approval.Alarm) { a.GrantID = "" }, - "an unknown cause": func(a *approval.Alarm) { a.Cause = "x" }, - "ended, but not ending": func(a *approval.Alarm) { a.Cause = audit.AlarmOutsideWindow }, + "a bad request ID": func(a *approval.Alarm) { a.RequestID = "x" }, + "a bad short code": func(a *approval.Alarm) { a.ShortCode = "" }, + "a bad client": func(a *approval.Alarm) { a.Client = "a b" }, + "a bad secret": func(a *approval.Alarm) { a.Secrets = []string{""} }, + "unknown with a grant": func(a *approval.Alarm) { a.Cause, a.Ended = audit.AlarmUnknownToken, false }, + "no grant": func(a *approval.Alarm) { a.GrantID = "" }, + "an unknown cause": func(a *approval.Alarm) { a.Cause = "x" }, + "ended, but not ending": func(a *approval.Alarm) { a.Cause = audit.AlarmOutsideWindow }, + "a bad owner": func(a *approval.Alarm) { a.Owner = "a b" }, + "the owner is the client": func(a *approval.Alarm) { a.Owner = "laptop" }, + "an owner, no grant": func(a *approval.Alarm) { + a.Owner, a.Cause, a.GrantID, a.Ended = "ci", audit.AlarmUnknownToken, "", false + }, } { a := alarmOf(audit.AlarmOldToken) change(&a) @@ -106,3 +111,20 @@ func TestAlarmRefusals(t *testing.T) { t.Errorf("%d calls", n) } } + +// TestAlarmOwner: an alarm for a token of another machine's grant names +// that machine. +func TestAlarmOwner(t *testing.T) { + a := alarmOf(audit.AlarmOldToken) + a.Client, a.Owner = "ci", "laptop" + txt, err := alarmText(a) + if err != nil { + t.Fatal(err) + } + if s := txt.plain.String(); !strings.Contains(s, "machine: ci") || !strings.Contains(s, "machine laptop") { + t.Errorf("text: %q", s) + } + if s := alarmOf(audit.AlarmOldToken); s.Owner != "" { + t.Fatal("alarmOf has an owner") + } +} diff --git a/internal/telegram/text.go b/internal/telegram/text.go index 1087529..3e5cb0a 100644 --- a/internal/telegram/text.go +++ b/internal/telegram/text.go @@ -667,6 +667,8 @@ func alarmText(a approval.Alarm) (*text, error) { return nil, bad("the secrets") case (a.GrantID == "") != (a.Cause == audit.AlarmUnknownToken) || (a.GrantID != "" && !validRequestID(a.GrantID)): return nil, bad("the grant ID") + case a.Owner != "" && (a.GrantID == "" || a.Owner == a.Client || !namePattern.MatchString(a.Owner)): + return nil, bad("the owner") } var what string switch a.Cause { @@ -693,5 +695,8 @@ func alarmText(a approval.Alarm) (*text, error) { if a.GrantID != "" { t.write("\n✅ grant: " + a.GrantID) } + if a.Owner != "" { + t.write("\n⚠️ the grant is of machine " + a.Owner + ": its token left that machine") + } return t, nil }