Skip to content

Commit 490b0cd

Browse files
Improve tome automation scheduling window (#1527)
* Improve tome automation scheduling window Modified tome automation logic to queue tasks if their schedule falls within the upcoming beacon check-in interval window (now to now + interval), rather than just checking the current minute. This prevents beacons with long sleep intervals from missing scheduled tasks. Added `TestHandleTomeAutomation_IntervalWindow` to verify the fix and prevent regressions. Also fixed a missing sqlite3 driver import in `tome_automation_test.go`. * Refactor tome automation to use time.Duration Updated `handleTomeAutomation` signature to accept `time.Duration` for interval instead of `int`, addressing PR feedback. Updated call sites and tests to pass duration. * Refactor tome automation for lookahead and range support Updated `handleTomeAutomation` to support lookahead scheduling for standard cron expressions (queueing tasks scheduled within the upcoming beacon interval) while enforcing strict current-time matching for range-based schedules (e.g., `* 6-12 * * *`). Refactored method to accept `time.Duration` for interval. Added comprehensive tests for interval window logic and cron range behavior. Fixed missing sqlite3 driver in tests. --------- Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com>
1 parent ee68dea commit 490b0cd

File tree

2 files changed

+118
-9
lines changed

2 files changed

+118
-9
lines changed

tavern/internal/c2/api_claim_tasks.go

Lines changed: 24 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ func init() {
4444
prometheus.MustRegister(metricTomeAutomationErrors)
4545
}
4646

47-
func (srv *Server) handleTomeAutomation(ctx context.Context, beaconID int, hostID int, isNewBeacon bool, isNewHost bool, now time.Time) {
47+
func (srv *Server) handleTomeAutomation(ctx context.Context, beaconID int, hostID int, isNewBeacon bool, isNewHost bool, now time.Time, interval time.Duration) {
4848
// Tome Automation Logic
4949
candidateTomes, err := srv.graph.Tome.Query().
5050
Where(tome.Or(
@@ -62,7 +62,7 @@ func (srv *Server) handleTomeAutomation(ctx context.Context, beaconID int, hostI
6262

6363
selectedTomes := make(map[int]*ent.Tome)
6464
parser := cron.NewParser(cron.Minute | cron.Hour | cron.Dom | cron.Month | cron.Dow)
65-
currentMinute := now.Truncate(time.Minute)
65+
cutoff := now.Add(interval)
6666

6767
for _, t := range candidateTomes {
6868
shouldRun := false
@@ -81,10 +81,27 @@ func (srv *Server) handleTomeAutomation(ctx context.Context, beaconID int, hostI
8181
if !shouldRun && t.RunOnSchedule != "" {
8282
sched, err := parser.Parse(t.RunOnSchedule)
8383
if err == nil {
84-
// Check if schedule matches current time
85-
// Next(now-1sec) == now?
86-
next := sched.Next(currentMinute.Add(-1 * time.Second))
87-
if next.Equal(currentMinute) {
84+
isMatch := false
85+
// If schedule contains a range (hyphen), checking for strict current match
86+
// without factoring in callback interval.
87+
if strings.Contains(t.RunOnSchedule, "-") {
88+
currentMinute := now.Truncate(time.Minute)
89+
// Verify if 'currentMinute' is a valid trigger time.
90+
// We check if the next trigger after (currentMinute - 1s) is exactly currentMinute.
91+
next := sched.Next(currentMinute.Add(-1 * time.Second))
92+
if next.Equal(currentMinute) {
93+
isMatch = true
94+
}
95+
} else {
96+
// Check if any point between now and the next expected check-in matches the run schedule
97+
// Next(now-1sec) <= now + interval?
98+
next := sched.Next(now.Add(-1 * time.Second))
99+
if !next.After(cutoff) {
100+
isMatch = true
101+
}
102+
}
103+
104+
if isMatch {
88105
// Check scheduled_hosts constraint
89106
hostCount, err := t.QueryScheduledHosts().Count(ctx)
90107
if err != nil {
@@ -294,7 +311,7 @@ func (srv *Server) ClaimTasks(ctx context.Context, req *c2pb.ClaimTasksRequest)
294311
}
295312

296313
// Run Tome Automation (non-blocking, best effort)
297-
srv.handleTomeAutomation(ctx, beaconID, hostID, isNewBeacon, isNewHost, now)
314+
srv.handleTomeAutomation(ctx, beaconID, hostID, isNewBeacon, isNewHost, now, time.Duration(req.GetBeacon().GetActiveTransport().GetInterval())*time.Second)
298315

299316
// Load Tasks
300317
tasks, err := srv.graph.Task.Query().

tavern/internal/c2/tome_automation_test.go

Lines changed: 94 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"time"
77

88
"github.com/stretchr/testify/assert"
9+
_ "github.com/mattn/go-sqlite3"
910
"realm.pub/tavern/internal/c2/c2pb"
1011
"realm.pub/tavern/internal/ent"
1112
"realm.pub/tavern/internal/ent/enttest"
@@ -138,7 +139,9 @@ func TestHandleTomeAutomation(t *testing.T) {
138139
client.Task.Delete().ExecX(ctx)
139140
client.Quest.Delete().ExecX(ctx)
140141

141-
srv.handleTomeAutomation(ctx, b.ID, h.ID, tt.isNewBeacon, tt.isNewHost, now)
142+
// Use 0 interval to match "current minute" logic (cutoff = now)
143+
// effectively checking sched.Next(now-1s) <= now
144+
srv.handleTomeAutomation(ctx, b.ID, h.ID, tt.isNewBeacon, tt.isNewHost, now, 0)
142145

143146
// Verify Tasks
144147
tasks := client.Task.Query().WithQuest(func(q *ent.QuestQuery) {
@@ -185,9 +188,98 @@ func TestHandleTomeAutomation_Deduplication(t *testing.T) {
185188
SaveX(ctx)
186189

187190
// Trigger all conditions
188-
srv.handleTomeAutomation(ctx, b.ID, h.ID, true, true, now)
191+
srv.handleTomeAutomation(ctx, b.ID, h.ID, true, true, now, 0)
189192

190193
// Should only have 1 task
191194
count := client.Task.Query().CountX(ctx)
192195
assert.Equal(t, 1, count, "Should only create one task despite multiple triggers matching")
193196
}
197+
198+
func TestHandleTomeAutomation_IntervalWindow(t *testing.T) {
199+
ctx := context.Background()
200+
client := enttest.Open(t, "sqlite3", "file:ent?mode=memory&cache=shared&_fk=1")
201+
defer client.Close()
202+
203+
srv := &Server{graph: client}
204+
// Date: 3:00:56 PM
205+
now := time.Date(2023, 10, 27, 15, 0, 56, 0, time.UTC)
206+
207+
h := client.Host.Create().
208+
SetIdentifier("test-host").
209+
SetPlatform(c2pb.Host_PLATFORM_LINUX).
210+
SaveX(ctx)
211+
b := client.Beacon.Create().
212+
SetIdentifier("test-beacon").
213+
SetHost(h).
214+
SetTransport(c2pb.ActiveTransport_TRANSPORT_HTTP1).
215+
SaveX(ctx)
216+
217+
// Schedule: 3:01:00 PM -> "1 15 * * *"
218+
client.Tome.Create().
219+
SetName("Scheduled Tome").
220+
SetDescription("Test").
221+
SetAuthor("Test Author").
222+
SetEldritch("print('schedule')").
223+
SetRunOnSchedule("1 15 * * *").
224+
SaveX(ctx)
225+
226+
// 1. First Check-in at 3:00:56 PM. Interval 120s.
227+
// Window: [3:00:56, 3:02:56].
228+
// Schedule 3:01:00 is in window.
229+
srv.handleTomeAutomation(ctx, b.ID, h.ID, false, false, now, 120*time.Second)
230+
231+
count := client.Task.Query().CountX(ctx)
232+
assert.Equal(t, 1, count, "Tome should be queued at 3:00:56 for 3:01:00 schedule")
233+
234+
// Clear tasks
235+
client.Task.Delete().ExecX(ctx)
236+
client.Quest.Delete().ExecX(ctx)
237+
238+
// 2. Second Check-in at 3:02:56 PM. Interval 120s.
239+
// Next checkin: 3:04:56.
240+
// Schedule 3:01:00 (tomorrow) is NOT in window.
241+
nextCheckin := now.Add(2 * time.Minute) // 3:02:56
242+
srv.handleTomeAutomation(ctx, b.ID, h.ID, false, false, nextCheckin, 120*time.Second)
243+
244+
count = client.Task.Query().CountX(ctx)
245+
assert.Equal(t, 0, count, "Tome should NOT be queued at 3:02:56")
246+
}
247+
248+
func TestHandleTomeAutomation_CronRange(t *testing.T) {
249+
ctx := context.Background()
250+
client := enttest.Open(t, "sqlite3", "file:ent?mode=memory&cache=shared&_fk=1")
251+
defer client.Close()
252+
253+
srv := &Server{graph: client}
254+
// 06:05:00
255+
now := time.Date(2023, 10, 27, 6, 5, 0, 0, time.UTC)
256+
257+
h := client.Host.Create().SetIdentifier("h").SetPlatform(c2pb.Host_PLATFORM_LINUX).SaveX(ctx)
258+
b := client.Beacon.Create().SetIdentifier("b").SetHost(h).SetTransport(c2pb.ActiveTransport_TRANSPORT_HTTP1).SaveX(ctx)
259+
260+
// Range Schedule: "* 6-12 * * *" (Every minute of hours 6-12)
261+
client.Tome.Create().
262+
SetName("Range Tome").
263+
SetDescription("Test").
264+
SetAuthor("Test").
265+
SetEldritch("print('range')").
266+
SetRunOnSchedule("* 6-12 * * *").
267+
SaveX(ctx)
268+
269+
// 1. Current time 06:05:00. Matches range. Should run.
270+
// Interval irrelevant (using 1h to prove lookahead is ignored if range logic applies)
271+
srv.handleTomeAutomation(ctx, b.ID, h.ID, false, false, now, 1*time.Hour)
272+
count := client.Task.Query().CountX(ctx)
273+
assert.Equal(t, 1, count, "Range tome should run at 06:05:00")
274+
275+
client.Task.Delete().ExecX(ctx)
276+
client.Quest.Delete().ExecX(ctx)
277+
278+
// 2. Current time 05:55:00. Does NOT match range.
279+
// Lookahead (1h) would see 06:00:00.
280+
// But Range logic should IGNORE lookahead.
281+
early := now.Add(-10 * time.Minute) // 05:55:00
282+
srv.handleTomeAutomation(ctx, b.ID, h.ID, false, false, early, 1*time.Hour)
283+
count = client.Task.Query().CountX(ctx)
284+
assert.Equal(t, 0, count, "Range tome should NOT run at 05:55:00 even with lookahead")
285+
}

0 commit comments

Comments
 (0)