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