From cee11e7c866addbf2b173f35202a47a9e4cdcfc0 Mon Sep 17 00:00:00 2001 From: adam Date: Sun, 11 Oct 2026 23:53:26 +0100 Subject: [PATCH] CLI: changeset commands default to JARL_CHANGESET, which is checked first jarl changeset show/diff/apply/discard take the changeset from -C or JARL_CHANGESET when no ID is given, and remove takes [ID] SEQ. A changeset from JARL_CHANGESET is checked before anything uses it (staging included): it must exist, be open and be the caller's own, so a stale export can't stage into an applied changeset or into another user's draft that later got its ID. Commands say on stderr which changeset they use, and apply and discard remind to unset JARL_CHANGESET. -C and ID arguments are used as given. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 9 ++++ cmd/jarl/changes.go | 95 ++++++++++++++++++++++++++----------- cmd/jarl/envchangeset.go | 98 +++++++++++++++++++++++++++++++++++++++ cmd/jarl/main.go | 17 ++++--- docs/cli.md | 31 +++++++++++-- internal/client/client.go | 33 +++++++++++-- scripts/e2e.sh | 38 +++++++++++++++ 7 files changed, 278 insertions(+), 43 deletions(-) create mode 100644 cmd/jarl/envchangeset.go diff --git a/CHANGELOG.md b/CHANGELOG.md index fd2412c..05cd2e2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,15 @@ All notable changes to Jarl are documented here. The format follows ### Added +- `jarl changeset show`, `diff`, `apply`, `discard` and `remove` take the + changeset from `-C` or `JARL_CHANGESET` when no ID is given + (`jarl changeset apply`). A changeset from `JARL_CHANGESET` is now + checked before it is used, for staging too: it must exist, be open and + be yours, so a stale export (an applied changeset, or an ID that later + turned into someone else's web UI draft) can't silently take your + changes; the error says what to do. Commands say on stderr which + changeset from `JARL_CHANGESET` they use, and `apply`/`discard` remind + you to `unset JARL_CHANGESET`. `-C` and ID arguments are used as given. - Address ranges such as `10.0.10.101-10.0.10.199` or `2001:db8::100-2001:db8::1ff` (also `!` negated, and with spaces around the dash) wherever an address match list is used: ACLs, views' diff --git a/cmd/jarl/changes.go b/cmd/jarl/changes.go index c6a3e09..52dc8b5 100644 --- a/cmd/jarl/changes.go +++ b/cmd/jarl/changes.go @@ -120,8 +120,12 @@ func byIDOrMatch(op string, d map[string]any) api.ChangeRequest { // changeset given with --changeset. func submitBatch(c *client.Client, description string, reqs []api.ChangeRequest) error { var resp api.MutationResponse - if c.Changeset > 0 { - if err := c.Do(ctx(), "POST", fmt.Sprintf("/changesets/%d/ops", c.Changeset), nil, map[string]any{"ops": reqs}, &resp); err != nil { + cs, err := c.UseChangeset(ctx()) + if err != nil { + return err + } + if cs > 0 { + if err := c.Do(ctx(), "POST", fmt.Sprintf("/changesets/%d/ops", cs), nil, map[string]any{"ops": reqs}, &resp); err != nil { return err } return report(&resp) @@ -230,8 +234,9 @@ func changesetCmd() *cobra.Command { Example: ` export JARL_CHANGESET=$(jarl changeset create "move web servers" -q) jarl record add example.com www2 A 192.0.2.20 jarl record delete example.com www A - jarl changeset diff $JARL_CHANGESET - jarl changeset apply $JARL_CHANGESET`, + jarl changeset diff # JARL_CHANGESET's + jarl changeset apply + unset JARL_CHANGESET`, RunE: func(cmd *cobra.Command, args []string) error { c, err := newClient() if err != nil { @@ -245,21 +250,25 @@ func changesetCmd() *cobra.Command { fmt.Println(cs.ID) return nil } - fmt.Printf("created changeset %d\nstage changes with --changeset %d (or export JARL_CHANGESET=%d)\n", cs.ID, cs.ID, cs.ID) + fmt.Printf("created changeset %d\nstage changes with --changeset %d (or export JARL_CHANGESET=%d), then 'jarl changeset apply'\n", cs.ID, cs.ID, cs.ID) return nil }, } show := &cobra.Command{ - Use: "show ID", - Short: "Show a changeset's operations", - Args: cobra.ExactArgs(1), + Use: "show [ID]", + Short: "Show a changeset's operations (default: -C or JARL_CHANGESET)", + Args: cobra.MaximumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { c, err := newClient() if err != nil { return err } + id, _, err := changesetArg(c, args, false) + if err != nil { + return err + } var cs api.Changeset - if err := c.Get(ctx(), "/changesets/"+args[0], nil, &cs); err != nil { + if err := c.Get(ctx(), "/changesets/"+id, nil, &cs); err != nil { return err } return output(cs, func(w io.Writer) { @@ -280,16 +289,20 @@ func changesetCmd() *cobra.Command { }, } diff := &cobra.Command{ - Use: "diff ID", - Short: "Validate an open changeset and show what applying it would change", - Args: cobra.ExactArgs(1), + Use: "diff [ID]", + Short: "Validate an open changeset and show what applying it would change (default: -C or JARL_CHANGESET)", + Args: cobra.MaximumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { c, err := newClient() if err != nil { return err } + id, fromEnv, err := changesetArg(c, args, true) + if err != nil { + return err + } var raw json.RawMessage - if err := c.Get(ctx(), "/changesets/"+args[0]+"/preview", nil, &raw); err != nil { + if err := c.Get(ctx(), "/changesets/"+id+"/preview", nil, &raw); err != nil { return err } if flagOutput != "table" { @@ -312,56 +325,82 @@ func changesetCmd() *cobra.Command { fmt.Println() fmt.Print(r.Diff) if r.Valid { - fmt.Println("\nvalid: apply with 'jarl changeset apply " + args[0] + "'") + how := "jarl changeset apply " + id + if fromEnv { + how = "jarl changeset apply" + } + fmt.Println("\nvalid: apply with '" + how + "'") return nil } return fmt.Errorf("the changeset can't be applied as it is") }, } apply := &cobra.Command{ - Use: "apply ID", - Short: "Apply an open changeset", - Args: cobra.ExactArgs(1), + Use: "apply [ID]", + Short: "Apply an open changeset (default: -C or JARL_CHANGESET)", + Args: cobra.MaximumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { c, err := newClient() if err != nil { return err } + id, fromEnv, err := changesetArg(c, args, true) + if err != nil { + return err + } var resp api.MutationResponse - if err := c.Do(ctx(), "POST", "/changesets/"+args[0]+"/apply", nil, map[string]any{"force": c.Force, "dry_run": c.DryRun}, &resp); err != nil { + if err := c.Do(ctx(), "POST", "/changesets/"+id+"/apply", nil, map[string]any{"force": c.Force, "dry_run": c.DryRun}, &resp); err != nil { return err } - return report(&resp) + if err := report(&resp); err != nil { + return err + } + if fromEnv && resp.Applied { + envChangesetDone(id, "applied") + } + return nil }, } discard := &cobra.Command{ - Use: "discard ID", - Short: "Throw away an open changeset", - Args: cobra.ExactArgs(1), + Use: "discard [ID]", + Short: "Throw away an open changeset (default: -C or JARL_CHANGESET)", + Args: cobra.MaximumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { c, err := newClient() if err != nil { return err } - if err := c.Do(ctx(), "POST", "/changesets/"+args[0]+"/discard", nil, nil, nil); err != nil { + id, fromEnv, err := changesetArg(c, args, true) + if err != nil { + return err + } + if err := c.Do(ctx(), "POST", "/changesets/"+id+"/discard", nil, nil, nil); err != nil { return err } if !flagQuiet { - fmt.Println("discarded changeset", args[0]) + fmt.Println("discarded changeset", id) + } + if fromEnv { + envChangesetDone(id, "discarded") } return nil }, } remove := &cobra.Command{ - Use: "remove ID SEQ", - Short: "Remove one operation from an open changeset", - Args: cobra.ExactArgs(2), + Use: "remove [ID] SEQ", + Short: "Remove one operation from an open changeset (default: -C or JARL_CHANGESET)", + Args: cobra.RangeArgs(1, 2), RunE: func(cmd *cobra.Command, args []string) error { c, err := newClient() if err != nil { return err } - return c.Do(ctx(), "DELETE", "/changesets/"+args[0]+"/ops/"+args[1], nil, nil, nil) + seq := args[len(args)-1] + id, _, err := changesetArg(c, args[:len(args)-1], true) + if err != nil { + return err + } + return c.Do(ctx(), "DELETE", "/changesets/"+id+"/ops/"+seq, nil, nil, nil) }, } revert := &cobra.Command{ diff --git a/cmd/jarl/envchangeset.go b/cmd/jarl/envchangeset.go new file mode 100644 index 0000000..e3dc61a --- /dev/null +++ b/cmd/jarl/envchangeset.go @@ -0,0 +1,98 @@ +package main + +import ( + "errors" + "fmt" + "os" + "strconv" + + "jarl/internal/api" + "jarl/internal/client" + "jarl/internal/model" +) + +// A changeset given with --changeset is used as given (jarld still refuses +// what isn't allowed). One taken from JARL_CHANGESET is checked first: an +// exported ID outlives the changeset (it gets applied), and changeset IDs +// keep growing, so a stale or mistyped one can name someone else's +// changeset later. It is used only when it is open and the caller's own, +// and the CLI says on stderr which changeset that is. + +// envChangeset returns the changeset in JARL_CHANGESET (0 if unset). +func envChangeset() (int64, error) { + v := os.Getenv("JARL_CHANGESET") + if v == "" { + return 0, nil + } + n, err := strconv.ParseInt(v, 10, 64) + if err != nil || n <= 0 { + return 0, fmt.Errorf("invalid JARL_CHANGESET %q", v) + } + return n, nil +} + +// checkEnvChangeset checks the changeset id from JARL_CHANGESET before it +// is used. With mustOwn, it must also be open and the caller's own (for +// staging into it, applying or discarding it). +func checkEnvChangeset(c *client.Client, id int64, mustOwn bool) (*api.Changeset, error) { + var cs api.Changeset + if err := c.Get(ctx(), fmt.Sprintf("/changesets/%d", id), nil, &cs); err != nil { + var ce *client.Error + if errors.As(err, &ce) && ce.Status == 404 { + return nil, fmt.Errorf("JARL_CHANGESET is %d, but there is no changeset %d: start one with 'export JARL_CHANGESET=$(jarl changeset create -q)', or 'unset JARL_CHANGESET' to make changes live", id, id) + } + return nil, err + } + if mustOwn { + if cs.Status != model.ChangesetOpen { + return nil, fmt.Errorf("JARL_CHANGESET is %d, but changeset %d is %s: 'unset JARL_CHANGESET' to make changes live, or start a new one with 'export JARL_CHANGESET=$(jarl changeset create -q)'", id, id, cs.Status) + } + var me api.Whoami + if err := c.Get(ctx(), "/auth/whoami", nil, &me); err != nil { + return nil, err + } + mine := cs.UserID == me.User.ID && (me.User.ID != 0 || cs.Username == me.User.Username) + if !mine { + return nil, fmt.Errorf("JARL_CHANGESET is %d, but changeset %d is %s's (made via %s), not yours, so it isn't used from JARL_CHANGESET; give it as -C %d to use it anyway, or 'unset JARL_CHANGESET'", id, id, cs.Username, cs.Via, id) + } + } + if !flagQuiet { + desc := cs.Description + if desc == "" { + desc = "no description" + } + fmt.Fprintf(os.Stderr, "changeset %d (%s, %d operations) from JARL_CHANGESET\n", id, desc, len(cs.Ops)) + } + return &cs, nil +} + +// changesetArg returns the changeset a changeset subcommand works on: the +// ID argument, else --changeset, else JARL_CHANGESET (checked; mustOwn as +// for checkEnvChangeset). fromEnv says it came from JARL_CHANGESET. +func changesetArg(c *client.Client, args []string, mustOwn bool) (id string, fromEnv bool, err error) { + if len(args) > 0 { + return args[0], false, nil + } + if flagChangeset > 0 { + return strconv.FormatInt(flagChangeset, 10), false, nil + } + n, err := envChangeset() + if err != nil { + return "", false, err + } + if n == 0 { + return "", false, errors.New("which changeset? Give its ID, or set JARL_CHANGESET (export JARL_CHANGESET=$(jarl changeset create -q))") + } + if _, err := checkEnvChangeset(c, n, mustOwn); err != nil { + return "", false, err + } + return strconv.FormatInt(n, 10), true, nil +} + +// envChangesetDone reminds that JARL_CHANGESET still names a changeset +// that was just applied or discarded: later changes would fail to stage. +func envChangesetDone(id, what string) { + if !flagQuiet { + fmt.Fprintf(os.Stderr, "JARL_CHANGESET still names changeset %s, which is now %s: run 'unset JARL_CHANGESET' (or export a new one) before making more changes\n", id, what) + } +} diff --git a/cmd/jarl/main.go b/cmd/jarl/main.go index 05e8c16..ce9de81 100644 --- a/cmd/jarl/main.go +++ b/cmd/jarl/main.go @@ -13,7 +13,6 @@ import ( "os" "path/filepath" "slices" - "strconv" "strings" "sync" "text/tabwriter" @@ -149,12 +148,18 @@ func newClient() (*client.Client, error) { } c.Changeset = flagChangeset if c.Changeset == 0 { - if v := os.Getenv("JARL_CHANGESET"); v != "" { - n, err := strconv.ParseInt(v, 10, 64) - if err != nil { - return nil, fmt.Errorf("invalid JARL_CHANGESET %q", v) - } + n, err := envChangeset() + if err != nil { + return nil, err + } + if n > 0 { + // Checked when a change is first staged into it + // (envchangeset.go). c.Changeset = n + c.CheckChangeset = func(context.Context) error { + _, err := checkEnvChangeset(c, n, true) + return err + } } } c.DryRun, c.Force = flagDryRun || c.Preflight, flagForce diff --git a/docs/cli.md b/docs/cli.md index 43c4de0..c5d025f 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -41,7 +41,7 @@ jarl whoami | API token (`@FILE` reads it from a file) | `--token` | `JARL_TOKEN` | `token` | | Skip TLS verification | `--insecure` | | `insecure` | | Extra trusted CA (PEM) | `--ca-cert` | | `ca_cert` | -| Changeset to stage into | `-C/--changeset` | `JARL_CHANGESET` | | +| Changeset to stage into (and the default for `jarl changeset diff/apply/...`) | `-C/--changeset` | `JARL_CHANGESET` (checked: open and yours, see [Changesets](#changesets-stage-review-apply)) | | Flags win over the environment, and the environment wins over the file. The file is `jarl/config.yaml` in the user's configuration directory, or @@ -345,9 +345,9 @@ reads standard input. export JARL_CHANGESET=$(jarl changeset create "renumber lab" -q) jarl record set lab.adbraun.lol gw A 10.20.0.1 jarl record set lab.adbraun.lol nas A 10.20.0.10 -jarl changeset diff $JARL_CHANGESET # validation + named-checkconf + diff -jarl changeset remove $JARL_CHANGESET 2 # drop operation #2 -jarl changeset apply $JARL_CHANGESET +jarl changeset diff # validation + named-checkconf + diff +jarl changeset remove 2 # drop operation #2 +jarl changeset apply unset JARL_CHANGESET jarl changeset list [--status open|applied|discarded] [--mine] @@ -357,6 +357,29 @@ jarl history [--limit 50] jarl changeset revert 42 -q | xargs jarl changeset apply # undo an applied changeset ``` +`show`, `diff`, `apply`, `discard` and `remove` work on the changeset you +name, else the one in `-C`, else the one in `JARL_CHANGESET`. + +A changeset from `JARL_CHANGESET` is checked before anything uses it, +because an exported ID easily outlives its changeset, and IDs keep growing +(every change, live ones included, gets the next one), so a stale or +mistyped ID can later name a changeset that isn't yours: + +- Staging into it, and `diff`, `apply`, `discard` and `remove` on it, need + it to be open and yours. Otherwise the command stops and says why + (`there is no changeset 99`, `changeset 42 is applied`, `changeset 57 is + certbot's`) and what to do. Commands that only read (`jarl zone list`) + and `jarl changeset create` don't care, so you can always start a new one. +- Each command says on stderr which changeset it uses: `changeset 42 (renumber + lab, 2 operations) from JARL_CHANGESET` (not with `-q`). +- After `apply` or `discard`, a reminder to `unset JARL_CHANGESET`: the CLI + can't change your shell's environment, and later changes would fail to + stage. + +`-C ID` and an ID argument are used as given, without these checks (jarld +still refuses a changeset that isn't open, or another user's unless you are +an administrator). + Changesets staged in the web UI show up here too, and the other way round. ## Views, ACLs, TSIG keys, options, settings diff --git a/internal/client/client.go b/internal/client/client.go index b226b12..dd34341 100644 --- a/internal/client/client.go +++ b/internal/client/client.go @@ -30,8 +30,14 @@ type Client struct { // Changeset, if set, stages changes into this changeset instead of // applying them. Changeset int64 - DryRun bool - Force bool + // CheckChangeset, if set, runs once before Changeset is first used, + // and its error stops the request. The CLI checks a changeset taken + // from JARL_CHANGESET with it. + CheckChangeset func(ctx context.Context) error + csChecked bool + csErr error + DryRun bool + Force bool // Preflight lets only requests through that can't change anything: // reads and dry runs. Running a command on several servers uses it to // check the command everywhere before running it anywhere. @@ -243,6 +249,19 @@ func (c *Client) Get(ctx context.Context, path string, query url.Values, out any return c.Do(ctx, http.MethodGet, path, query, nil, out) } +// UseChangeset returns the changeset to stage into (0 for none), once +// CheckChangeset has accepted it. +func (c *Client) UseChangeset(ctx context.Context) (int64, error) { + if c.Changeset == 0 { + return 0, nil + } + if c.CheckChangeset != nil && !c.csChecked { + c.csChecked = true + c.csErr = c.CheckChangeset(ctx) + } + return c.Changeset, c.csErr +} + // Mutate sends a configuration change, honoring Changeset, DryRun and // Force. func (c *Client) Mutate(ctx context.Context, method, path string, query url.Values, body any) (*api.MutationResponse, error) { @@ -250,8 +269,12 @@ func (c *Client) Mutate(ctx context.Context, method, path string, query url.Valu for k, v := range query { q[k] = v } - if c.Changeset > 0 { - q.Set("changeset", fmt.Sprint(c.Changeset)) + cs, err := c.UseChangeset(ctx) + if err != nil { + return &api.MutationResponse{}, err + } + if cs > 0 { + q.Set("changeset", fmt.Sprint(cs)) } if c.DryRun { q.Set("dry_run", "1") @@ -260,7 +283,7 @@ func (c *Client) Mutate(ctx context.Context, method, path string, query url.Valu q.Set("force", "1") } var out api.MutationResponse - err := c.Do(ctx, method, path, q, body, &out) + err = c.Do(ctx, method, path, q, body, &out) return &out, err } diff --git a/scripts/e2e.sh b/scripts/e2e.sh index 2cc936b..85730b1 100755 --- a/scripts/e2e.sh +++ b/scripts/e2e.sh @@ -227,6 +227,37 @@ run "changeset apply" 0 $J changeset apply $CS expect_dig "staged now live" "192.168.1.80" staged.adbraun.test A REV=$($J changeset revert $CS -q) run "apply revert changeset" 0 $J changeset apply $REV +# JARL_CHANGESET: staging and the changeset commands default to it, and +# it is checked before it is used. +export JARL_CHANGESET=$($J changeset create "e2e env" -q) +ENVCS=$JARL_CHANGESET +run "stage via JARL_CHANGESET" 0 $J record add adbraun.test envstaged A 192.168.1.81 +contains "changeset $ENVCS (e2e env, 0 operations) from JARL_CHANGESET" +expect_dig "env-staged not live yet" "" envstaged.adbraun.test A +run "changeset show defaults to it" 0 $J changeset show +contains "envstaged" +run "changeset diff defaults to it" 0 $J changeset diff +contains "apply with 'jarl changeset apply'" +OTHER=$($J changeset create "e2e other" -q) +run "-C wins over JARL_CHANGESET" 0 $J changeset show -C $OTHER +contains "e2e other" +run "an ID argument wins too" 0 $J changeset discard $OTHER +run "changeset apply defaults to it" 0 $J changeset apply +contains "unset JARL_CHANGESET" +expect_dig "env-staged live" "192.168.1.81" envstaged.adbraun.test A +run "staging into the applied changeset is refused" 1 $J record add adbraun.test envlate A 192.168.1.82 +contains "changeset $ENVCS is applied" +run "applying it again is refused" 1 $J changeset apply +contains "is applied" +export JARL_CHANGESET=999999 +run "a changeset that doesn't exist is explained" 1 $J record add adbraun.test envlate A 192.168.1.82 +contains "there is no changeset 999999" +run "reads don't care about a stale JARL_CHANGESET" 0 $J zone list +unset JARL_CHANGESET +expect_dig "nothing was staged or applied" "" envlate.adbraun.test A +run "changeset apply without an ID or JARL_CHANGESET" 1 $J changeset apply +contains "which changeset?" +run "clean up envstaged" 0 $J record delete adbraun.test envstaged A --yes expect_dig "staged reverted" "" staged.adbraun.test A run "history" 0 $J history contains "e2e batch" @@ -285,6 +316,13 @@ run "acme set again (idempotent)" 0 $J $ACME acme set adbraun.test value-one run "acme set wildcard second value" 0 $J $ACME acme set '*.adbraun.test' value-two expect_dig "acme TXT both" '"value-one" "value-two"' _acme-challenge.adbraun.test TXT run "acme token cannot touch A records (exit 3)" 3 $J $ACME record add adbraun.test evil A 192.168.1.66 +# Someone else's changeset in JARL_CHANGESET isn't used, even by an admin. +THEIRS=$($J $ACME changeset create "certbot's" -q) +run "another user's changeset from JARL_CHANGESET is refused" 1 env JARL_CHANGESET=$THEIRS $J record add adbraun.test notmine A 192.168.1.83 +contains "is certbot's" +run "applying it from JARL_CHANGESET is refused" 1 env JARL_CHANGESET=$THEIRS $J changeset apply +run "-C uses it anyway (admin)" 0 $J changeset show -C $THEIRS +run "discard certbot's changeset" 0 $J changeset discard $THEIRS run "certbot hooks" 0 env CERTBOT_DOMAIN=nas.adbraun.test CERTBOT_VALIDATION=cb-value $J $ACME acme certbot-auth expect_dig "certbot TXT" '"cb-value"' _acme-challenge.nas.adbraun.test TXT run "certbot cleanup" 0 env CERTBOT_DOMAIN=nas.adbraun.test CERTBOT_VALIDATION=cb-value $J $ACME acme certbot-cleanup