From 70426ee4c0bbc5bae3040400fe5948c0dc2fd652 Mon Sep 17 00:00:00 2001 From: Astrid Gealer Date: Fri, 23 Oct 2026 02:36:56 +0000 Subject: [PATCH] fix(config): compare resolve checked_at at the database's microsecond precision checkRIExchangeCloudCheck and validateLadderUnverifiedFail refuse a cloud check that does not postdate the row's last change, but compared the operator's nanosecond checked_at against an updated_at that Postgres had truncated to microseconds. A check stamped at the change's own instant read as a few hundred nanoseconds after it and was accepted, which is what TestPostgresStoreDB_ResolveStaleRIExchange_ValidatesCloudCheck catches now that #1165 runs it against the shared container. Truncate checked_at to microseconds before the "after the last change" comparison in both paths (checkPostdatesDBTime), and add unit tests that pin the boundary without a database. Closes #1251 Co-Authored-By: Claude Opus 5.5 --- .../config/store_postgres_ladder_resolve.go | 2 +- .../store_postgres_ladder_resolve_test.go | 20 ++++++++++ .../store_postgres_ri_exchange_resolve.go | 11 +++++- ...store_postgres_ri_exchange_resolve_test.go | 39 +++++++++++++++++++ 4 files changed, 70 insertions(+), 2 deletions(-) create mode 100644 internal/config/store_postgres_ri_exchange_resolve_test.go diff --git a/internal/config/store_postgres_ladder_resolve.go b/internal/config/store_postgres_ladder_resolve.go index aea265e56..187e3878d 100644 --- a/internal/config/store_postgres_ladder_resolve.go +++ b/internal/config/store_postgres_ladder_resolve.go @@ -198,7 +198,7 @@ func validateLadderUnverifiedFail(req LadderResolveFailed, layer string, account return fmt.Errorf("%w: checked_service must be %q", ErrLadderResolveInvalid, layer) case c.CheckedAt.After(req.Now): return fmt.Errorf("%w: checked_at is in the future", ErrLadderResolveInvalid) - case !c.CheckedAt.After(lastChange): + case !checkPostdatesDBTime(c.CheckedAt, lastChange): return fmt.Errorf("%w: checked_at must be after the last execution change at %s", ErrLadderResolveInvalid, lastChange.UTC().Format(time.RFC3339)) } return nil diff --git a/internal/config/store_postgres_ladder_resolve_test.go b/internal/config/store_postgres_ladder_resolve_test.go index ba4c4287d..78bf31bae 100644 --- a/internal/config/store_postgres_ladder_resolve_test.go +++ b/internal/config/store_postgres_ladder_resolve_test.go @@ -2,8 +2,10 @@ package config import ( "testing" + "time" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) // The chain-node allowlist fails closed: a live status or one added later is @@ -16,3 +18,21 @@ func TestLadderResolvableNodeStatus(t *testing.T) { assert.False(t, ladderResolvableNodeStatus(s), s) } } + +// lastChange comes back from Postgres truncated to microseconds; a check +// stamped with the change's own instant must not count as after it. +func TestValidateLadderUnverifiedFail_CheckedAtTheChangeIsRefused(t *testing.T) { + change := time.Date(2026, 10, 23, 1, 2, 3, 456789123, time.UTC) + req := LadderResolveFailed{ + Acknowledge: true, + Check: &LadderCloudCheck{AccountID: "acct-1", Region: "us-east-1", Service: "ec2", CheckedAt: change}, + Now: change.Add(time.Hour), + StaleAfter: time.Minute, + } + lastChange := change.Truncate(time.Microsecond) + + require.ErrorIs(t, validateLadderUnverifiedFail(req, "ec2", []string{"acct-1"}, lastChange), ErrLadderResolveInvalid) + + req.Check.CheckedAt = change.Add(time.Minute) + require.NoError(t, validateLadderUnverifiedFail(req, "ec2", []string{"acct-1"}, lastChange)) +} diff --git a/internal/config/store_postgres_ri_exchange_resolve.go b/internal/config/store_postgres_ri_exchange_resolve.go index b1469c1cf..ae6c45564 100644 --- a/internal/config/store_postgres_ri_exchange_resolve.go +++ b/internal/config/store_postgres_ri_exchange_resolve.go @@ -184,13 +184,22 @@ func checkRIExchangeCloudCheck(c RIExchangeCloudCheck, row riExchangeResolveRow) return fmt.Errorf("%w: checked_region is not the exchange's region", ErrRIExchangeResolveInvalid) case c.CheckedAt.After(row.dbNow): return fmt.Errorf("%w: checked_at is in the future", ErrRIExchangeResolveInvalid) - case !c.CheckedAt.After(row.updatedAt): + case !checkPostdatesDBTime(c.CheckedAt, row.updatedAt): return fmt.Errorf("%w: checked_at must be after the exchange's last change at %s", ErrRIExchangeResolveInvalid, row.updatedAt.UTC().Format(time.RFC3339)) } return nil } +// checkPostdatesDBTime reports whether an operator's check time is strictly +// after last, a timestamp read back from Postgres. Postgres keeps microseconds, +// so the check is compared at that precision too: a check taken within the +// same microsecond as the change cannot be ordered after it, and comparing the +// raw nanoseconds would let it through. +func checkPostdatesDBTime(checked, last time.Time) bool { + return checked.Truncate(time.Microsecond).After(last) +} + // applyRIExchangeResolve writes the outcome. completed keeps payment_due (the // reserved ceiling, an upper bound of what AWS accepted) and stamps // completed_at, so the row counts on today's UTC day like any other settled diff --git a/internal/config/store_postgres_ri_exchange_resolve_test.go b/internal/config/store_postgres_ri_exchange_resolve_test.go new file mode 100644 index 000000000..d46c3f50b --- /dev/null +++ b/internal/config/store_postgres_ri_exchange_resolve_test.go @@ -0,0 +1,39 @@ +package config + +import ( + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// Postgres keeps microseconds, so a last-change time read back from the +// database is the caller's time truncated. A check stamped with that same +// instant (nanoseconds included) must not count as after the change. +func TestCheckPostdatesDBTime(t *testing.T) { + change := time.Date(2026, 10, 23, 1, 2, 3, 456789123, time.UTC) + stored := change.Truncate(time.Microsecond) + + assert.False(t, checkPostdatesDBTime(change, stored), "same instant, extra nanoseconds") + assert.False(t, checkPostdatesDBTime(stored, stored), "equal") + assert.False(t, checkPostdatesDBTime(stored.Add(-time.Microsecond), stored), "before") + assert.True(t, checkPostdatesDBTime(stored.Add(time.Microsecond), stored), "one microsecond after") + assert.True(t, checkPostdatesDBTime(change.Add(time.Second), stored), "well after") +} + +func TestCheckRIExchangeCloudCheck_CheckedAtTheChangeIsRefused(t *testing.T) { + change := time.Date(2026, 10, 23, 1, 2, 3, 456789123, time.UTC) + row := riExchangeResolveRow{ + accountID: "123456789012", + region: "us-east-1", + updatedAt: change.Truncate(time.Microsecond), + dbNow: change.Add(time.Hour).Truncate(time.Microsecond), + } + check := RIExchangeCloudCheck{AccountID: row.accountID, Region: row.region, CheckedAt: change} + + require.ErrorIs(t, checkRIExchangeCloudCheck(check, row), ErrRIExchangeResolveInvalid) + + check.CheckedAt = change.Add(time.Minute) + require.NoError(t, checkRIExchangeCloudCheck(check, row)) +}