From b1914d40f2cc7bd9a00957a411858824a2cff7a7 Mon Sep 17 00:00:00 2001 From: Turgay Nalbant <182372382+tnalbant@users.noreply.github.com> Date: Tue, 27 Oct 2026 13:16:02 +0300 Subject: [PATCH 1/2] Scope audit delete-all to the active filter and export every matching row "X" promised "delete ALL matched entries, every one of the N" and then called ClearLocalLog(), which empties the whole file: a user who filtered the view lost entries that were never on screen. Deletion now runs through the same client.AuditQuery the view used, so only the visible subset is removed. "E" asked Audit.List for 10_000 rows, but List clamps Limit to 1000, so every export above 1000 matching entries was cut off with no warning. The export now walks pages of client.MaxAuditPage until the matching set is complete, and says so explicitly if the 120s budget ends first. The filter lives in one place: AuditQuery.Matches() is what List uses, and the TUI applies it to raw log lines via AuditEntryFromLocalLine(), so the view and the bulk actions cannot drift apart again. Signed-off-by: Turgay Nalbant <182372382+tnalbant@users.noreply.github.com> --- packages/core/client/localstore.go | 15 +++ packages/core/client/resources_audit.go | 127 +++++++++++-------- packages/core/client/resources_audit_test.go | 12 +- packages/tui/audit/audit.go | 11 ++ packages/tui/audit/audit_keys.go | 60 +++++++-- packages/tui/audit/audit_keys_test.go | 60 ++++++++- packages/tui/locallog.go | 41 ++++++ packages/tui/locallog_test.go | 50 +++++++- 8 files changed, 306 insertions(+), 70 deletions(-) diff --git a/packages/core/client/localstore.go b/packages/core/client/localstore.go index 45ca656..b61ea62 100644 --- a/packages/core/client/localstore.go +++ b/packages/core/client/localstore.go @@ -435,6 +435,21 @@ func readLog() []logLine { return out } +// AuditEntryFromLocalLine decodes one raw line of the local audit log into the +// same entry shape List returns. Callers that rewrite the log file themselves +// use it with AuditQuery.Matches so deleting "everything in the current view" +// applies exactly the filter the view used, instead of a second copy of it. +func AuditEntryFromLocalLine(raw []byte) (AuditEntry, bool) { + var l logLine + if json.Unmarshal(raw, &l) != nil { + return AuditEntry{}, false + } + if t, err := time.Parse(time.RFC3339Nano, l.TS); err == nil { + l.at = t.UnixMilli() + } + return toAuditEntry(l), true +} + func toAuditEntry(l logLine) AuditEntry { decision := l.Decision if decision == "" { diff --git a/packages/core/client/resources_audit.go b/packages/core/client/resources_audit.go index b123637..efe3512 100644 --- a/packages/core/client/resources_audit.go +++ b/packages/core/client/resources_audit.go @@ -9,10 +9,16 @@ import ( "errors" "fmt" "strings" + "time" ) type Audit struct{ c *Client } +// MaxAuditPage is the largest page Audit.List returns; a larger Limit is clamped +// to it. Callers that need every row (export, bulk delete) must walk the pages +// instead of asking for one oversized page and assuming they got everything. +const MaxAuditPage = 1000 + type AuditQuery struct { Filter string Tool string @@ -26,75 +32,90 @@ type AuditQuery struct { Signal string } +// Matches reports whether one entry falls inside the query's filter set. Limit, +// Offset and the paging window are deliberately ignored: they scope the page a +// caller shows, not the set a filter selects, so callers that act on "everything +// currently shown" (delete, export) can reuse this predicate on the raw log +// instead of re-implementing the filter and drifting away from the view. +func (q AuditQuery) Matches(e AuditEntry) bool { + decision := q.Filter + if decision == "DENIED" { + decision = "DENY" + } + lower := strings.ToLower + if decision != "" && e.Decision != decision { + return false + } + if q.Tool != "" && !strings.Contains(lower(e.ToolName), lower(q.Tool)) { + return false + } + if q.AgentName != "" { + name := "" + if e.AgentName != nil { + name = *e.AgentName + } + if !strings.Contains(lower(name), lower(q.AgentName)) { + return false + } + } + if q.SessionID != "" && (e.SessionID == nil || *e.SessionID != q.SessionID) { + return false + } + reason := "" + if e.Reason != nil { + reason = *e.Reason + } + if q.Signal == "dlp" && len(e.DLPMatches) == 0 && !strings.Contains(lower(reason), "dlp") { + return false + } + if q.Signal == "ratelimit" && !e.RateLimitBurst && !strings.Contains(lower(reason), "rate limit") { + return false + } + if q.Search != "" { + needle := lower(q.Search) + if !strings.Contains(lower(e.ToolName), needle) && + !strings.Contains(lower(reason), needle) && + !strings.Contains(lower(string(e.ArgumentsSummary)), needle) { + return false + } + } + if q.From > 0 || q.To > 0 { + at := int64(0) + if t, err := time.Parse(time.RFC3339Nano, e.CreatedAt); err == nil { + at = t.UnixMilli() + } + if at <= 0 { + return false + } + if q.From > 0 && at < q.From { + return false + } + if q.To > 0 && at > q.To { + return false + } + } + return true +} + func (a Audit) List(ctx context.Context, q AuditQuery) (AuditList, error) { limit := q.Limit if limit < 1 { limit = 50 } - if limit > 1000 { - limit = 1000 + if limit > MaxAuditPage { + limit = MaxAuditPage } offset := q.Offset if offset < 0 { offset = 0 } - decision := q.Filter - if decision == "DENIED" { - decision = "DENY" - } - lower := strings.ToLower rows := []AuditEntry{} for _, l := range readLog() { e := toAuditEntry(l) - if decision != "" && e.Decision != decision { - continue - } - if q.Tool != "" && !strings.Contains(lower(e.ToolName), lower(q.Tool)) { - continue - } - if q.AgentName != "" { - name := "" - if e.AgentName != nil { - name = *e.AgentName - } - if !strings.Contains(lower(name), lower(q.AgentName)) { - continue - } - } - if q.SessionID != "" && (e.SessionID == nil || *e.SessionID != q.SessionID) { + if !q.Matches(e) { continue } - reason := "" - if e.Reason != nil { - reason = *e.Reason - } - if q.Signal == "dlp" && len(e.DLPMatches) == 0 && !strings.Contains(lower(reason), "dlp") { - continue - } - if q.Signal == "ratelimit" && !e.RateLimitBurst && !strings.Contains(lower(reason), "rate limit") { - continue - } - if q.Search != "" { - needle := lower(q.Search) - if !strings.Contains(lower(e.ToolName), needle) && - !strings.Contains(lower(reason), needle) && - !strings.Contains(lower(string(e.ArgumentsSummary)), needle) { - continue - } - } - if q.From > 0 || q.To > 0 { - at := l.at - if at <= 0 { - continue - } - if q.From > 0 && at < q.From { - continue - } - if q.To > 0 && at > q.To { - continue - } - } rows = append(rows, e) } diff --git a/packages/core/client/resources_audit_test.go b/packages/core/client/resources_audit_test.go index de8182d..d19f96e 100644 --- a/packages/core/client/resources_audit_test.go +++ b/packages/core/client/resources_audit_test.go @@ -288,4 +288,14 @@ func TestAuditWhitelistWithUnknownEntryErrors(t *testing.T) { if _, err := a.Whitelist(context.Background(), "999", "exact"); err == nil { t.Error("expected an error for an entry id that does not exist") } -} \ No newline at end of file +} +func TestAuditQueryMatchesIgnoresTheResultWindow(t *testing.T) { + q := AuditQuery{Tool: "bash", Limit: 1, Offset: 900} + entry := AuditEntry{ToolName: "Bash", Decision: "ALLOW", CreatedAt: "2026-01-01T00:00:00Z"} + if !q.Matches(entry) { + t.Error("Matches = false, want true: Limit and Offset window the result, they do not filter it") + } + if (AuditQuery{Tool: "bash"}).Matches(AuditEntry{ToolName: "Write"}) { + t.Error("Matches = true for a tool outside the filter, want false") + } +} diff --git a/packages/tui/audit/audit.go b/packages/tui/audit/audit.go index 684622f..023004e 100644 --- a/packages/tui/audit/audit.go +++ b/packages/tui/audit/audit.go @@ -22,6 +22,10 @@ func nextToken() int { tokens++; return tokens } const ( auditPage = 500 auditKeyColumn = 19 + // exportPageSize is the largest page Audit.List honours. Exports walk pages + // of this size instead of trusting one oversized request, which used to cut + // the file off at the same number of rows. + exportPageSize = client.MaxAuditPage ) var ( @@ -195,6 +199,13 @@ func (p *Audit) query() client.AuditQuery { } } +// filterActive reports whether the view shows only a subset of the log, i.e. +// whether "delete all" is meant to stop at the visible entries. +func (p *Audit) filterActive() bool { + return p.tool != "" || p.agent != "" || p.search != "" || + auditDecisions[p.di] != "" || auditSignals[p.gi] != "" +} + func (p *Audit) pages(total int) int { n := (total + auditPage - 1) / auditPage if n < 1 { diff --git a/packages/tui/audit/audit_keys.go b/packages/tui/audit/audit_keys.go index 3ca55f0..6d63431 100644 --- a/packages/tui/audit/audit_keys.go +++ b/packages/tui/audit/audit_keys.go @@ -16,6 +16,7 @@ import ( tea "github.com/charmbracelet/bubbletea" + "github.com/solongate/agent-security/packages/core/client" "github.com/solongate/agent-security/packages/core/config" "github.com/solongate/agent-security/packages/tui" ) @@ -192,7 +193,7 @@ func (p *Audit) editField(k tea.KeyMsg) tea.Cmd { func (p *Audit) doDelete(kind string, cur logRow) tea.Cmd { p.note = &auditNote{text: "deleting…", level: "ok"} - gen := p.gen + gen, q, filtered := p.gen, p.query(), p.filterActive() return func() tea.Msg { fail := func(err error) tea.Msg { return auditNoteResult{gen: gen, note: auditNote{text: "✗ " + err.Error(), level: "bad"}} @@ -202,14 +203,26 @@ func (p *Audit) doDelete(kind string, cur logRow) tea.Cmd { if kind == "one" { n = tui.DeleteLocalEntry(cur.at, cur.tool, cur.session) } else { - n = tui.ClearLocalLog() + // Delete exactly what the view shows: the same query that produced the + // rows is the filter, so a user who narrowed the log cannot lose + // entries that were filtered out of sight. + n = tui.DeleteLocalLogMatching(func(raw []byte) bool { + e, ok := client.AuditEntryFromLocalLine(raw) + return ok && q.Matches(e) + }) } if n == 0 { - return fail(errors.New("entry not found in the local file")) + if kind == "one" { + return fail(errors.New("entry not found in the local file")) + } + return fail(errors.New("no entry in the local file matched the current view")) } text := "✓ entry deleted" if kind != "one" { - text = "✓ the whole log on this machine was deleted" + text = "✓ deleted " + strconv.Itoa(n) + " entries matching the active filter" + if !filtered { + text = "✓ the whole log on this machine was deleted" + } } return auditNoteResult{gen: gen, note: auditNote{text: text, level: "ok"}, reload: true, toTop: true} @@ -226,18 +239,31 @@ func (p *Audit) doExport(kind string, pageRows []logRow) tea.Cmd { note: auditNote{text: "✗ export failed: " + err.Error(), level: "bad"}} } rows := pageRows + truncatedAt := 0 if kind == "all" { ctx, cancel := context.WithTimeout(context.Background(), 120*time.Second) defer cancel() - q.Limit, q.Offset = 10_000, 0 - list, err := client.Audit.List(ctx, q) - if err != nil { - return fail(err) - } - rows = make([]logRow, 0, len(list.Entries)) - for _, e := range list.Entries { - rows = append(rows, cloudRow(e)) + // client.Audit.List clamps Limit to 1000, so one request per export + // silently cut every export above 1000 rows. Walk the pages instead + // and only stop when the matching set is complete. + rows = rows[:0] + for offset := 0; ; offset += exportPageSize { + q.Limit, q.Offset = exportPageSize, offset + list, err := client.Audit.List(ctx, q) + if err != nil { + return fail(err) + } + for _, e := range list.Entries { + rows = append(rows, cloudRow(e)) + } + if len(rows) >= list.Total { + break + } + if len(list.Entries) < exportPageSize || ctx.Err() != nil { + truncatedAt = list.Total + break + } } sort.SliceStable(rows, func(i, j int) bool { return rows[i].at > rows[j].at }) } @@ -262,7 +288,13 @@ func (p *Audit) doExport(kind string, pageRows []logRow) tea.Cmd { } _ = os.Chmod(file, 0o600) - return auditNoteResult{gen: gen, note: auditNote{ - text: "✓ exported " + strconv.Itoa(len(rows)) + " rows → " + file, level: "ok"}} + text := "✓ exported " + strconv.Itoa(len(rows)) + " rows → " + file + level := "ok" + if truncatedAt > len(rows) { + text = "⚠ exported " + strconv.Itoa(len(rows)) + " of " + strconv.Itoa(truncatedAt) + + " matching rows (timed out) → " + file + level = "bad" + } + return auditNoteResult{gen: gen, note: auditNote{text: text, level: level}} } } \ No newline at end of file diff --git a/packages/tui/audit/audit_keys_test.go b/packages/tui/audit/audit_keys_test.go index 9f2d6f1..68feac7 100644 --- a/packages/tui/audit/audit_keys_test.go +++ b/packages/tui/audit/audit_keys_test.go @@ -5,6 +5,7 @@ package audit import ( "encoding/json" + "fmt" "os" "path/filepath" "strconv" @@ -529,4 +530,61 @@ func TestDoExportAllQueriesAndSortsNewestFirst(t *testing.T) { if first.Tool != "Newer" { t.Errorf("first exported row = %q, want the newest entry first", first.Tool) } -} \ No newline at end of file +} +func TestDoDeleteAllOnlyDeletesEntriesMatchingTheActiveFilter(t *testing.T) { + p := newTestAudit(t) + writeLogLines(t, + `{"ts":"2026-01-01T00:00:00Z","tool":"Bash","decision":"ALLOW","session_id":"s1"}`, + `{"ts":"2026-01-01T00:01:00Z","tool":"Write","decision":"DENY","session_id":"s2"}`, + `{"ts":"2026-01-01T00:02:00Z","tool":"Bash","decision":"DENY","session_id":"s3"}`, + ) + p.tool = "bash" // the view filters case-insensitively, and so must the delete + + res := p.doDelete("all", logRow{})().(auditNoteResult) + if !res.reload || !res.toTop { + t.Errorf("result = %+v, want reload and toTop set on success", res) + } + if !strings.HasPrefix(res.note.text, "✓") { + t.Errorf("note = %q, want a success mark", res.note.text) + } + if !strings.Contains(res.note.text, "2") { + t.Errorf("note = %q, want it to report the 2 deleted entries", res.note.text) + } + + b, err := os.ReadFile(config.LocalLogFile()) + if err != nil { + t.Fatal(err) + } + if strings.Contains(string(b), `"tool":"Bash"`) { + t.Errorf("log = %q, want every filtered Bash entry deleted", string(b)) + } + if !strings.Contains(string(b), `"tool":"Write"`) { + t.Errorf("log = %q, want the entry outside the filter kept", string(b)) + } +} + +func TestDoExportAllExportsEveryMatchingRowPastTheClientPageLimit(t *testing.T) { + p := newTestAudit(t) + const entries = 1105 // above client.Audit.List's 1000-row page cap + lines := make([]string, 0, entries) + for i := 0; i < entries; i++ { + lines = append(lines, fmt.Sprintf(`{"ts":%q,"tool":"Bash","decision":"ALLOW","session_id":"s%d"}`, + time.Unix(int64(1700000000+i), 0).UTC().Format(time.RFC3339), i)) + } + writeLogLines(t, lines...) + + res := p.doExport("all", nil)().(auditNoteResult) + if !strings.Contains(res.note.text, strconv.Itoa(entries)) { + t.Errorf("note = %q, want it to report all %d rows", res.note.text, entries) + } + + file := filepath.Join(config.Dir(), "audit-export.jsonl") + b, err := os.ReadFile(file) + if err != nil { + t.Fatalf("export file not written: %v", err) + } + got := len(strings.Split(strings.TrimRight(string(b), "\n"), "\n")) + if got != entries { + t.Fatalf("got %d exported lines, want %d (a silent 1000-row cap truncates the export)", got, entries) + } +} diff --git a/packages/tui/locallog.go b/packages/tui/locallog.go index 19d7b60..5458957 100644 --- a/packages/tui/locallog.go +++ b/packages/tui/locallog.go @@ -150,6 +150,47 @@ func DeleteLocalEntry(at int64, tool, session string) int { return removed } +// DeleteLocalLogMatching removes every local audit-log entry whose raw line the +// predicate accepts and reports how many lines it removed. The audit view passes +// the same filter it displays, so "delete all" cannot remove entries the user +// filtered out of the current view. A nil predicate deletes nothing: the caller +// must state what it is willing to destroy. Lines that do not parse are kept, so +// malformed rows survive instead of being dropped silently. +func DeleteLocalLogMatching(match func(raw []byte) bool) int { + if match == nil { + return 0 + } + file := config.LocalLogFile() + b, size, ok := readLogWithSize(file) + if !ok { + return 0 + } + removed := 0 + var kept []string + for _, line := range strings.Split(string(b), "\n") { + if strings.TrimSpace(line) == "" { + continue + } + var j LocalLogLine + if json.Unmarshal([]byte(line), &j) == nil && match([]byte(line)) { + removed++ + continue + } + kept = append(kept, line) + } + if removed == 0 { + return 0 + } + body := "" + if len(kept) > 0 { + body = strings.Join(kept, "\n") + "\n" + } + if !rewriteLog(file, size, []byte(body)) { + return 0 + } + return removed +} + func ClearLocalLog() int { file := config.LocalLogFile() b, size, ok := readLogWithSize(file) diff --git a/packages/tui/locallog_test.go b/packages/tui/locallog_test.go index c11483f..2622fbd 100644 --- a/packages/tui/locallog_test.go +++ b/packages/tui/locallog_test.go @@ -359,4 +359,52 @@ func TestLockAuditLogTimesOutOnAHeldLock(t *testing.T) { if ok { t.Error("lockAuditLog() against a live, held lock returned ok=true, want false") } -} \ No newline at end of file +} +func TestDeleteLocalLogMatchingRemovesOnlyAcceptedLines(t *testing.T) { + file := localLogHome(t) + writeLogLines(t, file, []string{ + `{"ts":"2024-01-01T00:00:00Z","tool":"bash"}`, + `{"ts":"2024-01-01T00:00:01Z","tool":"edit"}`, + `{not json at all`, + }) + + n := DeleteLocalLogMatching(func(raw []byte) bool { + return strings.Contains(string(raw), `"tool":"bash"`) + }) + if n != 1 { + t.Errorf("DeleteLocalLogMatching() = %d, want 1", n) + } + + b, err := os.ReadFile(file) + if err != nil { + t.Fatal(err) + } + if strings.Contains(string(b), `"tool":"bash"`) { + t.Errorf("file = %q, want the accepted line deleted", string(b)) + } + if !strings.Contains(string(b), `"tool":"edit"`) { + t.Errorf("file = %q, want the rejected line kept", string(b)) + } + if !strings.Contains(string(b), "{not json at all") { + t.Errorf("file = %q, want a line that does not parse kept", string(b)) + } +} + +func TestDeleteLocalLogMatchingNilPredicateDeletesNothing(t *testing.T) { + file := localLogHome(t) + writeLogLines(t, file, []string{ + `{"ts":"2024-01-01T00:00:00Z","tool":"bash"}`, + `{"ts":"2024-01-01T00:00:01Z","tool":"edit"}`, + }) + + if n := DeleteLocalLogMatching(nil); n != 0 { + t.Errorf("DeleteLocalLogMatching(nil) = %d, want 0", n) + } + b, err := os.ReadFile(file) + if err != nil { + t.Fatal(err) + } + if strings.Count(string(b), "\n") != 2 { + t.Errorf("file = %q, want both lines untouched", string(b)) + } +} From 1ef115cbbcc23275ce73a7eeba46030c3967e629 Mon Sep 17 00:00:00 2001 From: Turgay Nalbant <182372382+tnalbant@users.noreply.github.com> Date: Tue, 27 Oct 2026 16:54:59 +0300 Subject: [PATCH 2/2] Keep the unfiltered delete-all byte-for-byte as before Routing every delete through the query predicate also kept lines that no longer parse, so an unfiltered "X" left corrupted rows behind while still reporting that the whole log was deleted. With no filter active, keep the previous ClearLocalLog() path; the predicate is only for a narrowed view, where a line that cannot be parsed cannot match the filter anyway. Signed-off-by: Turgay Nalbant <182372382+tnalbant@users.noreply.github.com> --- packages/tui/audit/audit_keys.go | 10 ++++++++-- packages/tui/audit/audit_keys_test.go | 20 ++++++++++++++++++++ 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/packages/tui/audit/audit_keys.go b/packages/tui/audit/audit_keys.go index 6d63431..13fdcc0 100644 --- a/packages/tui/audit/audit_keys.go +++ b/packages/tui/audit/audit_keys.go @@ -200,9 +200,15 @@ func (p *Audit) doDelete(kind string, cur logRow) tea.Cmd { } n := 0 - if kind == "one" { + switch { + case kind == "one": n = tui.DeleteLocalEntry(cur.at, cur.tool, cur.session) - } else { + case !filtered: + // No filter is active, so "delete all" still means the whole local + // file — including lines that no longer parse. Keep that exact + // behaviour; filtering only narrows what the user asked to delete. + n = tui.ClearLocalLog() + default: // Delete exactly what the view shows: the same query that produced the // rows is the filter, so a user who narrowed the log cannot lose // entries that were filtered out of sight. diff --git a/packages/tui/audit/audit_keys_test.go b/packages/tui/audit/audit_keys_test.go index 68feac7..8fafc97 100644 --- a/packages/tui/audit/audit_keys_test.go +++ b/packages/tui/audit/audit_keys_test.go @@ -588,3 +588,23 @@ func TestDoExportAllExportsEveryMatchingRowPastTheClientPageLimit(t *testing.T) t.Fatalf("got %d exported lines, want %d (a silent 1000-row cap truncates the export)", got, entries) } } + +func TestDoDeleteAllWithoutFilterAlsoClearsUnparseableLines(t *testing.T) { + p := newTestAudit(t) + writeLogLines(t, + `{"ts":"2026-01-01T00:00:00Z","tool":"Bash","decision":"ALLOW"}`, + `{corrupted line`, + ) + + res := p.doDelete("all", logRow{})().(auditNoteResult) + if !strings.Contains(res.note.text, "whole log") { + t.Errorf("note = %q, want the unfiltered message", res.note.text) + } + b, err := os.ReadFile(config.LocalLogFile()) + if err != nil { + t.Fatal(err) + } + if strings.TrimSpace(string(b)) != "" { + t.Errorf("log = %q, want the whole file emptied, corrupted lines included", string(b)) + } +}