From 9a165cf70be334bd20d9ecd15e5cc26efb9bd562 Mon Sep 17 00:00:00 2001 From: abrown38 Date: Mon, 19 Oct 2026 14:52:55 +0100 Subject: [PATCH] Staged undo, built-ins window, machine warnings; raw and dynamic set fixes - Undo is staged like any edit: in review mode the web UI's Undo, and hc history undo with -C or HOUSECARL_CHANGESET, add a revert operation that checks, when replayed, that nothing it touches changed since; applied, History links the entries. Imports and takeovers still can't be staged. - The rules page sums up a chain's built-ins in one line; a window lists them with what they do and their hits, and switches them. - Warnings about the machine (dashboard, hc status): nftables.service enabled (it loads /etc/nftables.conf at boot and flushes every table when stopped), and the imported table still in force beside Housecarl's. The docs say to disable nftables.service but not stop it. - A raw rule with other fields set is refused (raw_conflict) instead of silently ignoring them; the rule dialog's nftables section hides what it replaces, and the importer's raw rules carry only their text. - A dynamic set with configured entries is refused (set_dynamic_entries): they were silently ignored. The set dialog shows entries or the entry timeout, as fits. - The rule dialog's chain choices are just the chains' names. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 9 +- cmd/hc/rules.go | 9 +- docs/api.md | 5 +- docs/cli.md | 6 +- docs/design.md | 5 +- docs/import.md | 10 +- docs/operations.md | 19 +++- docs/rules.md | 12 ++- docs/validation.md | 12 +++ internal/client/client.go | 6 +- internal/engine/engine.go | 29 ++++-- internal/importer/importer.go | 7 +- internal/model/changeset.go | 5 +- internal/server/h_changesets.go | 21 +++- internal/server/h_config.go | 20 +++- internal/server/h_import.go | 5 + internal/server/h_status.go | 36 +++++++ internal/server/ops.go | 6 ++ internal/server/server.go | 5 +- internal/server/server_test.go | 70 ++++++++++++++ internal/validate/validate.go | 35 +++++++ internal/validate/validate_test.go | 2 + internal/web/static/assets/pages/history.js | 8 +- internal/web/static/assets/pages/rules.js | 101 +++++++++++++++++--- internal/web/static/assets/pages/sets.js | 16 +++- scripts/e2e.sh | 7 +- 26 files changed, 408 insertions(+), 58 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f3033f6..931a8cf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -35,7 +35,8 @@ the way Jarl and Steward are. review shows the problems, nftables' check and the diff of the ruleset. The web UI collects edits as pending changes by default (pending rows, a bar with Review and Apply now, a Changes page), with a switch for live - edits; hc stages with `-C ID` or `HOUSECARL_CHANGESET`, and has `hc + edits, and Undo in History is staged like any edit; hc stages with + `-C ID` or `HOUSECARL_CHANGESET`, and has `hc changeset create/list/show/diff/apply/discard/remove`. An applied changeset is on probation like any change. `--dry-run` shows the ruleset's diff too (docs/design.md, docs/cli.md). @@ -66,6 +67,9 @@ the way Jarl and Steward are. - **At boot:** the confirmed ruleset is loaded by `housecarl-ruleset.service` before the network comes up, without housecarld. An empty configuration applies nothing. +- **Warnings about the machine:** while `nftables.service` is enabled + (it loads `/etc/nftables.conf` at boot, and flushes every table when + stopped), and while the table an import came from still applies. - **Hits and the log:** packet and byte counters for every rule, NAT rule, built-in and policy; packets logged by rules and policies, read from netlink log group 4171 (`hc log -f`, the Log page). @@ -81,7 +85,8 @@ the way Jarl and Steward are. chain with hits and reordering, NAT, zones, sets, services, the log, settings, import, history with undo; the rule dialog keeps the less used parts, such as rate limits, quotas and time windows, behind "+" - buttons), with users, roles, scoped API + buttons; each chain's built-ins are listed, with their hits, and + switched in a window), with users, roles, scoped API tokens, sessions, the audit log, and the web UI's certificate from Let's Encrypt through Jarl, as in Steward. - **Packaging:** Debian packages `housecarl` and `housecarl-cli`, a diff --git a/cmd/hc/rules.go b/cmd/hc/rules.go index 0d8f455..2f3ce17 100644 --- a/cmd/hc/rules.go +++ b/cmd/hc/rules.go @@ -285,6 +285,9 @@ func ruleCmd() *cobra.Command { } extra := ruleExtras(r) action := r.Action + if r.Raw != "" { + action = "-" // the text's own + } if r.Log { action += "+log" } @@ -298,7 +301,11 @@ func ruleCmd() *cobra.Command { } cmd.AddCommand(ls) cmd.AddCommand(showCmd("rules", "rule", func(w io.Writer, r model.Rule) { - fmt.Fprintf(w, "ID:\t%d\nChain:\t%s\nMatch:\t%s%s\nAction:\t%s\n", r.ID, r.Chain, describe(r.Match), ruleExtras(r), r.Action) + if r.Raw == "" { + fmt.Fprintf(w, "ID:\t%d\nChain:\t%s\nMatch:\t%s%s\nAction:\t%s\n", r.ID, r.Chain, describe(r.Match), ruleExtras(r), r.Action) + } else { + fmt.Fprintf(w, "ID:\t%d\nChain:\t%s\n", r.ID, r.Chain) + } if r.Raw != "" { fmt.Fprintf(w, "Raw:\t%s\n", r.Raw) } diff --git a/docs/api.md b/docs/api.md index 97d8881..fd1534d 100644 --- a/docs/api.md +++ b/docs/api.md @@ -157,7 +157,10 @@ Changes staged to be reviewed and applied together ([design.md](design.md#review Staging (`?changeset=ID` on any change to objects or settings) checks the changeset with the new operation, and nftables checks its ruleset; an error refuses the operation. Only open changesets take operations, and -only their owner's (or an administrator's). Undo, `/import/nft` and +only their owner's (or an administrator's). An undo +(`POST /history/{id}/revert?changeset=ID`) is staged as one operation, +which checks when it is replayed that nothing it touches changed since; +applied, History links it to the entry it undoes. `/import/nft` and `/takeover` can't be staged: with a changeset named they answer 400. Reads of objects, settings and `/ruleset` with `?changeset=ID` show the diff --git a/docs/cli.md b/docs/cli.md index ec1c982..ea1d6ae 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -117,9 +117,9 @@ it, because an exported ID easily outlives its changeset: `-C ID` and an ID argument are used as given (housecarld still refuses a changeset that isn't open, or another user's unless you are an administrator). Only configuration changes are staged: dynamic set -entries, `confirm` and `rollback` are made at once, and `history undo`, -`import` and `takeover` can't be staged, so they are refused while a -changeset is named. Changesets staged in the web UI show up here too, +entries, `confirm` and `rollback` are made at once; `history undo` is +staged like any change; `import` and `takeover` can't be staged, so they +are refused while a changeset is named. Changesets staged in the web UI show up here too, and the other way round. ## Commands diff --git a/docs/design.md b/docs/design.md index 79ba092..1f2b9d4 100644 --- a/docs/design.md +++ b/docs/design.md @@ -75,8 +75,9 @@ to be reviewed and applied together with others: | CLI | Immediate: each command is a change of its own. Stage with `-C ID` or `HOUSECARL_CHANGESET`. | Scripts want a clear result per step. | | API | Immediate unless a changeset is named (`?changeset=ID`, `X-Housecarl-Changeset`). | As the CLI. | -Undo, imports and takeovers can't be staged; with a changeset named, they -are refused rather than applied. +An undo is staged like any edit (in review mode, the web UI's Undo +collects it for review). Imports and takeovers can't be staged; with a +changeset named, they are refused rather than applied. ## Its own table only diff --git a/docs/import.md b/docs/import.md index 9d19c83..fdba5f7 100644 --- a/docs/import.md +++ b/docs/import.md @@ -107,7 +107,10 @@ the old table: sudo hc takeover "inet main" ``` -From now on only Housecarl's rules apply. Over the network, this is on +From now on only Housecarl's rules apply. Until you do, the dashboard +and `hc status` warn that the imported table still applies +(`takeover_pending`); they also warn while `nftables.service` is enabled +(`nftables_service`). Over the network, this is on probation too: unless confirmed, the old table comes back. ## 4. The boot @@ -122,6 +125,11 @@ sudo systemctl disable nftables.service sudo mv /etc/nftables.conf /etc/nftables.conf.before-housecarl ``` +Disable it, but **don't stop it** (no `systemctl stop nftables`, no +`disable --now`): Debian's unit stops by running `nft flush ruleset`, +which deletes every table, Housecarl's and Docker's included, at once. +Disabled, it simply doesn't run at the next boot. + `housecarl-ruleset.service` runs after `nftables.service` in any case, so a leftover `flush ruleset` can't undo it. Docker, Incus and libvirt keep adding their own tables as before. diff --git a/docs/operations.md b/docs/operations.md index 2530c93..6068404 100644 --- a/docs/operations.md +++ b/docs/operations.md @@ -80,10 +80,21 @@ sudo nft -c -f /var/lib/housecarl/ruleset.nft # check it Dynamic sets start empty after a reboot. -Keep Debian's `nftables.service` disabled, or `/etc/nftables.conf` -without `flush ruleset`: otherwise it deletes Housecarl's table at -boot. `housecarl-ruleset.service` runs after it, which covers the -ordinary case. +Keep Debian's `nftables.service` disabled (`systemctl disable +nftables`, without `--now`): at boot it loads `/etc/nftables.conf`, and +stopping or restarting it runs `nft flush ruleset`, which deletes every +table, Housecarl's included. `housecarl-ruleset.service` runs after it +at boot, which covers the ordinary case, but not a restart of +`nftables.service` later. Housecarl never reads or writes +`/etc/nftables.conf`; its ruleset is saved in +`/var/lib/housecarl/ruleset.nft`. The dashboard and `hc status` warn +while `nftables.service` is enabled. + +Other programs' tables are left alone: fail2ban's (`inet f2b-table`), +Docker's, Incus's. Housecarl replaces only its own table, so they keep +their rules through every Housecarl change and restart, and nothing +needs restarting in a particular order. A packet must get past every +table on its hook, so a drop in any of them wins. ## Hits diff --git a/docs/rules.md b/docs/rules.md index 97b66d2..4ded18a 100644 --- a/docs/rules.md +++ b/docs/rules.md @@ -38,7 +38,10 @@ directly (`--from 192.168.1.3`); a set is better when several rules share them. A **dynamic** set's entries are added at run time, each with a timeout, -and nftables removes them when it runs out, even with housecarld stopped: +and nftables removes them when it runs out, even with housecarld stopped. +It has no configured entries (validation refuses them); its timeout is +the default for entries added without one, and without a timeout they +stay until removed: ```sh hc set add guests --dynamic --timeout 24h @@ -193,8 +196,11 @@ hand, placed like any other: hc rule add forward --raw 'iifname != "wan" ct state invalid ip daddr @trusted_v4 accept' ``` -It is checked by nftables with the rest of the ruleset, and can use the -sets' nftables names (`@trusted_v4`). The importer makes raw rules from +It is used exactly as written: its text replaces the rule's zones, +addresses, services, action, limits and log, so a raw rule with any of +those set is refused (`raw_conflict`); write them in the text. It is +checked by nftables with the rest of the ruleset, and can use the sets' +nftables names (`@trusted_v4`). The importer makes raw rules from what it can't translate. It goes into the ruleset as written: add `counter` to it to count its packets. diff --git a/docs/validation.md b/docs/validation.md index bd75dd5..47ffb5b 100644 --- a/docs/validation.md +++ b/docs/validation.md @@ -51,6 +51,7 @@ are unique per kind, ignoring case. |---|---|---| | `set_entry` | error | an entry that isn't an address, prefix (without host bits) or range | | `set_empty` | warning | a set (not dynamic) with no entries: rules using it match nothing | +| `set_dynamic_entries` | error | configured entries on a dynamic set: its entries come at run time | | `set_timeout` | error | a timeout on a set that isn't dynamic, or under a second | | `service_empty` | error | a service with no entries | | `service_entry` | error | an entry that isn't `PROTO/PORTS` (`tcp`, `udp`, `sctp`; a port or range) or `icmp/TYPE`, `icmpv6/TYPE` with a type nftables knows | @@ -77,6 +78,7 @@ These apply to rules, source NAT and redirects. | `rule_chain` | error | a chain that isn't `input`, `forward`, `output` or `early` | | `rule_action` | error | an action that isn't `accept`, `drop`, `reject` or `log` | | `raw_rule` | error | a raw rule over several lines, or with `;` | +| `raw_conflict` | error | a raw rule with other fields set (zones, addresses, services, limits, log…): it is used as written, so they would be ignored | | `early_conntrack` | error | connection states, limits or a connection limit in the early chain, which runs before connection tracking | | `early_reject` | error | reject in the early chain: drop instead | | `ct_state`, `ct_status` | error | an unknown connection state or status | @@ -115,3 +117,13 @@ These apply to rules, source NAT and redirects. | `snat_everything` | warning | no out zones and no sources: everything leaving the firewall is translated | | `redirect_service` | error | no service, or one without TCP, UDP or SCTP ports | | `redirect_to_port` | error | the local port isn't one port | + +## The machine + +These come from housecarld looking at the machine, not at the +configuration: the dashboard and `hc status` show them. + +| Code | | When | +|---|---|---| +| `nftables_service` | warning, *host* | Debian's `nftables.service` is enabled: at boot it loads `/etc/nftables.conf`, and stopping or restarting it runs `nft flush ruleset`, deleting Housecarl's table too. Disable it with `systemctl disable nftables` (without `--now`) | +| `takeover_pending` | warning, *host* | the table you imported from is still in force beside Housecarl's, so a packet must get past both; `hc takeover TABLE` deletes it once Housecarl's rules are right | diff --git a/internal/client/client.go b/internal/client/client.go index 0b0d47c..2d15431 100644 --- a/internal/client/client.go +++ b/internal/client/client.go @@ -39,9 +39,9 @@ type Client struct { } // Staged reports whether a request is a configuration change that a -// changeset takes: objects and settings, and the changes that can't be -// staged (undo, import, takeover), which the server then refuses rather -// than apply them. Changesets' own calls and dynamic set entries aren't. +// changeset takes: objects, settings and undo, and the changes that can't +// be staged (import, takeover), which the server then refuses rather than +// apply them. Changesets' own calls and dynamic set entries aren't. func Staged(method, path string) bool { if method == http.MethodGet || strings.Contains(path, "/elements") { return false diff --git a/internal/engine/engine.go b/internal/engine/engine.go index cbc8018..54707cf 100644 --- a/internal/engine/engine.go +++ b/internal/engine/engine.go @@ -137,6 +137,9 @@ type Edit func(st *model.State, newID func() int64) error type Options struct { DryRun bool Via string // web, cli, api, import + // RevertOf is the history entry the change undoes (a changeset with a + // staged undo), so History links the two. + RevertOf int64 } // Apply makes a change: edit runs on a copy of the configuration, the @@ -161,7 +164,7 @@ func (e *Engine) Apply(ctx context.Context, p *auth.Principal, description strin if idErr != nil { return nil, idErr } - return e.commit(ctx, p, description, opts, next, 0) + return e.commit(ctx, p, description, opts, next, opts.RevertOf) } // commit validates next and saves it as the new state. @@ -264,8 +267,22 @@ func (e *Engine) Revert(ctx context.Context, p *auth.Principal, id int64, opts O } e.mu.Lock() defer e.mu.Unlock() - cur := objects(e.state) next := e.state.Clone() + if err := Undo(next, h); err != nil { + return nil, err + } + return e.commit(ctx, p, UndoDescription(h), opts, next, h.ID) +} + +// UndoDescription is an undo's description: "undo 12: create rule …". +func UndoDescription(h *model.HistoryEntry) string { + return fmt.Sprintf("undo %d: %s", h.ID, h.Description) +} + +// Undo applies a history entry backwards to st, if every object it +// changed is still as it left it (a change made since would be lost). +func Undo(st *model.State, h *model.HistoryEntry) error { + cur := objects(st) for i := len(h.Changes) - 1; i >= 0; i-- { c := h.Changes[i] now := cur[key{c.Kind, c.ID}] @@ -274,13 +291,13 @@ func (e *Engine) Revert(ctx context.Context, p *auth.Principal, id int64, opts O if what == "" { what = fmt.Sprintf("%s %d", c.Kind, c.ID) } - return nil, Errorf(KindConflict, "%s %s has changed since entry %d; undo the later changes first, or change it by hand", c.Kind, what, id) + return Errorf(KindConflict, "%s %s has changed since entry %d; undo the later changes first, or change it by hand", c.Kind, what, h.ID) } - if err := put(next, c.Kind, c.ID, c.Before); err != nil { - return nil, err + if err := put(st, c.Kind, c.ID, c.Before); err != nil { + return err } } - return e.commit(ctx, p, fmt.Sprintf("undo %d: %s", h.ID, h.Description), opts, next, h.ID) + return nil } // Replace makes the configuration next as a whole (imports). diff --git a/internal/importer/importer.go b/internal/importer/importer.go index b968f32..c6a9ad7 100644 --- a/internal/importer/importer.go +++ b/internal/importer/importer.go @@ -39,6 +39,8 @@ type Input struct { // Result is the imported configuration and what to know about it. type Result struct { State *model.State + // Table is the table imported ("inet main"). + Table string Notes []string // Raw lists the rules that became raw rules. Raw []string @@ -206,7 +208,7 @@ func Import(in Input) (*Result, error) { im.run() im.nameForwards() im.st.Renumber() - return &Result{State: im.st, Notes: im.notes, Raw: im.raw}, nil + return &Result{State: im.st, Table: im.table.Family + " " + im.table.Name, Notes: im.notes, Raw: im.raw}, nil } func tableList(ts []nftTable) string { @@ -1464,7 +1466,8 @@ func (im *importer) rawRule(r *model.Rule, p *parsed) { if r.Action == "" { r.Action = model.ActionAccept } - r.Match, r.Raw = model.Match{}, text + // Only the text counts: what was parsed of it goes. + *r = model.Rule{ID: r.ID, Chain: r.Chain, Position: r.Position, Comment: r.Comment, Action: r.Action, Raw: text} im.st.Rules = append(im.st.Rules, r) why := strings.Join(p.unsupported, ", ") if why == "" { diff --git a/internal/model/changeset.go b/internal/model/changeset.go index 7974d54..7632734 100644 --- a/internal/model/changeset.go +++ b/internal/model/changeset.go @@ -35,13 +35,16 @@ const ( OpDelete = "delete" OpMove = "move" OpSettings = "settings" + // OpRevert undoes a history entry: Data is the entry (its changes), + // ID its number. + OpRevert = "revert" ) // Op is one change, as the API received it, so it can be stored and // replayed on whatever the configuration is when it is applied. type Op struct { Seq int `json:"seq"` - Op string `json:"op"` // create, update, delete, move, settings + Op string `json:"op"` // create, update, delete, move, settings, revert Kind string `json:"kind,omitempty"` // Ref names the object (ID, or name) for update, delete and move. Ref string `json:"ref,omitempty"` diff --git a/internal/server/h_changesets.go b/internal/server/h_changesets.go index 7c12c39..3bbb0d5 100644 --- a/internal/server/h_changesets.go +++ b/internal/server/h_changesets.go @@ -36,8 +36,8 @@ func changesetParam(r *http.Request) (int64, error) { return id, nil } -// notStaged refuses a changeset on a change that can't be staged (undo, -// import, takeover): it would otherwise be applied at once, unexpectedly. +// notStaged refuses a changeset on a change that can't be staged (an +// import, a takeover): it would otherwise be applied at once, unexpectedly. func notStaged(r *http.Request, what string) error { if id, err := changesetParam(r); err != nil || id == 0 { return err @@ -102,6 +102,11 @@ func (s *Server) stage(w http.ResponseWriter, r *http.Request, p *auth.Principal return nil } op.Summary = engine.Describe(changes) + if op.Op == model.OpRevert { + var h model.HistoryEntry + json.Unmarshal(op.Data, &h) + op.Summary = engine.UndoDescription(&h) + } op.AddedAt = s.nowFn() for _, o := range cs.Ops { if o.Seq >= op.Seq { @@ -353,7 +358,17 @@ func (s *Server) applyChangeset(w http.ResponseWriter, r *http.Request, p *auth. if len(cs.Ops) == 0 { return badRequest("changeset %d has no changes to apply", id) } - res, err := s.eng.Apply(r.Context(), p, cs.Description, engine.Options{Via: via(r, p)}, func(st *model.State, _ func() int64) error { + opts := engine.Options{Via: via(r, p)} + desc := cs.Description + for _, o := range cs.Ops { + if o.Op == model.OpRevert { + opts.RevertOf = o.ID // History links the undo + if desc == "" && len(cs.Ops) == 1 { + desc = o.Summary + } + } + } + res, err := s.eng.Apply(r.Context(), p, desc, opts, func(st *model.State, _ func() int64) error { return replay(st, cs.Ops) }) if err != nil { diff --git a/internal/server/h_config.go b/internal/server/h_config.go index 5462e97..fb9d377 100644 --- a/internal/server/h_config.go +++ b/internal/server/h_config.go @@ -469,13 +469,27 @@ func (s *Server) revertHistory(w http.ResponseWriter, r *http.Request, p *auth.P if err := require(p, auth.ResConfig, true); err != nil { return err } - if err := notStaged(r, "an undo"); err != nil { - return err - } id, err := pathID(r, "id") if err != nil { return err } + // Staged, an undo is an operation of its own, reviewed like any. + if csID, err := changesetParam(r); err != nil { + return err + } else if csID != 0 { + h, err := s.store.GetHistory(r.Context(), id) + if errors.Is(err, store.ErrNotFound) { + return notFound("history entry %d not found", id) + } + if err != nil { + return err + } + if h.RevertedBy != 0 { + return conflict("history entry %d was already undone by entry %d", id, h.RevertedBy) + } + data, _ := json.Marshal(h) + return s.stage(w, r, p, csID, model.Op{Op: model.OpRevert, ID: id, Data: data}) + } res, err := s.eng.Revert(r.Context(), p, id, engine.Options{DryRun: queryBool(r, "dry_run"), Via: via(r, p)}) if err != nil { s.writeErr(w, r, err, res) diff --git a/internal/server/h_import.go b/internal/server/h_import.go index 30a0ba6..620d10b 100644 --- a/internal/server/h_import.go +++ b/internal/server/h_import.go @@ -51,6 +51,11 @@ func (s *Server) importNft(w http.ResponseWriter, r *http.Request, p *auth.Princ return nil } s.afterChange(r.Context(), p, er) + if er.Applied { + // Until it is taken over, the old table still applies too: the + // status warns (takeover_pending). + s.store.SetMeta(r.Context(), importedTableKey, res.Table) + } out.MutationResponse = api.MutationResponse{Applied: er.Applied, DryRun: er.DryRun, HistoryID: er.HistoryID, Changes: er.Changes, Warnings: er.Warnings} if out.Changes == nil { out.Changes = []model.Change{} diff --git a/internal/server/h_status.go b/internal/server/h_status.go index d93330c..6294332 100644 --- a/internal/server/h_status.go +++ b/internal/server/h_status.go @@ -1,7 +1,10 @@ package server import ( + "context" + "fmt" "net/http" + "os" "sort" "strings" "sync" @@ -44,10 +47,43 @@ func (s *Server) status(w http.ResponseWriter, r *http.Request, p *auth.Principa if data, err := s.fw.ListRuleset(r.Context()); err == nil { out.OtherTables = otherTables(data) } + out.Problems = append(out.Problems, s.hostProblems(r.Context(), out.OtherTables)...) writeJSON(w, http.StatusOK, out) return nil } +const importedTableKey = "imported_table" + +// nftablesUnitLinks are where an enabled nftables.service shows (Debian +// enables it for sysinit.target). +var nftablesUnitLinks = []string{ + "/etc/systemd/system/sysinit.target.wants/nftables.service", + "/etc/systemd/system/multi-user.target.wants/nftables.service", +} + +// hostProblems are warnings about the machine rather than the +// configuration: another ruleset that still loads at boot, or the table +// an import came from, still in force beside Housecarl's. +func (s *Server) hostProblems(ctx context.Context, others []string) []validate.Problem { + var out []validate.Problem + for _, link := range s.unitLinks { + if _, err := os.Lstat(link); err == nil { + out = append(out, validate.Problem{Severity: validate.Warning, Code: "nftables_service", Object: "nftables.service", + Message: "nftables.service is enabled: at boot it loads /etc/nftables.conf, and stopping or restarting it flushes every table, Housecarl's included; disable it (systemctl disable nftables, without --now)"}) + break + } + } + if t, err := s.store.GetMeta(ctx, importedTableKey); err == nil && t != "" { + for _, o := range others { + if o == t { + out = append(out, validate.Problem{Severity: validate.Warning, Code: "takeover_pending", Object: "table " + t, + Message: fmt.Sprintf("%s, the table you imported, still applies beside Housecarl's: a packet must get past both; once Housecarl's rules are right, delete it with 'hc takeover %q'", t, t)}) + } + } + } + return out +} + // hostInterfaces lists the machine's interfaces with their zones. func (s *Server) hostInterfaces(st *model.State) []api.HostInterface { out := []api.HostInterface{} diff --git a/internal/server/ops.go b/internal/server/ops.go index 8e73fe8..1c13812 100644 --- a/internal/server/ops.go +++ b/internal/server/ops.go @@ -31,6 +31,12 @@ func applyOp(st *model.State, op *model.Op) (int64, error) { return moveOp(st, op) case model.OpSettings: return 0, settingsOp(st, op) + case model.OpRevert: + var h model.HistoryEntry + if err := json.Unmarshal(op.Data, &h); err != nil { + return 0, badRequest("revert: %v", err) + } + return 0, engine.Undo(st, &h) } return 0, badRequest("unknown operation %q", op.Op) } diff --git a/internal/server/server.go b/internal/server/server.go index 93888a7..0216704 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -88,6 +88,9 @@ type Server struct { probation probation logs *logRing + // unitLinks are where an enabled nftables.service shows (tests change + // them). + unitLinks []string // csMu orders changes to changesets (staging, applying, discarding). csMu sync.Mutex nowFn func() time.Time @@ -113,7 +116,7 @@ func New(cfg Config, eng *engine.Engine, fw Firewall, ifaces Interfaces, log *sl log = slog.Default() } s := &Server{cfg: cfg, eng: eng, fw: fw, ifaces: ifaces, store: eng.Store(), log: log, mux: http.NewServeMux(), limiter: newRateLimiter(), - web: web, started: time.Now(), logs: newLogRing(2000), nowFn: time.Now} + web: web, started: time.Now(), logs: newLogRing(2000), nowFn: time.Now, unitLinks: nftablesUnitLinks} s.routes() return s } diff --git a/internal/server/server_test.go b/internal/server/server_test.go index e769804..4506474 100644 --- a/internal/server/server_test.go +++ b/internal/server/server_test.go @@ -795,3 +795,73 @@ func TestAutoConfirm(t *testing.T) { t.Errorf("manual confirmation: %+v", mr) } } + +// TestStagedUndo: an undo staged in a changeset changes nothing until the +// changeset is applied; then History links the two entries. +func TestStagedUndo(t *testing.T) { + e := newEnv(t) + e.srv.probationFor = 5 * time.Second + var mr api.MutationResponse + e.call(e.token, "POST", "/zones", model.Zone{Name: "lan", Interfaces: []string{"eth1"}}, &mr) + e.call(e.token, "POST", "/confirm", nil, nil) + made := mr.HistoryID + var cs model.Changeset + e.call(e.token, "POST", "/changesets", nil, &cs) + q := "?changeset=" + itoa(cs.ID) + if code := e.call(e.token, "POST", "/history/"+itoa(made)+"/revert"+q, nil, &mr); code != 200 || !mr.Staged { + t.Fatalf("staging the undo: %d %+v", code, mr) + } + if op := mr.Changeset.Ops[0]; op.Op != model.OpRevert || !strings.HasPrefix(op.Summary, "undo "+itoa(made)+":") { + t.Errorf("the operation: %+v", op) + } + if len(e.eng.State().Zones) != 1 { + t.Fatal("a staged undo was applied") + } + // Staging it again can't work: the changeset already undoes it. + var eb api.ErrorBody + if code := e.call(e.token, "POST", "/history/"+itoa(made)+"/revert"+q, nil, &eb); code != 409 { + t.Errorf("a second undo: %d %+v", code, eb) + } + if code := e.call(e.token, "POST", "/changesets/"+itoa(cs.ID)+"/apply", nil, &mr); code != 200 || !mr.Applied { + t.Fatalf("apply: %d %+v", code, mr) + } + if len(e.eng.State().Zones) != 0 { + t.Error("the undo wasn't applied") + } + h, _ := e.srv.store.GetHistory(context.Background(), mr.HistoryID) + orig, _ := e.srv.store.GetHistory(context.Background(), made) + if h.RevertOf != made || orig.RevertedBy != h.ID || !strings.HasPrefix(h.Description, "undo ") { + t.Errorf("history: %+v, original %+v", h, orig) + } +} + +// TestHostProblems: an enabled nftables.service, and an imported table +// still in force, are warned about. +func TestHostProblems(t *testing.T) { + e := newEnv(t) + dir := t.TempDir() + link := dir + "/nftables.service" + e.srv.unitLinks = []string{link} + codes := func() string { + var st api.Status + e.call(e.token, "GET", "/status", nil, &st) + var out []string + for _, p := range st.Problems { + out = append(out, p.Code) + } + return strings.Join(out, " ") + } + if c := codes(); strings.Contains(c, "nftables_service") || strings.Contains(c, "takeover_pending") { + t.Errorf("warned without cause: %s", c) + } + os.Symlink("/usr/lib/systemd/system/nftables.service", link) // dangling is fine: Lstat + // The fake ruleset lists inet main: imported from it, it still applies. + e.srv.store.SetMeta(context.Background(), importedTableKey, "inet main") + if c := codes(); !strings.Contains(c, "nftables_service") || !strings.Contains(c, "takeover_pending") { + t.Errorf("not warned: %s", c) + } + e.srv.store.SetMeta(context.Background(), importedTableKey, "inet gone") + if c := codes(); strings.Contains(c, "takeover_pending") { + t.Errorf("warned about a table that is gone: %s", c) + } +} diff --git a/internal/validate/validate.go b/internal/validate/validate.go index 468ca74..fac5021 100644 --- a/internal/validate/validate.go +++ b/internal/validate/validate.go @@ -85,6 +85,35 @@ func Check(st *model.State, env *Env) []Problem { return c.out } +// rawIgnores names what a raw rule has besides its text, which the +// ruleset wouldn't use (the action too is the text's; it isn't checked). +func rawIgnores(r *model.Rule) []string { + m := r.Match + var out []string + for _, x := range []struct { + name string + set bool + }{ + {"zones", len(m.InZones) > 0 || len(m.OutZones) > 0}, + {"addresses", len(m.Sources) > 0 || len(m.Destinations) > 0}, + {"services", len(m.Services) > 0}, + {"family", m.Family != ""}, + {"connection state", len(r.CtState) > 0 || len(r.CtStatus) > 0}, + {"time window", len(r.Days) > 0 || r.HourFrom != "" || r.HourTo != ""}, + {"rate limit", r.Limit != nil}, + {"connection limit", r.ConnLimit != 0}, + {"quota", r.Quota != nil}, + {"address grouping", r.GroupV4 != 0 || r.GroupV6 != 0}, + {"log", r.Log || r.LogPrefix != ""}, + {"reject with", r.RejectWith != ""}, + } { + if x.set { + out = append(out, x.name) + } + } + return out +} + // maxByteRate is the largest byte rate per period nftables (1.1.3) can // hold: the kernel's token bucket multiplies the rate by the period. var maxByteRate = map[string]int64{"second": 4294967294, "minute": 307445734, "hour": 5124095, "day": 213503} @@ -202,6 +231,9 @@ func (c *checker) sets() { c.add(Error, "set_entry", obj, "%v", err) } } + if len(s.Entries) > 0 && s.Dynamic { + c.add(Error, "set_dynamic_entries", obj, "a dynamic set gets its entries at run time (hc set entries add, the Entries button), each with its timeout; leave out the configured ones, or make it a set that isn't dynamic") + } if len(s.Entries) == 0 && !s.Dynamic { c.add(Warning, "set_empty", obj, "the set is empty: rules using it match nothing") } @@ -331,6 +363,9 @@ func (c *checker) rule(r *model.Rule) { if strings.ContainsAny(r.Raw, ";\n\r") { c.add(Error, "raw_rule", obj, "a raw rule is one nftables rule on one line, without ;") } + if ignored := rawIgnores(r); len(ignored) > 0 { + c.add(Error, "raw_conflict", obj, "a raw rule is used as written, so its %s would be ignored: write them in the nftables rule, or leave them out", strings.Join(ignored, ", ")) + } return } in := r.Chain != model.ChainOutput diff --git a/internal/validate/validate_test.go b/internal/validate/validate_test.go index 5a1fc34..1c8a319 100644 --- a/internal/validate/validate_test.go +++ b/internal/validate/validate_test.go @@ -66,6 +66,7 @@ func TestCodes(t *testing.T) { {"set_entry", func(st *model.State) { st.Sets[0].Entries = []string{"10.0.10.5/24"} }}, {"set_empty", func(st *model.State) { st.Sets[0].Entries = nil }}, {"set_timeout", func(st *model.State) { st.Sets[0].Timeout = 60 }}, + {"set_dynamic_entries", func(st *model.State) { st.Sets[0].Dynamic = true }}, {"service_empty", func(st *model.State) { st.Services[0].Entries = nil }}, {"service_entry", func(st *model.State) { st.Services[0].Entries = []string{"tcp/99999"} }}, {"unknown_zone", func(st *model.State) { st.Rules[1].InZones = []string{"dmz"} }}, @@ -83,6 +84,7 @@ func TestCodes(t *testing.T) { {"rule_chain", func(st *model.State) { rule(st).Chain = "prerouting" }}, {"rule_action", func(st *model.State) { rule(st).Action = "allow" }}, {"raw_rule", func(st *model.State) { rule(st).Raw = "tcp dport 22 accept; udp dport 53 accept" }}, + {"raw_conflict", func(st *model.State) { rule(st).Raw = "tcp dport 22 accept" }}, // it still has sources and services {"early_conntrack", func(st *model.State) { rule(st).Chain = model.ChainEarly; rule(st).CtState = []string{"new"} }}, {"early_reject", func(st *model.State) { rule(st).Chain = model.ChainEarly; rule(st).Action = model.ActionReject }}, {"ct_state", func(st *model.State) { rule(st).CtState = []string{"fresh"} }}, diff --git a/internal/web/static/assets/pages/history.js b/internal/web/static/assets/pages/history.js index 8f592ce..c7342ba 100644 --- a/internal/web/static/assets/pages/history.js +++ b/internal/web/static/assets/pages/history.js @@ -1,6 +1,6 @@ // History: every configuration change, with undo. import { h, mount, table, time, confirmDialog, modal, errorBox, badge } from "../ui.js"; -import { get, change, can } from "../api.js"; +import { get, change, can, state } from "../api.js"; import { rerender } from "../app.js"; export async function render(el, params) { @@ -20,10 +20,10 @@ export async function render(el, params) { } async function undo(e) { - if (!await confirmDialog("Undo change #" + e.id, "Undo “" + e.description + "”? This is recorded as a new change, which can be undone too.", "Undo")) return; + if (!await confirmDialog("Undo change #" + e.id, "Undo “" + e.description + "”? This is recorded as a new change, which can be undone too." + (!state.immediate ? " It is added to your pending changes, to review and apply." : ""), "Undo")) return; try { - // An undo can't be staged: it is applied at once. - await change("POST", "/history/" + e.id + "/revert", undefined, { immediate: true }); + // Like any edit: staged for review, or applied at once in live mode. + await change("POST", "/history/" + e.id + "/revert"); rerender(); } catch (err) { modal("Can't undo", errorBox(err)); } } diff --git a/internal/web/static/assets/pages/rules.js b/internal/web/static/assets/pages/rules.js index 3260dfc..da93fd2 100644 --- a/internal/web/static/assets/pages/rules.js +++ b/internal/web/static/assets/pages/rules.js @@ -31,17 +31,14 @@ export async function render(el, params, query) { const write = can("config", true); const info = chains.find((c) => c[0] === chain) || chains[0]; const policy = settings[chain + "_policy"]; - const bi = (builtins[chain] || []).filter(([flag]) => flag === "" || settings.builtins[flag]); - const actionBadge = (r) => badge(r.action + (r.log ? " + log" : ""), { accept: "ok", drop: "err", reject: "warn", log: "" }[r.action]); + const actionBadge = (r) => r.raw ? badge("as written", "") : badge(r.action + (r.log ? " + log" : ""), { accept: "ok", drop: "err", reject: "warn", log: "" }[r.action]); mount(el, h("div", { class: "page-head" }, h("div", { class: "grow" }, h("h1", null, "Rules"), h("div", { class: "muted" }, "Checked top to bottom; the first rule that decides wins. ", info[2])), write ? h("div", { class: "actions" }, h("button", { class: "primary", onclick: () => ruleDialog(null, chain) }, "Add rule")) : null), h("div", { class: "tabs" }, chains.map(([k, label]) => h("a", { href: "#/rules?chain=" + k, class: k === chain ? "active" : "" }, label))), h("div", { class: "panel" }, - bi.length ? h("div", { class: "builtins muted small" }, h("strong", null, "Added by Housecarl first: "), - bi.map(([, mark, text], i) => [i ? " · " : "", text, mark ? [" (", hits(cs, "b-" + mark), ")"] : ""]), " — ", - h("a", { href: "#/settings" }, "settings")) : null, + builtinsRow(chain, settings, cs), table([ { label: "#", render: (r) => r.pending === "deleted" ? "" : String(live(rules).indexOf(r) + 1) }, { label: "Match", render: (r) => r.raw ? [badge("raw", ""), " ", h("span", { class: "mono small" }, r.raw)] @@ -73,6 +70,63 @@ function countsTraffic(r) { return !r.raw && (!!r.quota || !!(r.limit && ["bytes", "kbytes", "mbytes"].includes(r.limit.unit))); } +// What each built-in does (a switch covers all its rules). +const builtinText = { + loopback: "Accept everything on lo (the firewall talking to itself)", + established: "Accept established and related connections: replies, and the rest of connections a rule let in", + drop_invalid: "Drop invalid packets: what connection tracking can't place", + icmpv6: "Accept the ICMPv6 that IPv6 needs: errors, echo, neighbour discovery, MLD (RFC 4890)", + ping_internal: "Accept ping to the firewall from internal zones", + dhcpv6_client: "Accept DHCPv6 replies on external zones (your ISP's prefix delegation)", + antispoof: "Drop sources that can't come from outside, on external zones (Settings: the lists and your prefixes)", + mss_clamp: "Clamp TCP's MSS to the route's MTU on external zones (needed with PPPoE)", +}; + +// builtinsRow sums up the chain's built-ins in one line, with a button to +// see and switch them. +function builtinsRow(chain, settings, cs) { + const list = builtins[chain] || []; + const on = list.filter(([flag]) => flag === "" || settings.builtins[flag]); + if (!list.length) return null; + return h("div", { class: "builtins muted small" }, + on.length ? (on.length === 1 ? "Housecarl adds 1 rule before yours. " : "Housecarl adds " + on.length + " rules before yours. ") : "Housecarl adds no rules before yours here. ", + h("button", { class: "small", onclick: () => builtinsDialog(chain, settings, cs) }, "Built-ins…")); +} + +// builtinsDialog lists the chain's built-ins: what each does, its hits, +// and a switch (a setting, so it covers every chain the built-in is in). +async function builtinsDialog(chain, settings, cs) { + const list = builtins[chain] || []; + const flags = [...new Set(list.map(([flag]) => flag))]; + const write = can("config", true); + const boxes = {}; + const rows = flags.map((flag) => { + const marks = list.filter(([f]) => f === flag).map(([, mark]) => mark).filter(Boolean); + const box = flag ? h("input", { type: "checkbox", checked: settings.builtins[flag], disabled: !write }) : null; + if (box) boxes[flag] = box; + return { flag, marks, box, text: flag ? builtinText[flag] : list.find(([f]) => f === flag)[2] }; + }); + const err = h("div"); + const body = h("div", null, + h("p", { class: "muted" }, "Checked in this order, before your rules (rules with a quota or a byte rate come before the established built-in, so they see every packet). Each is a setting: switching one changes every chain it is in."), + table([ + { label: "On", render: (x) => x.box || h("span", { class: "muted small" }, "always") }, + { label: "Built-in", render: (x) => x.flag ? x.text : [x.text, h("div", { class: "muted small" }, "Part of each port forward.")] }, + { label: "Hits", render: (x) => x.marks.length ? hits(cs, ...x.marks.map((m) => "b-" + m)) : h("span", { class: "muted" }, "–") }, + ], rows), + err); + await modal("Built-ins of the " + chain + " chain", body, write ? [ + { label: "Cancel", value: null }, + { label: "Save", kind: "primary", onclick: async () => { + mount(err); + const b = {}; + for (const [flag, box] of Object.entries(boxes)) if (box.checked !== settings.builtins[flag]) b[flag] = box.checked; + if (!Object.keys(b).length) return true; + try { await change("PATCH", "/settings", { builtins: b }); rerender(); return true; } catch (e) { mount(err, errorBox(e)); return false; } + } }, + ] : [{ label: "Close", value: null }], { wide: true }); +} + function extras(r, q) { const x = []; if ((r.ct_state || []).length) x.push("state " + r.ct_state.join(",")); @@ -149,7 +203,7 @@ async function del(r) { export async function ruleDialog(r, chain) { const n = await names(); r = r || { chain, action: "accept" }; - const ch = select([["input", "input (to the firewall)"], ["forward", "forward (through it)"], ["output", "output (from it)"], ["early", "early (before conntrack)"]], r.chain || chain); + const ch = select(["input", "forward", "output", "early"], r.chain || chain); const match = matchEditor(r, n); const action = select([["accept", "accept"], ["drop", "drop"], ["reject", "reject"], ["log", "log only (and go on)"]], r.action); const rejectWith = select([["", "with an ICMP error"], ["tcp-reset", "with a TCP reset (TCP rules only)"]], r.reject_with || ""); @@ -205,6 +259,7 @@ export async function ruleDialog(r, chain) { sec.open = on; for (const n of nodes) n.classList.toggle("hidden", !on); add.classList.toggle("hidden", on); + if (sec.onShow) sec.onShow(on); if (on) { const f = nodes.find((n) => n.querySelector && n.querySelector("input, select, textarea")); if (f) f.querySelector("input, select, textarea").focus(); } updateGroup(); }; @@ -234,7 +289,7 @@ export async function ruleDialog(r, chain) { field("Connection limit", conn, "Match a source that already has more than this many connections."), ]); const rawFields = section("raw", "Write it in nftables", !!r.raw, [ - field("Raw nftables", raw, "Instead of the matches above: one nftables rule, for what Housecarl can't say. Checked by nftables."), + field("Raw nftables", raw, "One nftables rule, for what Housecarl can't say, used as written: it replaces the zones, addresses, services, action, limits and log (write them in it). Checked by nftables."), ]); // Grouping only means something with a per-address limit, quota or // connection limit. @@ -248,14 +303,30 @@ export async function ruleDialog(r, chain) { limitKey.addEventListener("change", updateGroup); quotaKey.addEventListener("change", updateGroup); + // Written in nftables, a rule is its text: the fields it replaces go + // (and the other parts close). + const matchNodes = match.flat().filter(Boolean); + const actionField = field("Action", h("div", null, action, " ", rejectWith)).filter(Boolean); + const logField = field("Log", h("div", null, log, logPrefix)).filter(Boolean); + const addersLabel = h("label"); + sections.raw.onShow = (on) => { + for (const n of [...matchNodes, ...actionField, ...logField]) n.classList.toggle("hidden", on); + for (const [k, sec] of Object.entries(sections)) { + if (k === "raw") continue; + if (on && sec.open) for (const n of sec.nodes) n.classList.add("hidden"); + if (on) sec.open = false; + } + for (const b of adders.children) if (!b.textContent.includes("nftables")) b.classList.toggle("hidden", on); + updateGroup(); + }; const body = h("div", null, h("div", { class: "form-grid" }, field("Chain", ch), - ...match.flat(), - field("Action", h("div", null, action, " ", rejectWith)), + ...matchNodes, + ...actionField, ...stateFields, ...timeFields, ...rateFields, ...quotaFields, ...connFields, ...groupFields, ...rawFields, - h("label"), adders, - field("Log", h("div", null, log, logPrefix)), + addersLabel, adders, + ...logField, field("Comment", comment), r.id ? [h("label"), disabled] : null), err); @@ -270,7 +341,9 @@ export async function ruleDialog(r, chain) { async function save(dry) { mount(err); const on = (k) => sections[k].open; - const v = { chain: ch.value, ...match.value(), action: action.value, reject_with: action.value === "reject" ? rejectWith.value : "", + const isRaw = on("raw"); + const m = isRaw ? { in_zones: [], not_in: false, out_zones: [], not_out: false, sources: [], not_sources: false, destinations: [], not_destinations: false, services: [], family: "" } : match.value(); + const v = { chain: ch.value, ...m, action: isRaw ? "accept" : action.value, reject_with: !isRaw && action.value === "reject" ? rejectWith.value : "", ct_state: on("state") ? states.value() : [], ct_status: on("state") ? statuses.value() : [], days: on("time") ? days.value() : [], hour_from: on("time") ? hourFrom.value.trim() : "", hour_to: on("time") ? hourTo.value.trim() : "", limit: on("rate") && rate.value ? { rate: parseInt(rate.value, 10), unit: unit.value, per: per.value, burst: parseInt(burst.value || "0", 10), over: over.input.checked, @@ -278,8 +351,8 @@ export async function ruleDialog(r, chain) { quota: on("quota") && quotaAmount.value ? { bytes: parseInt(quotaAmount.value, 10) * (quotaUnit.value === "gb" ? 1 << 30 : 1 << 20), over: quotaOver.input.checked, per_source: quotaKey.value === "source", per_destination: quotaKey.value === "destination", reset: quotaReset.value, day: quotaReset.value === "monthly" ? parseInt(quotaDay.value || "1", 10) : 0 } : null, - group_v4: parseInt(group4.value || "0", 10), group_v6: parseInt(group6.value || "0", 10), - conn_limit: on("conn") ? parseInt(conn.value || "0", 10) : 0, log: log.input.checked, log_prefix: logPrefix.value.trim(), + group_v4: isRaw ? 0 : parseInt(group4.value || "0", 10), group_v6: isRaw ? 0 : parseInt(group6.value || "0", 10), + conn_limit: on("conn") ? parseInt(conn.value || "0", 10) : 0, log: !isRaw && log.input.checked, log_prefix: isRaw ? "" : logPrefix.value.trim(), raw: on("raw") ? raw.value.trim() : "", comment: comment.value.trim() }; if (r.id) v.disabled = disabled.input.checked; try { diff --git a/internal/web/static/assets/pages/sets.js b/internal/web/static/assets/pages/sets.js index 9536d55..fe7eae8 100644 --- a/internal/web/static/assets/pages/sets.js +++ b/internal/web/static/assets/pages/sets.js @@ -31,19 +31,29 @@ async function setDialog(s) { s = s || { entries: [] }; const name = input({ value: s.name || "", placeholder: "trusted" }); const list = listEditor(s.entries, { placeholder: "192.168.1.0/24, 2001:db8:1::/64, 192.168.1.100-192.168.1.199", addLabel: "Add entry" }); - const dynamic = checkbox("dynamic: entries are added at run time", s.dynamic); + const dynamic = checkbox("dynamic: entries are added at run time (the Entries button, hc set entries, a program with a token), each for a time", s.dynamic); + // A dynamic set has no configured entries; only it has a timeout. + const entryFields = field("Entries", list).filter(Boolean); + const timeoutFields = field("Entry timeout", timeout, "How long an entry stays unless it is added with its own. Empty: until removed (or a reboot).").filter(Boolean); + const showKind = () => { + for (const n of entryFields) n.classList.toggle("hidden", dynamic.input.checked); + for (const n of timeoutFields) n.classList.toggle("hidden", !dynamic.input.checked); + }; + dynamic.input.addEventListener("change", showKind); + showKind(); const timeout = input({ value: s.timeout ? fmtSeconds(s.timeout) : "", placeholder: "1h, 1d (none: until removed)", class: "short" }); const comment = input({ value: s.comment || "" }); const err = h("div"); await modal(s.id ? "Edit set " + s.name : "Add set", h("div", null, h("div", { class: "form-grid" }, - field("Name", name), field("Entries", list), h("label"), dynamic, field("Entry timeout", timeout, "A dynamic set's default."), field("Comment", comment)), err), [ + field("Name", name), ...entryFields, h("label"), dynamic, ...timeoutFields, field("Comment", comment)), err), [ { label: "Cancel", value: null }, { label: s.id ? "Save" : "Add set", kind: "primary", onclick: async () => { mount(err); const t = parseSeconds(timeout.value); if (t === null) { mount(err, h("div", { class: "msg err" }, "Write the timeout as 1h, 30m or 1d.")); return false; } try { - await change(s.id ? "PATCH" : "POST", s.id ? "/sets/" + s.id : "/sets", { name: name.value.trim(), entries: list.value(), dynamic: dynamic.input.checked, timeout: t, comment: comment.value.trim() }); + await change(s.id ? "PATCH" : "POST", s.id ? "/sets/" + s.id : "/sets", { name: name.value.trim(), entries: dynamic.input.checked ? [] : list.value(), dynamic: dynamic.input.checked, + timeout: dynamic.input.checked ? t : 0, comment: comment.value.trim() }); rerender(); return true; } catch (e) { mount(err, errorBox(e)); return false; } diff --git a/scripts/e2e.sh b/scripts/e2e.sh index 11b80a3..7ea211b 100755 --- a/scripts/e2e.sh +++ b/scripts/e2e.sh @@ -240,9 +240,14 @@ contains "^jellyfin$" run "a stale HOUSECARL_CHANGESET is refused" 1 CE zone add stale -i stale0 contains "is applied" CS2=$(H changeset create -q) -run "an undo can't be staged" 6 H -C "$CS2" history undo 1 +UNDO=$(H zone add undome -i undo0 | sed -n 's/.*history \([0-9]*\);.*/\1/p') +run "an undo is staged" 0 H -C "$CS2" history undo "$UNDO" +contains "staged in changeset $CS2" +run "not applied yet: the zone is there" 0 H zone show undome +run "an import can't be staged" 6 sh -c "echo '{\"nftables\":[]}' | docker exec -i ${P}-router /w/hc -C $CS2 import -" contains "can't be staged" run "discarded" 0 H changeset discard "$CS2" +run "the zone stays" 0 H zone show undome run "a dry run shows the ruleset's diff" 0 H -n set add e2e_dry 10.1.2.3 contains "^+.*set e2e_dry_v4"