From b87cc6fb6250e8b517ef945498771c37eaec77b1 Mon Sep 17 00:00:00 2001 From: abrown38 Date: Fri, 23 Oct 2026 12:53:37 +0100 Subject: [PATCH] CLI: shorter, plainer output Tables name a journal source by its first two units or identifiers; guard test shows 20 matching lines and addresses (--all for all), no /32; Housecarl connection errors without Go's request URL, said once by guard check; status tells "not running" from "no permission"; bans says when there are none; "1 ban lifted". Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 12 ++++ cmd/guard/main.go | 129 +++++++++++++++++++++++++++-------- internal/daemon/daemon.go | 24 ++++++- internal/housecarl/client.go | 37 ++++++++-- scripts/e2e.sh | 4 +- 5 files changed, 169 insertions(+), 37 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 263ae2f..0ed4970 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -36,3 +36,15 @@ All notable changes to Guard are documented here. The format follows `SupplementaryGroups`. `sudo -u guard guard test` and `guard check` could read neither `/var/log`'s files (`permission denied`) nor the journal. Upgrading adds the groups. +- Shorter, plainer command output. Tables name a journal source by its + first two units or identifiers (`journal postfix/smtpd, + postfix/submission/smtpd +1`), so `presets`, `check` and `status` + fit a normal terminal. `guard test` prints the first 20 matching + lines (not 200) and the first 20 addresses it would ban (`--all` for + every one), single addresses without `/32`, and counts the matches + from `never_ban` addresses. When Housecarl doesn't answer, `guard check` + says so once ("Housecarl at URL doesn't answer: connection refused") + instead of once per ban list with the request's URL, and only asks for + `log:read` when Housecarl refused the log. `guard status` tells "not + running" from "no permission". `guard bans` says when there are none; + `guard unban` says "1 ban lifted". diff --git a/cmd/guard/main.go b/cmd/guard/main.go index bc7511e..a662c72 100644 --- a/cmd/guard/main.go +++ b/cmd/guard/main.go @@ -14,6 +14,7 @@ import ( "net" "net/http" "net/netip" + "net/url" "os" "os/exec" "os/signal" @@ -161,7 +162,7 @@ func checkCmd() *cobra.Command { } lists[x.Ban.List] = true drops = drops || x.Kind() == config.SourceHousecarlDrops - fmt.Fprintf(w, "%s\t%s\t%d\t%d in %s\t%s\t%s\n", x.Name, daemon.Describe(x), len(x.Match), x.Hits, x.Within, x.Ban.List, ports(x.Ban.Protocol, x.Ban.Port)) + fmt.Fprintf(w, "%s\t%s\t%d\t%d in %s\t%s\t%s\n", x.Name, daemon.Short(x), len(x.Match), x.Hits, x.Within, x.Ban.List, ports(x.Ban.Protocol, x.Ban.Port)) } }) hc, err := client(cfg) @@ -169,9 +170,17 @@ func checkCmd() *cobra.Command { return err } bad := false + names := make([]string, 0, len(lists)) for l := range lists { + names = append(names, l) + } + sort.Strings(names) + for _, l := range names { bl, err := hc.BanList(ctx(), l) + var down *housecarl.UnreachableError switch { + case errors.As(err, &down): + return err case err != nil: fmt.Printf("ban list %s: %v\n", l, err) bad = true @@ -183,7 +192,11 @@ func checkCmd() *cobra.Command { } if drops { if _, err := hc.Log(ctx(), time.Now()); err != nil { - fmt.Printf("Housecarl's log (housecarl-drops): %v (the token needs log:read)\n", err) + hint := "" + if housecarl.Refused(err) { + hint = " (the token needs log:read)" + } + fmt.Printf("Housecarl's log (housecarl-drops): %v%s\n", err, hint) bad = true } else { fmt.Println("Housecarl's log (housecarl-drops): ok") @@ -266,7 +279,7 @@ func presetsCmd() *cobra.Command { fmt.Fprintln(w, "PRESET\tSOURCE\tWHAT") for _, n := range names { p := ps[n] - fmt.Fprintf(w, "%s\t%s\t%s\n", n, daemon.Describe(&config.Watch{File: p.File, Journal: p.Journal, Source: p.Source, Protocol: "tcp"}), p.Description) + fmt.Fprintf(w, "%s\t%s\t%s\n", n, daemon.Short(&config.Watch{File: p.File, Journal: p.Journal, Source: p.Source, Protocol: "tcp"}), p.Description) } }) return nil @@ -331,10 +344,14 @@ the watch's window (within) is used.`, count int ban bool } + type found struct { + addr, text, why string + } hits := map[string]*hit{} order := []string{} tr := map[string][]time.Time{} - matched := 0 + var shown []found + matched, spared := 0, 0 for _, l := range in { a, ok := m.Match(l.Text) if !ok { @@ -342,17 +359,14 @@ the watch's window (within) is used.`, } matched++ why := never.Why(a) - if all || matched <= 200 { - note := "" - if why != "" { - note = " (never banned: " + why + ")" - } - fmt.Printf("%-39s %s%s\n", a, clip(l.Text, 160), note) + if all || len(shown) < testShown { + shown = append(shown, found{a.String(), l.Text, why}) } if why != "" { + spared++ continue } - g := match.Group(a, 64).String() + g := address(match.Group(a, 64)) h := hits[g] if h == nil { h = &hit{} @@ -371,39 +385,72 @@ the watch's window (within) is used.`, h.ban = h.ban || h.count >= w.Hits } } - if matched > 200 && !all { - fmt.Printf("… %d more matching lines (--all shows them)\n", matched-200) + width := 0 + for _, f := range shown { + width = max(width, len(f.addr)) } - fmt.Printf("\n%d lines, %d matched, %d addresses (IPv6 by /64)\n", len(in), matched, len(order)) + for _, f := range shown { + note := "" + if f.why != "" { + note = " (never banned: " + f.why + ")" + } + fmt.Printf("%-*s %s%s\n", width, f.addr, clip(f.text, 120), note) + } + if matched > len(shown) { + fmt.Printf("… and %d more matching lines (--all shows them)\n", matched-len(shown)) + } + fmt.Printf("\n%d lines, %d matched, %s (IPv6 by /64)", len(in), matched, plural(len(order), "address")) + if spared > 0 { + fmt.Printf("; %d matches from never_ban addresses", spared) + } + fmt.Println() sort.SliceStable(order, func(i, j int) bool { return hits[order[i]].count > hits[order[j]].count }) - banned := 0 + var banned []string for _, g := range order { if hits[g].ban { - banned++ + banned = append(banned, g) } } - if banned == 0 { - fmt.Printf("nobody would be banned (%d hits within %s)\n", w.Hits, w.Within) + rule := fmt.Sprintf("%d hits within %s", w.Hits, w.Within) + if !timed { + rule = fmt.Sprintf("%d hits; a file's lines have no times, so all of them count", w.Hits) + } + if len(banned) == 0 { + fmt.Printf("Nobody would be banned (%s)\n", rule) return nil } - window := "within " + w.Within.String() - if !timed { - window = "in all (a file's lines have no time)" + fmt.Printf("Would ban %d (%s):\n", len(banned), rule) + width = 0 + for _, g := range banned { + width = max(width, len(g)) } - fmt.Printf("would ban %d (%d hits %s):\n", banned, w.Hits, window) - for _, g := range order { - if hits[g].ban { - fmt.Printf(" %-39s %d matches\n", g, hits[g].count) + for i, g := range banned { + if i == testShown && !all { + fmt.Printf(" … and %d more (--all shows them)\n", len(banned)-i) + break } + fmt.Printf(" %-*s %s\n", width, g, plural(hits[g].count, "match")) } return nil }, } cmd.Flags().IntVarP(&lines, "lines", "n", 5000, "a journal watch without FILE: the last this many entries") - cmd.Flags().BoolVar(&all, "all", false, "print every matching line, not the first 200") + cmd.Flags().BoolVar(&all, "all", false, "print every matching line and address, not the first 20") return cmd } +// testShown is how many matching lines, and addresses to ban, guard +// test prints without --all. +const testShown = 20 + +// address is a ban's address: a single address without its /32 or /128. +func address(p netip.Prefix) string { + if p.IsSingleIP() { + return p.Addr().String() + } + return p.String() +} + func readLines(path string) ([]source.Line, error) { var r io.Reader = os.Stdin if path != "-" { @@ -473,8 +520,17 @@ func statusCmd() *cobra.Command { return d.DialContext(c, "unix", cfg.Socket) }}} resp, err := hc.Get("http://guard/status") - if err != nil { - return fmt.Errorf("Guard isn't running, or %s can't be reached (as root, or the guard user): %w", cfg.Socket, err) + switch { + case errors.Is(err, os.ErrNotExist), errors.Is(err, syscall.ECONNREFUSED): + return fmt.Errorf("Guard isn't running (nothing answers on %s)", cfg.Socket) + case errors.Is(err, os.ErrPermission): + return fmt.Errorf("no permission for %s: run this as root or the guard user", cfg.Socket) + case err != nil: + var ue *url.Error + if errors.As(err, &ue) { + err = ue.Err + } + return fmt.Errorf("Guard's socket %s: %w", cfg.Socket, err) } defer resp.Body.Close() var s daemon.Status @@ -519,6 +575,17 @@ func statusCmd() *cobra.Command { } } +// plural is "1 ban", "2 bans", "2 matches". +func plural(n int, what string) string { + if n == 1 { + return "1 " + what + } + if strings.HasSuffix(what, "s") || strings.HasSuffix(what, "ch") { + return strconv.Itoa(n) + " " + what + "es" + } + return strconv.Itoa(n) + " " + what + "s" +} + func splitComma(s string) []string { if s == "" { return nil @@ -562,6 +629,10 @@ func bansCmd() *cobra.Command { if flagJSON { return printJSON(bans) } + if len(bans) == 0 { + fmt.Printf("No active bans in %s\n", l) + return nil + } table(func(w io.Writer) { fmt.Fprintln(w, "ID\tADDRESS\tPORT\tENDS IN\tBY\tSOURCE") for _, b := range bans { @@ -668,7 +739,7 @@ func unbanCmd() *cobra.Command { if err != nil { return err } - fmt.Printf("%d bans lifted\n", n) + fmt.Printf("%s lifted\n", plural(n, "ban")) return nil }, } diff --git a/internal/daemon/daemon.go b/internal/daemon/daemon.go index ed57b36..396d757 100644 --- a/internal/daemon/daemon.go +++ b/internal/daemon/daemon.go @@ -173,6 +173,28 @@ func Describe(w *config.Watch) string { return "file " + w.File } +// Short describes a watch's source in a table's few columns: at most +// two journal units or identifiers, then how many more. +func Short(w *config.Watch) string { + switch w.Kind() { + case "journal": + names := w.Journal.Unit + if len(names) == 0 { + names = w.Journal.Identifier + } else { + names = append(append([]string(nil), names...), w.Journal.Identifier...) + } + s := "journal " + strings.Join(names[:min(len(names), 2)], ", ") + if len(names) > 2 { + s += fmt.Sprintf(" +%d", len(names)-2) + } + return s + case config.SourceHousecarlDrops: + return "Housecarl drops (" + w.Protocol + ")" + } + return "file " + w.File +} + // loadGroups asks Housecarl how each ban list groups IPv6 addresses, so // bounties are counted as the list bans. A list that can't be read now // counts by /64, its default. @@ -409,7 +431,7 @@ type WatchStatus struct { func (d *Daemon) Status() Status { s := Status{Started: d.started} for _, w := range d.watches { - s.Watches = append(s.Watches, WatchStatus{Name: w.Cfg.Name, Source: Describe(w.Cfg), List: w.Cfg.Ban.List, Hits: w.Cfg.Hits, + s.Watches = append(s.Watches, WatchStatus{Name: w.Cfg.Name, Source: Short(w.Cfg), List: w.Cfg.Ban.List, Hits: w.Cfg.Hits, Within: w.Cfg.Within.String(), Lines: w.lines.Load(), Matches: w.matches.Load(), Bans: w.bans.Load(), Top: w.Tracker.Top(10, d.Now())}) } s.Recent, s.Pending = d.state.Snapshot() diff --git a/internal/housecarl/client.go b/internal/housecarl/client.go index 248f488..7609536 100644 --- a/internal/housecarl/client.go +++ b/internal/housecarl/client.go @@ -27,9 +27,10 @@ const Prefix = "/api/v1" // Client talks to one Housecarl. type Client struct { - base string - token string - http *http.Client + base string + socket string + token string + http *http.Client } // Options say how to reach Housecarl. @@ -72,7 +73,7 @@ func New(o Options) (*Client, error) { } tr.TLSClientConfig = cfg } - return &Client{base: base, token: strings.TrimSpace(o.Token), http: &http.Client{Transport: tr, Timeout: timeout}}, nil + return &Client{base: base, socket: o.Socket, token: strings.TrimSpace(o.Token), http: &http.Client{Transport: tr, Timeout: timeout}}, nil } // Error is an error answer from Housecarl. @@ -89,6 +90,32 @@ func (e *Error) Error() string { return e.Message } +// UnreachableError is a request Housecarl never answered: no +// connection, a TLS failure, a timeout. +type UnreachableError struct { + Where string + Err error +} + +func (e *UnreachableError) Error() string { + err := e.Err + var ue *url.Error + if errors.As(err, &ue) { + err = ue.Err // without the method and the URL with its query + } + return "Housecarl at " + e.Where + " doesn't answer: " + err.Error() +} + +func (e *UnreachableError) Unwrap() error { return e.Err } + +// where is where the client reaches Housecarl, for messages. +func (c *Client) where() string { + if c.socket != "" { + return c.socket + } + return c.base +} + // Refused reports whether Housecarl refused a request for good: asking // again won't help (a bad request, no permission, an address never // banned). Anything else (no answer, a server error) is worth retrying. @@ -123,7 +150,7 @@ func (c *Client) do(ctx context.Context, method, path string, query url.Values, } resp, err := c.http.Do(req) if err != nil { - return err + return &UnreachableError{Where: c.where(), Err: err} } defer resp.Body.Close() data, _ := io.ReadAll(io.LimitReader(resp.Body, 16<<20)) diff --git a/scripts/e2e.sh b/scripts/e2e.sh index 345f6a4..85c6297 100755 --- a/scripts/e2e.sh +++ b/scripts/e2e.sh @@ -142,10 +142,10 @@ not_contains "$NET2" run "ssh from it still works" 0 connect 22 $NET2 contains "^ssh$" run "guard test over the log" 0 docker exec ${P}-srv /w/guard -c /w/guard.yaml test ssh /w/auth.log -contains "would ban 1" +contains "Would ban 1" contains "never banned: in never_ban $NET2/32" run "guard unban" 0 docker exec ${P}-srv /w/guard -c /w/guard.yaml unban $NET -contains "1 bans lifted" +contains "1 ban lifted" run "ssh again" 0 connect 22 contains "^ssh$"