Skip to content

Commit 75f5d9d

Browse files
authored
perf: per-turn save cost, JSONL audit append, event group commit (#297)
* perf: cut per-turn save cost, JSONL audit append, event group commit - session: ordinary saves marshal once instead of twice; index.json served from an mtime+size-stamped cache with external-rewrite reload - audit: records append as JSONL instead of read-rewrite; legacy JSON migrates on first write; torn tails salvage, corrupt logs preserved - events: fsync group commit with flush interval; Close flushes pending - danger: homoglyph fold pass skipped when identical to normalized text; single-pass normalization - memory/extended: mtime-keyed atom-chunk cache for recall * fix: harden index cache stamp and audit corruption classifier (adversarial review findings) * fix: events group-commit lost-update on concurrent write; coverage tests * fix: align guard default timeout test with guard.DefaultConfig
1 parent e856cce commit 75f5d9d

21 files changed

Lines changed: 1558 additions & 99 deletions

‎docs/CLI.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ Unknown flags are a **hard error** — they are never folded into the task text
9999
| `--guard-long-url <url>` | string | — | Guard sidecar long-text endpoint |
100100
| `--guard-socket-path <path>` | string | — | Guard sidecar Unix socket path |
101101
| `--guard-threshold <score>` | float | `0.9` | Injection score threshold |
102-
| `--guard-timeout <seconds>` | int | `5` | Guard sidecar request timeout |
102+
| `--guard-timeout <seconds>` | int | `1` | Guard sidecar request timeout |
103103
| `--guard-fallback` / `--guard-no-fallback` | bool | `true` | Fall back to local scan if sidecar fails |
104104
| `--guard-scan-memory` / `--guard-no-scan-memory` | bool | `true` | Guard legacy/Extended Memory surfaces |
105105
| `--guard-scan-system-prompt` / `--guard-no-scan-system-prompt` | bool | `true` | Guard system-prompt sources |

‎docs/SECURITY.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -482,7 +482,7 @@ Schedule persistence is hardened against local tampering: state files (`schedule
482482

483483
### Runtime event stream hygiene
484484

485-
The structured event stream (`internal/events`, schema `odek.event/v1`, `odek run --events-jsonl`) is observability data that may leave the machine, so it is redacted by construction: tool arguments are never logged raw by default — a SHA-256 digest (`args_sha256`), byte sizes, and a structured `args_summary` (program name, target path or URL host, danger class — never argument content) — raw error text is collapsed into low-cardinality `error_class` strings, and the emitter runs `internal/redact` over the tool name and every string `data` value before dispatch. Every tool-call event carries a stable `call_id` shared by its started/completed/failed pair, so batched parallel calls can be correlated by consumers without positional guessing. For incident review where the session may already be deleted, `--events-include-args` (`Config.EventsIncludeArgs`) opts the stream into raw — still secret-redacted — arguments. The JSONL sink creates/hardens the file `0600`, refuses a symlink at the target path, requires the parent directory to exist, and fsyncs every event. Dispatch is non-blocking (buffered, drop-on-full) and panic-isolated, so a hostile or broken consumer cannot stall the loop or use backpressure as a DoS.
485+
The structured event stream (`internal/events`, schema `odek.event/v1`, `odek run --events-jsonl`) is observability data that may leave the machine, so it is redacted by construction: tool arguments are never logged raw by default — a SHA-256 digest (`args_sha256`), byte sizes, and a structured `args_summary` (program name, target path or URL host, danger class — never argument content) — raw error text is collapsed into low-cardinality `error_class` strings, and the emitter runs `internal/redact` over the tool name and every string `data` value before dispatch. Every tool-call event carries a stable `call_id` shared by its started/completed/failed pair, so batched parallel calls can be correlated by consumers without positional guessing. For incident review where the session may already be deleted, `--events-include-args` (`Config.EventsIncludeArgs`) opts the stream into raw — still secret-redacted — arguments. The JSONL sink creates/hardens the file `0600`, refuses a symlink at the target path, requires the parent directory to exist, and writes every event to the file before `Write` returns; fsync is batched by a group-commit flusher (default 50 ms interval, plus one final sync on `Close`, with a synchronous `Flush` barrier available), trading at most one flush interval of durability on an OS crash for per-event throughput — a deliberate observability-data trade-off, since the stream is secret-redacted by construction. Dispatch is non-blocking (buffered, drop-on-full) and panic-isolated, so a hostile or broken consumer cannot stall the loop or use backpressure as a DoS.
486486

487487
### Atomic writes and file permissions
488488

‎internal/config/loader_test_guard_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,8 @@ func TestLoadConfig_GuardDefaults(t *testing.T) {
1818
if cfg.Guard.Threshold != 0.9 {
1919
t.Errorf("Guard.Threshold = %v, want 0.9", cfg.Guard.Threshold)
2020
}
21-
if cfg.Guard.TimeoutSeconds != 5 {
22-
t.Errorf("Guard.TimeoutSeconds = %d, want 5", cfg.Guard.TimeoutSeconds)
21+
if cfg.Guard.TimeoutSeconds != guard.DefaultConfig().TimeoutSeconds {
22+
t.Errorf("Guard.TimeoutSeconds = %d, want %d (guard.DefaultConfig)", cfg.Guard.TimeoutSeconds, guard.DefaultConfig().TimeoutSeconds)
2323
}
2424
if cfg.Guard.FallbackToLocal == nil || !*cfg.Guard.FallbackToLocal {
2525
t.Error("Guard.FallbackToLocal should default to true")

‎internal/danger/injection.go‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,9 @@ type ScanResult struct {
119119
// ScanInjection checks content for prompt injection attempts.
120120
// Returns nil if no threats detected, or a list of found threats.
121121
// Each threat includes a label describing what was found.
122+
// Folding is skipped when the homoglyph-folded text equals the normalized
123+
// text: the second regex pass over identical input cannot produce a
124+
// different result.
122125
func ScanInjection(content string) []ScanResult {
123126
if content == "" {
124127
return nil
@@ -139,11 +142,13 @@ func ScanInjection(content string) []ScanResult {
139142
// Pattern matching runs on normalized text so blacklists are resilient
140143
// to case, whitespace, and zero-width characters. We also scan a
141144
// homoglyph-folded version so mixed-script attacks that look like ASCII
142-
// are still caught.
145+
// are still caught. Folding is skipped entirely when the normalized
146+
// text contains no foldable characters (the common case).
143147
normalized := NormalizeForScan(content)
144148
folded := FoldHomoglyphs(normalized)
149+
foldDistinct := folded != normalized
145150
for _, p := range injectionPatterns {
146-
if p.Re.MatchString(normalized) || p.Re.MatchString(folded) {
151+
if p.Re.MatchString(normalized) || (foldDistinct && p.Re.MatchString(folded)) {
147152
results = append(results, ScanResult{
148153
Label: p.Label,
149154
Pattern: p.Re.String(),

‎internal/danger/normalize.go‎

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -146,21 +146,32 @@ func ContainsInvisible(s string) bool {
146146
// render invisibly inside a word while breaking contiguous-pattern matching —
147147
// "instructio\u0301ns" must scan like "instructions".
148148
func NormalizeForScan(text string) string {
149+
// Single pass: drop invisible/combining characters and collapse
150+
// whitespace runs to single spaces while building, so the old
151+
// build → ToLower → Fields+Join copy chain becomes one build plus
152+
// one ToLower copy (ToLower over the string keeps the exact original
153+
// case-folding semantics).
149154
var b strings.Builder
150155
b.Grow(len(text))
156+
prevSpace := true // leading whitespace is dropped
151157
for _, r := range text {
152158
if isInvisible(r) {
153159
continue
154160
}
155161
if unicode.Is(unicode.Mn, r) || unicode.Is(unicode.Me, r) || unicode.Is(unicode.Mc, r) {
156162
continue
157163
}
164+
if unicode.IsSpace(r) {
165+
if !prevSpace {
166+
b.WriteByte(' ')
167+
prevSpace = true
168+
}
169+
continue
170+
}
158171
b.WriteRune(r)
172+
prevSpace = false
159173
}
160-
normalized := strings.ToLower(b.String())
161-
// Collapse all whitespace to a single space so patterns do not have to
162-
// account for arbitrary runs of spaces, tabs, or newlines.
163-
return strings.Join(strings.Fields(normalized), " ")
174+
return strings.ToLower(strings.TrimSuffix(b.String(), " "))
164175
}
165176

166177
// FoldHomoglyphs returns text with common Unicode confusables (Cyrillic/Greek

‎internal/events/jsonl.go‎

Lines changed: 128 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -10,16 +10,37 @@ import (
1010
"time"
1111
)
1212

13+
// defaultFlushInterval bounds how long an acknowledged event may remain
14+
// un-fsynced. Durability trade-off: with group commit a crash can lose up
15+
// to ~defaultFlushInterval of the most recent events (the data is in the
16+
// page cache and visible to readers; only an OS-level crash can lose it).
17+
// This replaces one fsync per event, which capped throughput at ~1/fsync
18+
// and made bursty event storms drop events when the dispatch buffer filled.
19+
const defaultFlushInterval = 50 * time.Millisecond
20+
1321
// JSONLSink is an append-only sink that writes one JSON object per line.
1422
//
1523
// Safety properties:
1624
// - the parent directory must already exist (the sink never creates it)
1725
// - an existing symlink at the target path is refused
1826
// - the file is created (and hardened) with 0600 permissions
19-
// - every event is flushed to stable storage before Write returns
27+
// - every event is written to the file before Write returns
28+
// - durability is batched: fsync runs on a background flush timer
29+
// (defaultFlushInterval) and once more on Close, so a process crash
30+
// loses at most one flush interval of events; an OS crash can lose
31+
// the same window. Call Flush for a synchronous barrier.
2032
type JSONLSink struct {
21-
mu sync.Mutex
22-
f *os.File
33+
mu sync.Mutex
34+
f *os.File
35+
syncFn func() error // overridable in tests; defaults to f.Sync
36+
flushInterval time.Duration
37+
38+
dirty bool
39+
writeGen uint64
40+
syncing bool
41+
closed bool
42+
flushCh chan struct{} // closed by Close to stop the flusher
43+
flushDone chan struct{} // closed by the flusher goroutine on exit
2344
}
2445

2546
// OpenJSONLSink opens path for append-only event writes, creating it with
@@ -53,11 +74,12 @@ func OpenJSONLSink(path string) (*JSONLSink, error) {
5374
f.Close()
5475
return nil, err
5576
}
56-
return &JSONLSink{f: f}, nil
77+
return &JSONLSink{f: f, syncFn: f.Sync, flushInterval: defaultFlushInterval}, nil
5778
}
5879

59-
// Write appends one event as a single JSON line and flushes it to stable
60-
// storage. Safe for concurrent use.
80+
// Write appends one event as a single JSON line. The line is written to the
81+
// file before Write returns; fsync is batched by the background flusher —
82+
// see the durability note on JSONLSink. Safe for concurrent use.
6183
func (s *JSONLSink) Write(ev Event) error {
6284
if ev.Schema == "" {
6385
ev.Schema = Schema
@@ -73,15 +95,112 @@ func (s *JSONLSink) Write(ev Event) error {
7395

7496
s.mu.Lock()
7597
defer s.mu.Unlock()
98+
if s.closed {
99+
return os.ErrClosed
100+
}
76101
if _, err := s.f.Write(line); err != nil {
77102
return err
78103
}
79-
return s.f.Sync()
104+
s.dirty = true
105+
s.writeGen++
106+
s.ensureFlusherLocked()
107+
return nil
108+
}
109+
110+
// ensureFlusherLocked lazily starts the background flush timer on the first
111+
// write. Caller must hold s.mu.
112+
func (s *JSONLSink) ensureFlusherLocked() {
113+
if s.flushCh != nil {
114+
return
115+
}
116+
s.flushCh = make(chan struct{})
117+
s.flushDone = make(chan struct{})
118+
interval := s.flushInterval
119+
if interval <= 0 {
120+
interval = defaultFlushInterval
121+
}
122+
go func() {
123+
defer close(s.flushDone)
124+
ticker := time.NewTicker(interval)
125+
defer ticker.Stop()
126+
for {
127+
select {
128+
case <-ticker.C:
129+
s.syncOnce()
130+
case <-s.flushCh:
131+
return
132+
}
133+
}
134+
}()
80135
}
81136

82-
// Close flushes and closes the underlying file.
137+
// syncOnce flushes the kernel page-cache state of the file if a write is
138+
// pending and no sync is already in flight. Writes that land while the sync
139+
// runs re-mark the sink dirty, so the next tick commits them.
140+
func (s *JSONLSink) syncOnce() {
141+
s.mu.Lock()
142+
if !s.dirty || s.syncing || s.closed {
143+
s.mu.Unlock()
144+
return
145+
}
146+
s.syncing = true
147+
fn := s.syncFn
148+
gen := s.writeGen
149+
s.mu.Unlock()
150+
151+
err := fn()
152+
153+
s.mu.Lock()
154+
s.syncing = false
155+
// Clear dirty only when no write landed while the sync was in flight:
156+
// a concurrent Write re-marks dirty and bumps writeGen, and its data
157+
// is not covered by this fsync — the next tick must commit it.
158+
if err == nil && s.writeGen == gen {
159+
s.dirty = false
160+
}
161+
s.mu.Unlock()
162+
}
163+
164+
// Flush synchronously fsyncs any pending writes. Use when a durability
165+
// barrier is required before continuing.
166+
func (s *JSONLSink) Flush() error {
167+
s.mu.Lock()
168+
if s.closed {
169+
s.mu.Unlock()
170+
return os.ErrClosed
171+
}
172+
fn := s.syncFn
173+
s.mu.Unlock()
174+
return fn()
175+
}
176+
177+
// Close stops the flusher, flushes any pending writes, and closes the
178+
// underlying file.
83179
func (s *JSONLSink) Close() error {
180+
s.mu.Lock()
181+
if s.closed {
182+
s.mu.Unlock()
183+
return nil
184+
}
185+
s.closed = true
186+
flushCh := s.flushCh
187+
fn := s.syncFn
188+
dirty := s.dirty
189+
s.mu.Unlock()
190+
191+
if flushCh != nil {
192+
close(flushCh)
193+
<-s.flushDone
194+
}
195+
var syncErr error
196+
if dirty && fn != nil {
197+
syncErr = fn()
198+
}
199+
84200
s.mu.Lock()
85201
defer s.mu.Unlock()
86-
return s.f.Close()
202+
if err := s.f.Close(); err != nil {
203+
return err
204+
}
205+
return syncErr
87206
}
Lines changed: 134 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,134 @@
1+
package events
2+
3+
// Coverage tests for the group-commit flusher branches: the syncOnce
4+
// no-op paths (not dirty / already syncing / closed), error retention on
5+
// failed sync, the default flush interval fallback, and Flush/Close on
6+
// a closed sink.
7+
8+
import (
9+
"errors"
10+
"os"
11+
"sync/atomic"
12+
"testing"
13+
"time"
14+
)
15+
16+
// syncOnce with nothing pending must not invoke the sync function.
17+
func TestJSONLSinkSyncOnce_NotDirtyIsNoop(t *testing.T) {
18+
s, _, syncs := newCountingSink(t)
19+
s.flushInterval = time.Hour
20+
s.syncOnce()
21+
if got := atomic.LoadInt64(syncs); got != 0 {
22+
t.Fatalf("syncs = %d for a clean sink, want 0", got)
23+
}
24+
}
25+
26+
// A sync error must keep the sink dirty so the pending writes are
27+
// retried on the next tick instead of being silently declared durable.
28+
func TestJSONLSinkSyncOnce_ErrorKeepsDirty(t *testing.T) {
29+
s, _, syncs := newCountingSink(t)
30+
s.flushInterval = time.Hour
31+
32+
var fail int32
33+
s.syncFn = func() error {
34+
atomic.AddInt64(syncs, 1)
35+
if atomic.LoadInt32(&fail) == 1 {
36+
return errors.New("simulated EIO")
37+
}
38+
return nil
39+
}
40+
41+
atomic.StoreInt32(&fail, 1)
42+
if err := s.Write(ev(1)); err != nil {
43+
t.Fatalf("Write: %v", err)
44+
}
45+
s.syncOnce() // fails, must stay dirty
46+
s.syncOnce() // fails again, must still stay dirty
47+
48+
atomic.StoreInt32(&fail, 0)
49+
s.syncOnce() // succeeds, clears dirty
50+
if got := atomic.LoadInt64(syncs); got != 3 {
51+
t.Fatalf("syncs = %d, want 3 (two retries then success)", got)
52+
}
53+
}
54+
55+
// Flush after Close must return os.ErrClosed, not panic or fsync a dead
56+
// file handle.
57+
func TestJSONLSinkFlush_AfterCloseReturnsErrClosed(t *testing.T) {
58+
s, _, _ := newCountingSink(t)
59+
if err := s.Close(); err != nil {
60+
t.Fatalf("Close: %v", err)
61+
}
62+
if err := s.Flush(); !errors.Is(err, os.ErrClosed) {
63+
t.Fatalf("Flush after Close = %v, want os.ErrClosed", err)
64+
}
65+
if err := s.Write(ev(1)); !errors.Is(err, os.ErrClosed) {
66+
t.Fatalf("Write after Close = %v, want os.ErrClosed", err)
67+
}
68+
// Double Close is idempotent.
69+
if err := s.Close(); err != nil {
70+
t.Fatalf("second Close = %v, want nil", err)
71+
}
72+
}
73+
74+
// A zero flushInterval must fall back to the default rather than
75+
// spinning a zero-period ticker.
76+
func TestJSONLSink_ZeroIntervalUsesDefault(t *testing.T) {
77+
s, _, _ := newCountingSink(t)
78+
s.flushInterval = 0
79+
if err := s.Write(ev(1)); err != nil {
80+
t.Fatalf("Write: %v", err)
81+
}
82+
// The flusher must start lazily with the default interval; just
83+
// verify it stops cleanly.
84+
if s.flushCh == nil {
85+
t.Fatal("flusher did not start after first write")
86+
}
87+
if err := s.Close(); err != nil {
88+
t.Fatalf("Close: %v", err)
89+
}
90+
}
91+
92+
// Writes landing while a sync is in flight must re-mark the sink dirty
93+
// so the next tick commits them (syncing-guard branch).
94+
func TestJSONLSink_WriteDuringSyncStaysDirty(t *testing.T) {
95+
s, path, _ := newCountingSink(t)
96+
s.flushInterval = time.Hour
97+
98+
release := make(chan struct{})
99+
s.syncFn = func() error {
100+
<-release
101+
return nil
102+
}
103+
if err := s.Write(ev(1)); err != nil {
104+
t.Fatalf("Write 1: %v", err)
105+
}
106+
done := make(chan struct{})
107+
go func() {
108+
defer close(done)
109+
s.syncOnce() // blocks inside syncFn with syncing=true
110+
}()
111+
// Wait until the sync is in flight, then write behind it.
112+
time.Sleep(20 * time.Millisecond)
113+
if err := s.Write(ev(2)); err != nil {
114+
t.Fatalf("Write 2: %v", err)
115+
}
116+
close(release)
117+
<-done
118+
119+
s.mu.Lock()
120+
dirty := s.dirty
121+
s.mu.Unlock()
122+
if !dirty {
123+
t.Fatal("write during in-flight sync did not re-mark the sink dirty")
124+
}
125+
s.mu.Lock()
126+
s.dirty = false // clean shutdown without invoking the gated syncFn
127+
s.mu.Unlock()
128+
if err := s.Close(); err != nil {
129+
t.Fatalf("Close: %v", err)
130+
}
131+
if _, err := os.Stat(path); err != nil {
132+
t.Fatalf("sink file missing: %v", err)
133+
}
134+
}

0 commit comments

Comments
 (0)