amadan

carlos / platform Public

console: fix billPass day-1 CE window, cost-guard daily pulls, method-probe leak, CE retry classification

Review fix round (1 Critical + 3 Important):

- CRITICAL: billPass's current-month TimePeriod collapsed Start==End
  on day 1 of any month (CE's End is exclusive) -> ValidationException
  -> the old any-error disableCE latch killed the previous month's
  pull in the same tick. End is now tomorrow's UTC date, plus a belt-
  and-braces skip if Start/End ever format to the same day.

- IMPORTANT: CE bills $0.01/request; an hourly tick pulling both
  months unconditionally cost ~$14.40/mo for a cost-tracking
  feature. pullBillMonth now checks the stored bill first and skips
  the CE call entirely when a costexplorer pull already landed today
  (UTC), and permanently stops re-pulling a closed month once it was
  pulled on or after day 3 of the following month (CE's late-arriving
  line items settle by then).

- IMPORTANT: the three write doors were registered with a method
  prefix ('POST /path'), so an unauthenticated GET got Go's default
  405 + Allow — leaking route existence, contradicting the doc
  comment's own non-probeability claim. Registered bare instead; each
  handler checks r.Method first, before the session/cliUser gate, so
  a wrong-method request is byte-identical to an unknown route.

- IMPORTANT: disableCE latched permanently on ANY GetCostAndUsage
  failure, so a throttle or a 5xx would silently kill pulls until
  redeploy. Now only auth-shaped failures latch: AccessDenied, an
  unrecognized/invalid credential (isAuthShapedError, matched via
  smithy.APIError codes) at query time, or the AWS config/credential
  chain never resolving at all (checked once, explicitly, bounded to
  5s, at client-construction time — this also fixes a real bug the
  naive classifier introduced: an unresolvable credential chain isn't
  a smithy.APIError, so without this it would retry, and pay the
  SDK's own multi-attempt IMDS timeout, on EVERY hourly tick
  forever). Everything else logs and retries next tick.

New tests: TestBillPassHandlesMonthBoundary, TestBillPassSkipsSecond
PullSameDay, TestBillPassStopsPullingSettledClosedMonth, TestBillPass
RetriesAfterTransientCEFailure, TestEconomicsWriteDoorsRejectNonPOST,
TestIsAuthShapedError (cepull_test.go).

go test ./internal/console/ -run 'BillPass|ScenarioSave|APIBill|Economics|IsAuthShaped'
-count=1: all pass, 5.2s (TestEconomicsPassWritesCensusOnce down from
40s to 5s under the bounded credential probe). Full go build ./...,
go vet ./..., go test ./... -count=1 (21 packages) and
GOOS=darwin go build ./... all clean.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Paul Campbell 684b76bbcd3fb0c8522b16fb011aedc2b4fd9dcd parent d27f0bb
5 files changed, +432 −38
  • internal/console/cepull.go +80 −15
  • internal/console/cepull_test.go +41 −0
  • internal/console/economicsbill_test.go +183 −0
  • internal/console/economicspass.go +119 −19
  • internal/console/server.go +9 −4
diff --git a/internal/console/cepull.go b/internal/console/cepull.go
index 80caf522b..52e5510d4 100644
--- a/internal/console/cepull.go
+++ b/internal/console/cepull.go
@@ -4,29 +4,38 @@
// environments), must not have that surface as a recurring error: the spec's
// posture is "a deployment that never pulled shows manual entry, not an
// error" (economicsview.go's loadRecentBills carries the same posture for a
-// missing bill object). realCECosts is where that posture is enforced: a
-// construction or query failure is logged exactly ONCE per process
-// (s.ceWarnOnce) and then s.ceDisabled latches true, so every later billPass
-// tick returns the same cached failure immediately — no repeated AWS calls,
-// no repeated log spam, hourly, forever.
+// missing bill object). realCECosts is where that posture is enforced —
+// but ONLY for construction-class failures: the AWS config never resolving
+// at all, no usable credential in the chain (checked once, explicitly,
+// bounded to 5s — see realCECosts), or a GetCostAndUsage failure shaped
+// like AccessDenied/an invalid credential (isAuthShapedError). Those latch
+// s.ceDisabled after one log line (s.ceWarnOnce) and stay off for the rest
+// of the process — there is no "billing IAM might appear later" retry
+// worth making. Everything else — throttling, a 5xx, a transient network
+// blip — is NOT auth-shaped: it logs and the next hourly tick tries again,
+// because CE itself throttles under sustained load and a single blip must
+// not silently kill bill pulls until redeploy.
package console
import (
"context"
+ "errors"
"fmt"
"log"
"strconv"
+ "strings"
"time"
"github.com/aws/aws-sdk-go-v2/aws"
awsconfig "github.com/aws/aws-sdk-go-v2/config"
"github.com/aws/aws-sdk-go-v2/service/costexplorer"
cetypes "github.com/aws/aws-sdk-go-v2/service/costexplorer/types"
+ "github.com/aws/smithy-go"
)
// resolveCECosts is billPass's entry point into the seam: the test/fake
// s.ceCosts function when a test set one, otherwise the real, lazily-built,
-// process-cached Cost Explorer client. Guarded by sync.Once/a plain bool
+// process-cached Cost Explorer client. Guarded by sync.Once/plain bools
// rather than a mutex because billPass only ever runs from
// StartEconomicsPass's single-goroutine hourly ticker (economicspass.go) —
// there is no concurrent caller for these fields to race against.
@@ -38,11 +47,9 @@ func (s *Server) resolveCECosts(ctx context.Context, start, end time.Time) (map[
}
// realCECosts builds the Cost Explorer client once (s.ceClientOnce) and
-// reuses it for the life of the process. Any failure — building the AWS
-// config, or the GetCostAndUsage call itself (AccessDenied is the expected
-// shape for a deployment that never granted billing IAM) — latches
-// s.ceDisabled so every subsequent call fails fast without touching AWS
-// again, and logs exactly once via s.ceWarnOnce.
+// reuses it for the life of the process. See the package doc comment above
+// for which failures latch s.ceDisabled (auth-shaped, permanent) versus
+// which just fail this one tick (everything else, retried next hour).
func (s *Server) realCECosts(ctx context.Context, start, end time.Time) (map[string]float64, error) {
if s.ceDisabled {
return nil, fmt.Errorf("console: cost explorer disabled for this process")
@@ -50,9 +57,31 @@ func (s *Server) realCECosts(ctx context.Context, start, end time.Time) (map[str
s.ceClientOnce.Do(func() {
cfg, err := awsconfig.LoadDefaultConfig(ctx)
if err != nil {
+ // The AWS config itself failed to resolve — opaque and
+ // permanent for this process: there is no "retry the SDK's
+ // environment/instance-metadata lookup next tick" path, so
+ // this always latches.
s.disableCE(fmt.Errorf("console: cost explorer: load aws config: %w", err))
return
}
+ // Force credential resolution HERE, once, under a short bound —
+ // rather than letting it happen lazily inside the first
+ // GetCostAndUsage call below. LoadDefaultConfig never itself
+ // contacts the network; a missing credential chain (no IMDS role,
+ // no ~/.aws/credentials — exactly the "opaque credential-chain
+ // construction failure" this seam exists to survive) only
+ // surfaces once something tries to actually sign a request, as a
+ // generic wrapped error with no smithy.APIError to classify —
+ // left unhandled here, EVERY hourly tick would pay the SDK's own
+ // multi-attempt IMDS retry/timeout before failing, forever. Doing
+ // it once, bounded, at construction time keeps that cost to a
+ // single ~5s probe for the life of the process instead.
+ credCtx, cancel := context.WithTimeout(ctx, 5*time.Second)
+ defer cancel()
+ if _, err := cfg.Credentials.Retrieve(credCtx); err != nil {
+ s.disableCE(fmt.Errorf("console: cost explorer: resolve aws credentials: %w", err))
+ return
+ }
s.ceClient = costexplorer.NewFromConfig(cfg)
})
if s.ceDisabled {
@@ -71,7 +100,11 @@ func (s *Server) realCECosts(ctx context.Context, start, end time.Time) (map[str
},
})
if err != nil {
- s.disableCE(fmt.Errorf("console: cost explorer: get cost and usage: %w", err))
+ if isAuthShapedError(err) {
+ s.disableCE(fmt.Errorf("console: cost explorer: get cost and usage: %w", err))
+ } else {
+ log.Printf("console: cost explorer: get cost and usage: %v (not auth-shaped; will retry next tick)", err)
+ }
return nil, err
}
@@ -96,10 +129,42 @@ func (s *Server) realCECosts(ctx context.Context, start, end time.Time) (map[str
return costs, nil
}
+// isAuthShapedError reports whether err is the "billing IAM isn't set up
+// here" shape — the only GetCostAndUsage failure realCECosts treats as
+// permanent for the process. AWS surfaces a missing/wrong grant as a smithy
+// API error whose code names the problem; Cost Explorer's own generated
+// exception types (types/errors.go) don't include these because they're
+// account/credential-wide, not CE-specific — they arrive as
+// *smithy.GenericAPIError instead, which is why this matches on
+// ErrorCode() rather than a typed CE exception. Matched by substring
+// (Contains, not exact-equal) because AWS is inconsistent about the
+// "Exception" suffix across services and error-message formats
+// (AccessDenied vs AccessDeniedException being the canonical example).
+func isAuthShapedError(err error) bool {
+ var apiErr smithy.APIError
+ if !errors.As(err, &apiErr) {
+ return false
+ }
+ code := apiErr.ErrorCode()
+ for _, shape := range []string{
+ "AccessDenied",
+ "UnrecognizedClient",
+ "InvalidClientTokenId",
+ "InvalidAccessKeyId",
+ "AuthFailure",
+ "ExpiredToken",
+ } {
+ if strings.Contains(code, shape) {
+ return true
+ }
+ }
+ return false
+}
+
// disableCE logs err exactly once for the process (s.ceWarnOnce) and
-// latches s.ceDisabled — called on both the construction-failure and the
-// query-failure path, so either shape of "billing IAM isn't set up here"
-// ends the same way: one log line, then silent manual mode.
+// latches s.ceDisabled — called only for auth-shaped failures (see
+// isAuthShapedError), so "billing IAM isn't set up here" ends in one log
+// line and silent manual mode, while a throttle or a 5xx keeps retrying.
func (s *Server) disableCE(err error) {
s.ceDisabled = true
s.ceWarnOnce.Do(func() {
diff --git a/internal/console/cepull_test.go b/internal/console/cepull_test.go
new file mode 100644
index 000000000..6ccb5d180
--- /dev/null
+++ b/internal/console/cepull_test.go
@@ -0,0 +1,41 @@
+package console
+
+import (
+ "errors"
+ "testing"
+
+ "github.com/aws/smithy-go"
+)
+
+// TestIsAuthShapedError pins the classifier the review round added:
+// realCECosts must latch (permanently disable itself for the process) only
+// on the "billing IAM isn't set up here" shape — AccessDenied and an
+// unrecognized/invalid credential — and must NOT latch on a throttle, a
+// service-side limit, a 5xx, or a plain non-API error, all of which are
+// worth retrying on the next hourly tick.
+func TestIsAuthShapedError(t *testing.T) {
+ cases := []struct {
+ name string
+ err error
+ want bool
+ }{
+ {"access denied exception", &smithy.GenericAPIError{Code: "AccessDeniedException"}, true},
+ {"access denied bare", &smithy.GenericAPIError{Code: "AccessDenied"}, true},
+ {"unrecognized client", &smithy.GenericAPIError{Code: "UnrecognizedClientException"}, true},
+ {"invalid client token id", &smithy.GenericAPIError{Code: "InvalidClientTokenId"}, true},
+ {"invalid access key id", &smithy.GenericAPIError{Code: "InvalidAccessKeyId"}, true},
+ {"expired token", &smithy.GenericAPIError{Code: "ExpiredTokenException"}, true},
+ {"auth failure", &smithy.GenericAPIError{Code: "AuthFailure"}, true},
+ {"throttling", &smithy.GenericAPIError{Code: "ThrottlingException"}, false},
+ {"limit exceeded", &smithy.GenericAPIError{Code: "LimitExceededException"}, false},
+ {"internal error", &smithy.GenericAPIError{Code: "InternalError"}, false},
+ {"service unavailable", &smithy.GenericAPIError{Code: "ServiceUnavailableException"}, false},
+ {"plain error", errors.New("network is down"), false},
+ {"nil", nil, false},
+ }
+ for _, c := range cases {
+ if got := isAuthShapedError(c.err); got != c.want {
+ t.Errorf("%s: isAuthShapedError(%v) = %v, want %v", c.name, c.err, got, c.want)
+ }
+ }
+}
diff --git a/internal/console/economicsbill_test.go b/internal/console/economicsbill_test.go
index 342d1026e..13e3e84be 100644
--- a/internal/console/economicsbill_test.go
+++ b/internal/console/economicsbill_test.go
@@ -107,6 +107,189 @@ func TestBillPassNoCEIsQuiet(t *testing.T) {
}
}
+// TestBillPassHandlesMonthBoundary is the review-round regression for the
+// day-1 bug: CE's TimePeriod.End is EXCLUSIVE and must be strictly after
+// Start. On day 1 of any month, the naive "current month" period
+// (firstOfThisMonth .. now) collapses Start==End (both format to the same
+// calendar date), which CE would refuse outright — and that refusal used to
+// latch ceDisabled and kill the PREVIOUS month's pull in the very same
+// tick, the most valuable and just-closed one. The fake fails loudly if it
+// is ever asked for a same-day period, so this proves billPass never sends
+// one; both months must land.
+func TestBillPassHandlesMonthBoundary(t *testing.T) {
+ fx := newConsoleFixture(t)
+ ctx := context.Background()
+ now := time.Date(2026, 9, 1, 3, 0, 0, 0, time.UTC)
+
+ fx.srv.ceCosts = func(ctx context.Context, start, end time.Time) (map[string]float64, error) {
+ if start.Format("2006-01-02") == end.Format("2006-01-02") {
+ t.Fatalf("ceCosts called with start == end == %s (day 1 collapse)", start.Format("2006-01-02"))
+ }
+ return map[string]float64{"AmazonEC2": 10}, nil
+ }
+
+ if err := fx.srv.billPass(ctx, now); err != nil {
+ t.Fatal(err)
+ }
+ for _, month := range []string{"2026-09", "2026-08"} {
+ if _, _, err := fx.rel.Get(ctx, economics.BillKey(month)); err != nil {
+ t.Fatalf("bill %s not written on day 1 of the month: %v", month, err)
+ }
+ }
+}
+
+// TestBillPassSkipsSecondPullSameDay: CE bills $0.01/request, so an hourly
+// pass tick must pull each month AT MOST ONCE per UTC day — a second
+// billPass call the same day must not touch the ceCosts seam again at all.
+func TestBillPassSkipsSecondPullSameDay(t *testing.T) {
+ fx := newConsoleFixture(t)
+ ctx := context.Background()
+ now := time.Date(2026, 8, 11, 3, 0, 0, 0, time.UTC)
+
+ var calls int
+ fx.srv.ceCosts = func(ctx context.Context, start, end time.Time) (map[string]float64, error) {
+ calls++
+ return map[string]float64{"AmazonEC2": 10}, nil
+ }
+
+ if err := fx.srv.billPass(ctx, now); err != nil {
+ t.Fatal(err)
+ }
+ first := calls
+ if first != 2 {
+ t.Fatalf("first billPass made %d ceCosts calls, want 2 (current + previous month)", first)
+ }
+
+ if err := fx.srv.billPass(ctx, now.Add(4*time.Hour)); err != nil {
+ t.Fatal(err)
+ }
+ if calls != first {
+ t.Fatalf("second billPass the same UTC day made %d more ceCosts calls, want 0", calls-first)
+ }
+}
+
+// TestBillPassStopsPullingSettledClosedMonth: a closed (previous) month
+// that was already pulled on or after day 3 of the following month has
+// seen CE's late-arriving line items settle — billPass must stop pulling
+// it forever, rather than paying for it every day for the rest of time.
+// The still-open current month, on a later day, still pulls normally.
+func TestBillPassStopsPullingSettledClosedMonth(t *testing.T) {
+ fx := newConsoleFixture(t)
+ ctx := context.Background()
+
+ settled := economics.Bill{
+ Schema: 1, Month: "2026-07", TotalUSD: 10,
+ Services: map[string]float64{"AmazonEC2": 10}, Source: economics.SourceCostExplorer,
+ PulledAt: time.Date(2026, 8, 5, 3, 0, 0, 0, time.UTC), // day 5 >= the day-3 grace
+ }
+ putStoreJSON(t, fx.rel, economics.BillKey("2026-07"), settled)
+
+ var calls int
+ fx.srv.ceCosts = func(ctx context.Context, start, end time.Time) (map[string]float64, error) {
+ calls++
+ return map[string]float64{"AmazonEC2": 99}, nil
+ }
+
+ if err := fx.srv.billPass(ctx, time.Date(2026, 8, 20, 3, 0, 0, 0, time.UTC)); err != nil {
+ t.Fatal(err)
+ }
+ if calls != 1 {
+ t.Fatalf("billPass made %d ceCosts calls, want 1 (current month only; the settled closed month must be skipped)", calls)
+ }
+ raw, _, err := fx.rel.Get(ctx, economics.BillKey("2026-07"))
+ if err != nil {
+ t.Fatal(err)
+ }
+ var got economics.Bill
+ if err := json.Unmarshal(raw, &got); err != nil {
+ t.Fatal(err)
+ }
+ if got.TotalUSD != 10 {
+ t.Fatalf("settled closed-month bill total = %v, want unchanged 10 (must not have been re-pulled)", got.TotalUSD)
+ }
+}
+
+// TestBillPassRetriesAfterTransientCEFailure: a non-auth-shaped ceCosts
+// failure (a throttle, a network blip) must not disable further attempts —
+// realCECosts' auth-shaped latch (cepull.go, isAuthShapedError) is the only
+// thing that may do that, and the seam bypasses realCECosts entirely, so
+// this proves billPass itself adds no independent latch of its own: a tick
+// whose calls all fail leaves nothing written, and the very next tick's
+// calls still reach the seam and succeed.
+func TestBillPassRetriesAfterTransientCEFailure(t *testing.T) {
+ fx := newConsoleFixture(t)
+ ctx := context.Background()
+ now := time.Date(2026, 8, 11, 3, 0, 0, 0, time.UTC)
+
+ var calls int
+ fx.srv.ceCosts = func(ctx context.Context, start, end time.Time) (map[string]float64, error) {
+ calls++
+ if calls <= 2 { // both months fail on the first tick
+ return nil, errors.New("ThrottlingException: rate exceeded")
+ }
+ return map[string]float64{"AmazonEC2": 42}, nil
+ }
+
+ if err := fx.srv.billPass(ctx, now); err != nil {
+ t.Fatal(err)
+ }
+ if _, _, err := fx.rel.Get(ctx, economics.BillKey("2026-08")); !errors.Is(err, os.ErrNotExist) {
+ t.Fatal("bill written despite every ceCosts call failing on the first tick")
+ }
+
+ if err := fx.srv.billPass(ctx, now.Add(time.Hour)); err != nil {
+ t.Fatal(err)
+ }
+ raw, _, err := fx.rel.Get(ctx, economics.BillKey("2026-08"))
+ if err != nil {
+ t.Fatal(err)
+ }
+ var b economics.Bill
+ if err := json.Unmarshal(raw, &b); err != nil {
+ t.Fatal(err)
+ }
+ if b.TotalUSD != 42 {
+ t.Fatalf("bill total = %v, want 42 (the retried tick's data)", b.TotalUSD)
+ }
+}
+
+// TestEconomicsWriteDoorsRejectNonPOST: the three write doors are
+// registered WITHOUT a method prefix (server.go) specifically so a
+// wrong-method request never gets Go's default 405 + Allow — which would
+// itself confirm the route exists to someone who merely guessed the URL.
+// Every one of them must answer a GET with the exact same 404 an unknown
+// route gets, signed out AND signed in as an operator (the method check
+// runs before the session/operator gate).
+func TestEconomicsWriteDoorsRejectNonPOST(t *testing.T) {
+ fx := newConsoleFixture(t)
+ out := newBrowser(t)
+ ghostStatus, _, ghostBody := get(t, out, fx.ts.URL+"/this-route-does-not-exist")
+
+ consolePaths := []string{"/fleet/economics/scenarios", "/fleet/economics/bill"}
+ for _, path := range consolePaths {
+ if status, _, body := get(t, out, fx.ts.URL+path); status != ghostStatus || body != ghostBody {
+ t.Fatalf("signed-out GET %s = %d %q, want the same 404 as a missing route (%d %q)", path, status, body, ghostStatus, ghostBody)
+ }
+ }
+
+ c := signInPaul(t, fx)
+ if err := fx.st.SetOperator("paul@keymail.dev", true); err != nil {
+ t.Fatal(err)
+ }
+ for _, path := range consolePaths {
+ if status, _, body := get(t, c, fx.ts.URL+path); status != ghostStatus || body != ghostBody {
+ t.Fatalf("operator GET %s = %d %q, want the same 404 as a missing route (%d %q)", path, status, body, ghostStatus, ghostBody)
+ }
+ }
+
+ // The API door: an unauthenticated GET (no Authorization header) must
+ // be the same ghost 404 too, not cliUser's 401 — that would confirm
+ // the route exists just as surely as a 405 would.
+ if status, _, body := get(t, out, fx.ts.URL+"/api/economics/bill"); status != ghostStatus || body != ghostBody {
+ t.Fatalf("unauthenticated GET /api/economics/bill = %d %q, want the same 404 as a missing route (%d %q)", status, body, ghostStatus, ghostBody)
+ }
+}
+
// TestScenarioSaveGateAndValidation pins the gate order (session ->
// IsOperator -> byte-identical 404, matching TestFleetOperatorOnly's
// comparison shape) BEFORE csrf, then bad-JSON and happy-path behaviour.
diff --git a/internal/console/economicspass.go b/internal/console/economicspass.go
index ef2b14211..c3b598306 100644
--- a/internal/console/economicspass.go
+++ b/internal/console/economicspass.go
@@ -588,6 +588,13 @@ func (s *Server) statementsPass(ctx context.Context, now time.Time) error {
return nil
}
+// billCEDayGrace is how many days into the FOLLOWING month a closed month's
+// bill must have been pulled before billPass stops re-pulling it forever —
+// CE's late-arriving line items settle by then, so a pull on or after this
+// day is treated as final. Below this day, the closed month is pulled at
+// most once per UTC day, same as the current (open) month.
+const billCEDayGrace = 3
+
// billPass pulls this month and last month's whole-account bill through the
// ceCosts seam (cepull.go) and writes each, manual-wins. It ALWAYS returns
// nil: a Cost Explorer failure (no billing IAM grant, a disabled seam, AWS
@@ -596,20 +603,37 @@ func (s *Server) statementsPass(ctx context.Context, now time.Time) error {
// (economicsview.go's loadRecentBills carries the same posture for a bill
// object that simply isn't there yet). Each month's pull/write is
// independent — a failure on one must not skip the other.
+//
+// The open (current) month's TimePeriod.End is tomorrow's UTC date, not
+// now — CE's End is EXCLUSIVE and must be strictly after Start
+// (firstOfThisMonth): on day 1 of any month, now truncated to its own day
+// equals firstOfThisMonth, which would send Start==End and CE would refuse
+// the whole request with a ValidationException. Bumping End to the day
+// AFTER now's UTC day (CE accepts a request that reaches into "today",
+// not-yet-complete) sidesteps day 1 without a special case. The extra
+// Format-equality check below is belt and braces against any other path
+// that could still produce a same-day Start/End.
func (s *Server) billPass(ctx context.Context, now time.Time) error {
nowUTC := now.UTC()
+ today := time.Date(nowUTC.Year(), nowUTC.Month(), nowUTC.Day(), 0, 0, 0, 0, time.UTC)
+ tomorrow := today.AddDate(0, 0, 1)
firstOfThisMonth := time.Date(nowUTC.Year(), nowUTC.Month(), 1, 0, 0, 0, 0, time.UTC)
firstOfLastMonth := firstOfThisMonth.AddDate(0, -1, 0)
periods := []struct {
- month string
- start, end time.Time
+ month string
+ start, end time.Time
+ closedMonth bool // the just-finished month: eligible for the day-3 settle-and-stop rule
}{
- {usage.MonthOf(nowUTC), firstOfThisMonth, nowUTC},
- {usage.MonthOf(firstOfLastMonth), firstOfLastMonth, firstOfThisMonth},
+ {usage.MonthOf(nowUTC), firstOfThisMonth, tomorrow, false},
+ {usage.MonthOf(firstOfLastMonth), firstOfLastMonth, firstOfThisMonth, true},
}
for _, p := range periods {
- if err := s.pullBillMonth(ctx, p.month, p.start, p.end, nowUTC); err != nil {
+ if p.start.Format("2006-01-02") == p.end.Format("2006-01-02") {
+ log.Printf("console: bill pass %s: start/end both %s; CE requires start<end, skipping this tick", p.month, p.start.Format("2006-01-02"))
+ continue
+ }
+ if err := s.pullBillMonth(ctx, p.month, p.start, p.end, nowUTC, p.closedMonth); err != nil {
log.Printf("console: bill pass %s: %v", p.month, err)
}
}
@@ -617,8 +641,22 @@ func (s *Server) billPass(ctx context.Context, now time.Time) error {
}
// pullBillMonth pulls one month's whole-account by-service costs through
-// the ceCosts seam and writes it, manual-wins (upsertBill's manualWins=true).
-func (s *Server) pullBillMonth(ctx context.Context, month string, start, end, pulledAt time.Time) error {
+// the ceCosts seam and writes it, manual-wins (upsertBill's manualWins=true)
+// — but first checks whether a pull is even worth making: CE bills
+// $0.01/request, and the hourly pass tick would otherwise burn roughly
+// $14/mo (2 months × 24 ticks/day × 30 days) pulling data that, most hours,
+// hasn't changed since the last pull. shouldSkipCEPull enforces "at most
+// once per UTC day" for both months, plus the day-3-of-next-month
+// settle-and-stop rule for closedMonth.
+func (s *Server) pullBillMonth(ctx context.Context, month string, start, end, now time.Time, closedMonth bool) error {
+ existing, err := s.getBill(ctx, month)
+ if err != nil {
+ return fmt.Errorf("read existing bill: %w", err)
+ }
+ if shouldSkipCEPull(existing, closedMonth, now) {
+ return nil
+ }
+
costs, err := s.resolveCECosts(ctx, start, end)
if err != nil {
return fmt.Errorf("cost explorer: %w", err)
@@ -633,11 +671,50 @@ func (s *Server) pullBillMonth(ctx context.Context, month string, start, end, pu
TotalUSD: total,
Services: costs,
Source: economics.SourceCostExplorer,
- PulledAt: pulledAt,
+ PulledAt: now,
}
return s.upsertBill(ctx, bill, true)
}
+// shouldSkipCEPull decides whether pullBillMonth should skip calling Cost
+// Explorer THIS tick, given the bill currently stored for the month (nil if
+// none) — see pullBillMonth's doc comment for why this exists at all. Only
+// ever consults a SourceCostExplorer bill: a SourceManual bill is a
+// different caller's business (upsertBill's manualWins check handles it at
+// write time), not a reason to skip a pull here.
+func shouldSkipCEPull(existing *economics.Bill, closedMonth bool, now time.Time) bool {
+ if existing == nil || existing.Source != economics.SourceCostExplorer {
+ return false
+ }
+ if closedMonth && existing.PulledAt.Day() >= billCEDayGrace {
+ // Settled: a closed month pulled on or after day 3 of the
+ // following month has already seen CE's late-arriving line items
+ // land. Its numbers don't move again — re-pulling it forever
+ // would be pure waste, unlike the open month, which keeps
+ // changing all month.
+ return true
+ }
+ return usage.DayOf(existing.PulledAt) == usage.DayOf(now)
+}
+
+// getBill reads and parses the stored bill for month, or (nil, nil) if none
+// exists yet — the "no bill pulled yet" state loadRecentBills
+// (economicsview.go) also treats as normal, not an error.
+func (s *Server) getBill(ctx context.Context, month string) (*economics.Bill, error) {
+ raw, _, err := s.cfg.Releases.Get(ctx, economics.BillKey(month))
+ if err != nil {
+ if errors.Is(err, os.ErrNotExist) {
+ return nil, nil
+ }
+ return nil, err
+ }
+ var b economics.Bill
+ if jerr := json.Unmarshal(raw, &b); jerr != nil {
+ return nil, jerr
+ }
+ return &b, nil
+}
+
// upsertBill is one CAS read-etag-then-put attempt at economics.BillKey(bill
// .Month) — appconfig's applyAt pattern (internal/appconfig/store.go:162-190),
// one attempt, no retry loop: a lost race surfaces as release.ErrCASConflict
@@ -770,12 +847,22 @@ func (s *Server) putActiveScenario(ctx context.Context, name string) error {
}
// handleEconomicsScenarioSave writes a named pricing scenario (and,
-// optionally, activates it). Gate is handleFleet's exact shape — session ->
-// IsOperator -> a bare http.NotFound byte-identical with an unknown route —
-// BEFORE the CSRF check, per the console security rule: role existence must
-// never be probeable, and a CSRF rejection would leak that this route
-// exists to a non-operator who merely guessed the URL.
+// optionally, activates it). Registered without a method prefix
+// (server.go), so the FIRST check is r.Method — a GET (or anything but
+// POST) is byte-identical to an unknown route, never a 405 + Allow, which
+// would leak that this path exists to someone who merely guessed it. Method
+// check comes before EVERYTHING else, including the session gate: a
+// wrong-method request must be indistinguishable from an unknown route
+// regardless of who's asking. Past that, gate is handleFleet's exact shape
+// — session -> IsOperator -> a bare http.NotFound byte-identical with an
+// unknown route — BEFORE the CSRF check, per the console security rule:
+// role existence must never be probeable, and a CSRF rejection would leak
+// that this route exists to a non-operator who merely guessed the URL.
func (s *Server) handleEconomicsScenarioSave(w http.ResponseWriter, r *http.Request) {
+ if r.Method != http.MethodPost {
+ http.NotFound(w, r)
+ return
+ }
addr, raw, ok := s.session(r)
if !ok {
http.NotFound(w, r)
@@ -873,8 +960,13 @@ func parseBillLines(text string) (map[string]float64, error) {
// handleEconomicsBillSave writes a manually-entered bill for one month —
// SourceManual, always overwriting whatever is there (upsertBill's
-// manualWins=false). Same gate shape as handleEconomicsScenarioSave.
+// manualWins=false). Same gate shape as handleEconomicsScenarioSave,
+// including the method check before everything else.
func (s *Server) handleEconomicsBillSave(w http.ResponseWriter, r *http.Request) {
+ if r.Method != http.MethodPost {
+ http.NotFound(w, r)
+ return
+ }
addr, raw, ok := s.session(r)
if !ok {
http.NotFound(w, r)
@@ -936,12 +1028,20 @@ func (s *Server) handleEconomicsBillSave(w http.ResponseWriter, r *http.Request)
}
// handleAPIEconomicsBill is the CLI's manual-bill door (issue: `carlos
-// economics bill`, Task 15). Gate mirrors the console form's exactly, just
-// through the bearer/cliUser identity instead of a session: cliUser ->
-// IsOperator -> a bare http.NotFound byte-identical with an unknown API
-// route, BEFORE the body is even read — same role-not-probeable reasoning
-// as handleEconomicsScenarioSave.
+// economics bill`, Task 15). Registered without a method prefix
+// (server.go); the method check comes first, same reasoning as
+// handleEconomicsScenarioSave — a GET here (no Authorization header) must
+// not reach cliUser's 401, which would itself confirm the route exists.
+// Past that, gate mirrors the console form's exactly, just through the
+// bearer/cliUser identity instead of a session: cliUser -> IsOperator -> a
+// bare http.NotFound byte-identical with an unknown API route, BEFORE the
+// body is even read — same role-not-probeable reasoning as
+// handleEconomicsScenarioSave.
func (s *Server) handleAPIEconomicsBill(w http.ResponseWriter, r *http.Request) {
+ if r.Method != http.MethodPost {
+ http.NotFound(w, r)
+ return
+ }
addr, ok := s.cliUser(w, r)
if !ok {
return
diff --git a/internal/console/server.go b/internal/console/server.go
index abf5fa5e2..d102a600e 100644
--- a/internal/console/server.go
+++ b/internal/console/server.go
@@ -381,10 +381,15 @@ func NewServer(cfg Config, st *Store) (*Server, error) {
s.mux.HandleFunc("GET /fleet/metrics", s.handleFleetMetrics)
// Unit economics (Task 12): scenario/bill writes, operator-only, same
// gate shape as handleFleet above. GET /fleet/economics itself is
- // Task 13.
- s.mux.HandleFunc("POST /fleet/economics/scenarios", s.handleEconomicsScenarioSave)
- s.mux.HandleFunc("POST /fleet/economics/bill", s.handleEconomicsBillSave)
- s.mux.HandleFunc("POST /api/economics/bill", s.handleAPIEconomicsBill)
+ // Task 13. Registered WITHOUT a method prefix, deliberately: a
+ // "POST /path" pattern answers a GET with 405 + Allow, which leaks that
+ // the route exists to someone who merely guessed the URL — the exact
+ // probe handleFleet's 404-to-everyone gate exists to prevent. Each
+ // handler below checks r.Method itself, before anything else, so a
+ // wrong-method request is byte-identical to an unknown route.
+ s.mux.HandleFunc("/fleet/economics/scenarios", s.handleEconomicsScenarioSave)
+ s.mux.HandleFunc("/fleet/economics/bill", s.handleEconomicsBillSave)
+ s.mux.HandleFunc("/api/economics/bill", s.handleAPIEconomicsBill)
// CLI identity: the device-code login `carlos auth login` drives, the
// approval page, and the tokens an owner can see and kill (authcli.go).
s.registerCLIRoutes()