diff --git a/internal/action/audit.go b/internal/action/audit.go index d02da8ffa6..577a0676fb 100644 --- a/internal/action/audit.go +++ b/internal/action/audit.go @@ -71,12 +71,26 @@ func (s *Action) Audit(c *cli.Context) error { case "csv": return saveReport(ctx, r.RenderCSV, c.String("output-file"), "csv") default: - if err := r.PrintResults(ctx); err != nil { - return err + var err error + if c.Bool("full") { + debug.Log("Printing full report") + err = r.PrintResults(ctx) + } + if c.Bool("summary") { + debug.Log("Printing summary") + + nerr := r.PrintSummary(ctx) + // do not overwrite err if it is already set + if err == nil { + err = nerr + } + } + if !c.Bool("full") && !c.Bool("summary") { + out.Warning(ctx, "No output format specified. Use `--full` or `--summary` to specify.") } - } - return nil + return err + } } func saveReport(ctx context.Context, f func(io.Writer) error, path, suffix string) error { diff --git a/internal/action/audit_test.go b/internal/action/audit_test.go index 85f982822a..a10548a4af 100644 --- a/internal/action/audit_test.go +++ b/internal/action/audit_test.go @@ -33,17 +33,17 @@ func TestAudit(t *testing.T) { stdout = os.Stdout }() - t.Run("expect audit complaints on very weak passwords", func(t *testing.T) { + t.Run("expect audit to complains on very weak passwords", func(t *testing.T) { sec := secrets.NewAKV() sec.SetPassword("123") require.NoError(t, act.Store.Set(ctx, "bar", sec)) require.NoError(t, act.Store.Set(ctx, "baz", sec)) - require.Error(t, act.Audit(gptest.CliCtx(ctx, t))) + require.Error(t, act.Audit(gptest.CliCtxWithFlags(ctx, t, map[string]string{"full": "true"})), buf.String()) buf.Reset() }) - t.Run("test with filter and very passwords", func(t *testing.T) { + t.Run("test with filter", func(t *testing.T) { c := gptest.CliCtx(ctx, t, "foo") require.Error(t, act.Audit(c)) buf.Reset() diff --git a/internal/action/commands.go b/internal/action/commands.go index 7a49dd7c3c..a68003b70c 100644 --- a/internal/action/commands.go +++ b/internal/action/commands.go @@ -97,8 +97,13 @@ func (s *Action) GetCommands() []*cli.Command { Usage: "HTML template. If not set use the built-in default.", }, &cli.BoolFlag{ - Name: "failed", - Usage: "Report only entries that failed validation. Default: false (reports all)", + Name: "full", + Usage: "Print full details of all findings. Default: false", + }, + &cli.BoolFlag{ + Name: "summary", + Usage: "Print a summary of the audit results. Default: true (print summary)", + Value: true, }, }, }, diff --git a/internal/audit/output.go b/internal/audit/output.go index b682ac50ba..e9051534ce 100644 --- a/internal/audit/output.go +++ b/internal/audit/output.go @@ -7,6 +7,7 @@ import ( "io" "os" "sort" + "strings" "text/template" "time" @@ -23,17 +24,35 @@ func (r *Report) PrintResults(ctx context.Context) error { return nil } + debug.Log("Printing results for %d secrets", len(r.Secrets)) + var failed bool for _, name := range set.SortedKeys(r.Secrets) { s := r.Secrets[name] - out.Printf(ctx, "%s (age: %s)", name, s.Age.String()) + var sb strings.Builder + sb.WriteString(fmt.Sprintf("%s (age: %s) ", name, s.HumanizeAge())) + if !s.HasFindings() { + sb.WriteString("OK") + out.OK(ctx, sb.String()) + + continue + } + sb.WriteString("Potentially weak. ") for k, v := range s.Findings { if v.Severity == "error" || v.Severity == "warning" { failed = true } - out.Errorf(ctx, "[%s] %s: %s", v.Severity, k, v.Message) + switch v.Severity { + case "error": + fallthrough + case "warning": + sb.WriteString(fmt.Sprintf("%s: %s. ", k, v.Message)) + default: + continue + } } + out.Warning(ctx, sb.String()) } if failed { @@ -43,6 +62,34 @@ func (r *Report) PrintResults(ctx context.Context) error { return nil } +func (r *Report) PrintSummary(ctx context.Context) error { + if r == nil { + out.Warning(ctx, "Empty report") + + return nil + } + + debug.Log("Printing summary for %d findings", len(r.Findings)) + + for _, name := range set.SortedKeys(r.Findings) { + f := r.Findings[name] + if f.Len() < 1 { + continue + } + // TODO add details about the analyzer, not just the name + out.Printf(ctx, "Analyzer %s found issues: ", name) + for _, v := range set.Sorted(f.Elements()) { + out.Printf(ctx, "- %s", v) + } + } + + if len(r.Findings) > 0 { + return fmt.Errorf("weak password or duplicates detected") + } + + return nil +} + func (r *Report) RenderCSV(w io.Writer) error { cw := csv.NewWriter(w) diff --git a/internal/audit/report.go b/internal/audit/report.go index 6db3fe72de..3402e64491 100644 --- a/internal/audit/report.go +++ b/internal/audit/report.go @@ -7,6 +7,7 @@ import ( "github.com/gopasspw/gopass/internal/hashsum" "github.com/gopasspw/gopass/internal/set" + "github.com/gopasspw/gopass/pkg/debug" ) type Finding struct { @@ -15,11 +16,39 @@ type Finding struct { } type SecretReport struct { - Name string + Name string + // analyzer -> finding details Findings map[string]Finding Age time.Duration } +func (s *SecretReport) HasFindings() bool { + for _, f := range s.Findings { + if f.Severity != "none" { + return true + } + } + + return false +} + +func (s *SecretReport) HumanizeAge() string { + if s.Age < 24*time.Hour { + return fmt.Sprintf("%d hours", int(s.Age.Hours())) + } + days := int(s.Age.Hours() / 24) + if days < 30 { + return fmt.Sprintf("%d days", days) + } + months := days / 30 + if months < 12 { + return fmt.Sprintf("%d months", months) + } + years := months / 12 + + return fmt.Sprintf("%d years", years) +} + func errors(e []error) []string { s := make([]string, 0, len(e)) for _, es := range e { @@ -30,15 +59,25 @@ func errors(e []error) []string { } type Report struct { - Secrets map[string]SecretReport + // secret name -> report + Secrets map[string]SecretReport + + // finding -> secrets + Findings map[string]set.Set[string] + Template string Duration time.Duration } type ReportBuilder struct { + // protects all below sync.Mutex + // secret name -> report secrets map[string]SecretReport + // finding -> secrets + findings map[string]set.Set[string] + // SHA512(password) -> secret names duplicates map[string]set.Set[string] @@ -76,6 +115,7 @@ func (r *ReportBuilder) AddFinding(secret, finding, message, severity string) { r.Lock() defer r.Unlock() + // record individual findings s := r.secrets[secret] s.Name = secret if s.Findings == nil { @@ -86,6 +126,11 @@ func (r *ReportBuilder) AddFinding(secret, finding, message, severity string) { f.Severity = severity s.Findings[finding] = f r.secrets[secret] = s + + // record secrets per finding, for the summary + ss := r.findings[finding] + ss.Add(secret) + r.findings[finding] = ss } func (r *ReportBuilder) SetAge(name string, age time.Duration) { @@ -105,6 +150,7 @@ func (r *ReportBuilder) SetAge(name string, age time.Duration) { func newReport() *ReportBuilder { return &ReportBuilder{ secrets: make(map[string]SecretReport, 512), + findings: make(map[string]set.Set[string], 512), duplicates: make(map[string]set.Set[string], 512), sha1sums: make(map[string]set.Set[string], 512), t0: time.Now().UTC(), @@ -134,6 +180,7 @@ func (r *ReportBuilder) Finalize() *Report { ret := &Report{ Secrets: make(map[string]SecretReport, len(r.secrets)), + Findings: make(map[string]set.Set[string], len(r.findings)), Duration: time.Since(r.t0), } @@ -141,5 +188,11 @@ func (r *ReportBuilder) Finalize() *Report { ret.Secrets[k] = r.secrets[k] } + for k := range r.findings { + ret.Findings[k] = r.findings[k] + } + + debug.Log("Finalized report: %d secrets, %d findings", len(ret.Secrets), len(ret.Findings)) + return ret } diff --git a/internal/backend/storage/fs/rcs.go b/internal/backend/storage/fs/rcs.go index 169a1c04f0..a0e710ffd6 100644 --- a/internal/backend/storage/fs/rcs.go +++ b/internal/backend/storage/fs/rcs.go @@ -75,7 +75,7 @@ func (s *Store) Revisions(context.Context, string) ([]backend.Revision, error) { Hash: "latest", Date: time.Now(), }, - }, nil + }, backend.ErrNotSupported } // GetRevision only supports getting the latest revision. diff --git a/internal/backend/storage/fs/rcs_test.go b/internal/backend/storage/fs/rcs_test.go index 94c2b6d7f0..d32ed14425 100644 --- a/internal/backend/storage/fs/rcs_test.go +++ b/internal/backend/storage/fs/rcs_test.go @@ -28,7 +28,7 @@ func TestRCS(t *testing.T) { assert.Equal(t, "fs", g.Name()) require.Error(t, g.AddRemote(ctx, "foo", "bar")) revs, err := g.Revisions(ctx, "foo") - require.NoError(t, err) + require.Error(t, err) assert.Len(t, revs, 1) body, err := g.GetRevision(ctx, "foo", "latest") require.Error(t, err) diff --git a/internal/store/leaf/convert.go b/internal/store/leaf/convert.go index c04f9ad194..5c0e8b02ac 100644 --- a/internal/store/leaf/convert.go +++ b/internal/store/leaf/convert.go @@ -2,6 +2,7 @@ package leaf import ( "context" + "errors" "fmt" "os" "path/filepath" @@ -95,7 +96,10 @@ func (s *Store) Convert(ctx context.Context, cryptoBe backend.CryptoBackend, sto debug.Log("converting %s", e) revs, err := s.ListRevisions(ctx, e) if err != nil { - return fmt.Errorf("failed to list revision of %s: %w", e, err) + // the fs backend does not support revisions. but we can still convert the "latest" (only) revision. + if !errors.Is(err, backend.ErrNotSupported) || len(revs) < 1 { + return fmt.Errorf("failed to list revision of %s: %w", e, err) + } } sort.Sort(sort.Reverse(backend.Revisions(revs))) diff --git a/internal/store/leaf/rcs_test.go b/internal/store/leaf/rcs_test.go index f62ef6b2ee..6466e9307c 100644 --- a/internal/store/leaf/rcs_test.go +++ b/internal/store/leaf/rcs_test.go @@ -46,8 +46,8 @@ func TestGitRevisions(t *testing.T) { require.NoError(t, s.Storage().InitConfig(ctx, "foo", "bar@baz.com")) revs, err := s.ListRevisions(ctx, "foo") - require.NoError(t, err) - assert.Len(t, revs, 1) + require.Error(t, err) // not supported by the fs backend + assert.Len(t, revs, 1) // but it will still give a fake "latest" rev sec, err := s.GetRevision(ctx, "foo", "latest") require.Error(t, err) diff --git a/internal/store/root/rcs_test.go b/internal/store/root/rcs_test.go index 51177de55a..b4efed35ce 100644 --- a/internal/store/root/rcs_test.go +++ b/internal/store/root/rcs_test.go @@ -25,6 +25,6 @@ func TestRCS(t *testing.T) { require.Error(t, rs.RCSStatus(ctx, "")) revs, err := rs.ListRevisions(ctx, "foo") - require.NoError(t, err) + require.Error(t, err) assert.Len(t, revs, 1) } diff --git a/main_test.go b/main_test.go index 2fe9033686..331c3856e4 100644 --- a/main_test.go +++ b/main_test.go @@ -60,7 +60,6 @@ var commandsWithError = set.Map([]string{ ".alias.add", ".alias.remove", ".alias.delete", - ".audit", ".cat", ".clone", ".copy", diff --git a/tests/audit_test.go b/tests/audit_test.go index f93b3f311c..538e8c9b56 100644 --- a/tests/audit_test.go +++ b/tests/audit_test.go @@ -17,6 +17,7 @@ func TestAudit(t *testing.T) { t.Run("audit the test store", func(t *testing.T) { out, err := ts.run("audit") require.Error(t, err) - assert.Contains(t, out, "weak password") + assert.Contains(t, out, "crunchy") + assert.Contains(t, out, "zxcvbn") }) } diff --git a/zsh.completion b/zsh.completion index f28f81a504..4a02eb37bb 100644 --- a/zsh.completion +++ b/zsh.completion @@ -24,7 +24,7 @@ _gopass () { ;; audit) - _arguments : "--format[Output format. text, csv or html. Default: text]" "--output-file[Output filename. Used for csv and html]" "--template[HTML template. If not set use the built-in default.]" "--failed[Report only entries that failed validation. Default: false (reports all)]" + _arguments : "--format[Output format. text, csv or html. Default: text]" "--output-file[Output filename. Used for csv and html]" "--template[HTML template. If not set use the built-in default.]" "--full[Print full details of all findings. Default: false]" "--summary[Print a summary of the audit results. Default: true (print summary)]" ;;