diff --git a/internal/handlers/billing_coverage_test.go b/internal/handlers/billing_coverage_test.go index a369da34..c9fff027 100644 --- a/internal/handlers/billing_coverage_test.go +++ b/internal/handlers/billing_coverage_test.go @@ -1089,10 +1089,16 @@ func TestBrevo_Receive_UnknownMessageID_Returns200MatchedFalse(t *testing.T) { db, mock, err := sqlmock.New() require.NoError(t, err) defer db.Close() - // 0 rows affected → matched=false branch + // 0 rows affected → existence probe → no row → matched=false branch. + // (bug bash #6: delivered UPDATE now carries the terminal-class guard + + // a follow-up SELECT to distinguish terminal-kept from genuinely unknown.) mock.ExpectExec(`UPDATE forwarder_sent`). - WithArgs("delivered", "brevo", "stranger"). + WithArgs("delivered", "brevo", "stranger", + "bounced_hard", "bounced_soft", "rejected", "complaint", "unsubscribed"). WillReturnResult(sqlmock.NewResult(0, 0)) + mock.ExpectQuery(`SELECT classification FROM forwarder_sent`). + WithArgs("brevo", "stranger"). + WillReturnRows(sqlmock.NewRows([]string{"classification"})) cfg := &config.Config{BrevoWebhookSecret: "correct_secret_at_least_32_bytes_xx"} app := brevoTxAppCoverage(t, db, cfg) @@ -1107,3 +1113,34 @@ func TestBrevo_Receive_UnknownMessageID_Returns200MatchedFalse(t *testing.T) { require.NoError(t, json.NewDecoder(resp.Body).Decode(&body)) assert.Equal(t, false, body["matched"]) } + +// TestBrevo_Receive_Delivered_ProbeError covers the delivered-handler SELECT +// existence-probe error branch (brevo_webhook.go: `if qErr != nil`). When the +// terminal-class-guarded UPDATE affects 0 rows, the handler runs a follow-up +// SELECT to distinguish "terminal-kept" from "genuinely unknown". If THAT +// probe errors (a real DB fault, not ErrNoRows), the handler must surface the +// error (→ 500) rather than mislabel the message — Brevo will retry. +func TestBrevo_Receive_Delivered_ProbeError(t *testing.T) { + db, mock, err := sqlmock.New() + require.NoError(t, err) + defer db.Close() + mock.ExpectExec(`UPDATE forwarder_sent`). + WithArgs("delivered", "brevo", "stranger", + "bounced_hard", "bounced_soft", "rejected", "complaint", "unsubscribed"). + WillReturnResult(sqlmock.NewResult(0, 0)) + // Probe returns a non-ErrNoRows fault → qErr != nil branch. + mock.ExpectQuery(`SELECT classification FROM forwarder_sent`). + WithArgs("brevo", "stranger"). + WillReturnError(errors.New("probe boom")) + + cfg := &config.Config{BrevoWebhookSecret: "correct_secret_at_least_32_bytes_xx"} + app := brevoTxAppCoverage(t, db, cfg) + req := httptest.NewRequest(http.MethodPost, "/webhooks/brevo/correct_secret_at_least_32_bytes_xx", + bytes.NewBufferString(`{"event":"delivered","email":"u@example.com","message-id":"stranger"}`)) + req.Header.Set("Content-Type", "application/json") + resp, err := app.Test(req, -1) + require.NoError(t, err) + defer resp.Body.Close() + assert.Equal(t, http.StatusInternalServerError, resp.StatusCode) + require.NoError(t, mock.ExpectationsWereMet()) +} diff --git a/internal/handlers/brevo_webhook.go b/internal/handlers/brevo_webhook.go index 5dc49057..d8bd8c02 100644 --- a/internal/handlers/brevo_webhook.go +++ b/internal/handlers/brevo_webhook.go @@ -483,13 +483,21 @@ func (h *BrevoTransactionalWebhookHandler) Receive(c *fiber.Ctx) error { // GREATEST so a re-delivery of the same event doesn't bump the // timestamp. func handleBrevoDelivered(ctx context.Context, h *BrevoTransactionalWebhookHandler, evt brevoTransactionalEvent) (bool, error) { + // bug bash #6: a 'delivered' event must NOT clobber a TERMINAL negative + // outcome. Brevo delivers webhook events out of order, so a late + // 'delivered' (SMTP-accept) can arrive after a 'bounced_*' / 'rejected' / + // 'complaint' / 'unsubscribed' has already been recorded — overwriting the + // ledger's truth surface (rule 12). The guard preserves terminal classes. res, err := h.db.ExecContext(ctx, ` UPDATE forwarder_sent SET classification = $1, delivered_at = COALESCE(GREATEST(delivered_at, NOW()), NOW()) WHERE provider = $2 AND provider_id = $3 - `, LedgerClassDelivered, brevoProviderName, evt.MessageID) + AND classification NOT IN ($4, $5, $6, $7, $8) + `, LedgerClassDelivered, brevoProviderName, evt.MessageID, + LedgerClassBouncedHard, LedgerClassBouncedSoft, LedgerClassRejected, + LedgerClassComplaint, LedgerClassUnsubscribed) if err != nil { return false, err } @@ -497,14 +505,37 @@ func handleBrevoDelivered(ctx context.Context, h *BrevoTransactionalWebhookHandl if err != nil { return false, err } - if n == 0 { + if n > 0 { + slog.Info("webhook.brevo.delivered", + "message_id", evt.MessageID, + "recipient_masked", models.MaskEmail(evt.Email), + "rows_updated", n, + ) + return true, nil + } + // n == 0: either no row matches this messageId, OR the row exists but + // already holds a terminal class we intentionally preserved. Distinguish + // the two so a real-but-terminal message isn't mislabeled "unknown". + var existingClass string + qErr := h.db.QueryRowContext(ctx, ` + SELECT classification FROM forwarder_sent + WHERE provider = $1 AND provider_id = $2 + LIMIT 1 + `, brevoProviderName, evt.MessageID).Scan(&existingClass) + if qErr == sql.ErrNoRows { warnUnknownBrevoMessage(ctx, evt, brevoEventDelivered) return false, nil } - slog.Info("webhook.brevo.delivered", + if qErr != nil { + return false, qErr + } + // Row matched but a terminal classification was kept — this IS a known + // message (matched=true), we just don't downgrade the outcome. + slog.Info("webhook.brevo.delivered_kept_terminal", "message_id", evt.MessageID, "recipient_masked", models.MaskEmail(evt.Email), - "rows_updated", n, + "kept_classification", existingClass, + "note", "out-of-order delivered ignored — terminal class preserved (bug bash #6)", ) return true, nil } diff --git a/internal/handlers/brevo_webhook_test.go b/internal/handlers/brevo_webhook_test.go index 2efa94e8..cf7b9d9d 100644 --- a/internal/handlers/brevo_webhook_test.go +++ b/internal/handlers/brevo_webhook_test.go @@ -86,8 +86,12 @@ func expectClassificationUpdate(mock sqlmock.Sqlmock, class, providerID string, // expectDeliveredUpdate sets up a sqlmock expectation for the // delivered-stamping path (classification + delivered_at). func expectDeliveredUpdate(mock sqlmock.Sqlmock, providerID string, rowsAffected int64) { + // The delivered UPDATE now carries the terminal-class non-clobber guard + // (bug bash #6): classification NOT IN (bounced_hard, bounced_soft, + // rejected, complaint, unsubscribed) — 5 extra positional args. mock.ExpectExec(`UPDATE forwarder_sent`). - WithArgs("delivered", "brevo", providerID). + WithArgs("delivered", "brevo", providerID, + "bounced_hard", "bounced_soft", "rejected", "complaint", "unsubscribed"). WillReturnResult(sqlmock.NewResult(0, rowsAffected)) } @@ -321,6 +325,12 @@ func TestBrevoTxWebhook_UnknownMessageID_Returns200MatchedFalse(t *testing.T) { // UPDATE runs but affects 0 rows. expectDeliveredUpdate(mock, "msg-orphan", 0) + // bug bash #6: a 0-row delivered UPDATE now probes whether a row exists at + // all (terminal-kept) vs truly unknown. Empty result → Scan returns + // ErrNoRows → genuinely unknown → matched:false. + mock.ExpectQuery(`SELECT classification FROM forwarder_sent`). + WithArgs("brevo", "msg-orphan"). + WillReturnRows(sqlmock.NewRows([]string{"classification"})) h := handlers.NewBrevoTransactionalWebhookHandler(db, &config.Config{BrevoWebhookSecret: testBrevoTxSecret}) app := brevoTxApp(t, h) @@ -335,6 +345,39 @@ func TestBrevoTxWebhook_UnknownMessageID_Returns200MatchedFalse(t *testing.T) { } } +// ── 9b. bug bash #6: an out-of-order 'delivered' must NOT clobber a terminal +// bounce/complaint. The UPDATE excludes terminal classes (0 rows), the +// existence probe finds the kept terminal class → matched:true, no +// downgrade. + +func TestBrevoTxWebhook_DeliveredAfterBounce_KeepsTerminalClass(t *testing.T) { + db, mock, _ := sqlmock.New() + defer db.Close() + + // Terminal-guarded UPDATE matches no row (classification already terminal). + expectDeliveredUpdate(mock, "msg-bounced", 0) + // Existence probe returns the preserved terminal class. + mock.ExpectQuery(`SELECT classification FROM forwarder_sent`). + WithArgs("brevo", "msg-bounced"). + WillReturnRows(sqlmock.NewRows([]string{"classification"}).AddRow("bounced_hard")) + + h := handlers.NewBrevoTransactionalWebhookHandler(db, &config.Config{BrevoWebhookSecret: testBrevoTxSecret}) + app := brevoTxApp(t, h) + + body := `{"event":"delivered","email":"u@example.com","message-id":"msg-bounced"}` + resp := postBrevoTx(t, app, testBrevoTxSecret, body) + if resp.StatusCode != http.StatusOK { + t.Fatalf("status = %d; want 200", resp.StatusCode) + } + raw, _ := io.ReadAll(resp.Body) + if !strings.Contains(string(raw), `"matched":true`) { + t.Errorf("delivered-after-bounce must be matched:true (known message, terminal class kept); got %s", string(raw)) + } + if err := mock.ExpectationsWereMet(); err != nil { + t.Errorf("unmet sqlmock expectations: %v", err) + } +} + // ── 10. Coverage test (CLAUDE.md rule 18): every documented Brevo event // has a handler. This iterates the live registry. A new event // added to brevoDocumentedEvents but not brevoEventHandlers fails diff --git a/internal/handlers/cache.go b/internal/handlers/cache.go index 0ec2a3f2..08148c7a 100644 --- a/internal/handlers/cache.go +++ b/internal/handlers/cache.go @@ -345,7 +345,7 @@ func (h *CacheHandler) newCacheAuthenticated( Fingerprint: fp, CloudVendor: vendor, CountryCode: country, - ExpiresAt: nil, + ExpiresAt: resourceExpiryForTier(tier), // free→24h TTL, paid→permanent (bug bash #4) CreatedRequestID: requestID, ParentResourceID: parentRootID, }) @@ -522,7 +522,7 @@ func (h *CacheHandler) ProvisionForTwinCore(ctx context.Context, in ProvisionFor Fingerprint: in.Fingerprint, CloudVendor: in.CloudVendor, CountryCode: in.CountryCode, - ExpiresAt: nil, + ExpiresAt: resourceExpiryForTier(in.Tier), // free→24h TTL, paid→permanent (bug bash #4) CreatedRequestID: in.RequestID, ParentResourceID: in.ParentRootID, }) diff --git a/internal/handlers/db.go b/internal/handlers/db.go index 1e81442c..f919c30a 100644 --- a/internal/handlers/db.go +++ b/internal/handlers/db.go @@ -396,7 +396,7 @@ func (h *DBHandler) newDBAuthenticated( Fingerprint: fp, CloudVendor: vendor, CountryCode: country, - ExpiresAt: nil, // permanent + ExpiresAt: resourceExpiryForTier(tier), // free→24h TTL, paid→permanent (bug bash #4) CreatedRequestID: requestID, ParentResourceID: parentRootID, }) @@ -606,7 +606,7 @@ func (h *DBHandler) ProvisionForTwinCore(ctx context.Context, in ProvisionForTwi Fingerprint: in.Fingerprint, CloudVendor: in.CloudVendor, CountryCode: in.CountryCode, - ExpiresAt: nil, // permanent — twin inherits source's no-TTL status + ExpiresAt: resourceExpiryForTier(in.Tier), // free→24h TTL, paid→permanent (bug bash #4) CreatedRequestID: in.RequestID, ParentResourceID: in.ParentRootID, }) diff --git a/internal/handlers/deploy.go b/internal/handlers/deploy.go index f06dfbdc..aad2bd1d 100644 --- a/internal/handlers/deploy.go +++ b/internal/handlers/deploy.go @@ -1425,7 +1425,10 @@ func (h *DeployHandler) CancelDelete(c *fiber.Ctx) error { return respondError(c, fiber.StatusServiceUnavailable, "fetch_failed", "Failed to fetch deployment") } if d.TeamID != team.ID { - return respondError(c, fiber.StatusForbidden, "forbidden", "You do not own this deployment") + // 404, not 403 (bug bash #22): a cross-tenant 403 confirms the + // deployment EXISTS, leaking its existence to a non-owner. Mirror the + // not-found posture used by the other deploy endpoints. + return respondError(c, fiber.StatusNotFound, "not_found", "Deployment not found") } deps := requestDeletionDeps{ diff --git a/internal/handlers/deploy_stack_promote_approval_coverage_test.go b/internal/handlers/deploy_stack_promote_approval_coverage_test.go index 7d60e208..c65ae90d 100644 --- a/internal/handlers/deploy_stack_promote_approval_coverage_test.go +++ b/internal/handlers/deploy_stack_promote_approval_coverage_test.go @@ -456,7 +456,7 @@ func TestDeployDelete_PaidTier_QueuesPendingConfirmation(t *testing.T) { assert.Contains(t, []int{http.StatusAccepted, http.StatusOK}, resp.StatusCode) } -func TestDeployCancelDelete_CrossTeam_Returns403(t *testing.T) { +func TestDeployCancelDelete_CrossTeam_Returns404(t *testing.T) { requireTestDB(t) db, cleanDB := testhelpers.SetupTestDB(t) defer cleanDB() @@ -477,7 +477,9 @@ func TestDeployCancelDelete_CrossTeam_Returns403(t *testing.T) { resp, err := app.Test(req, 10000) require.NoError(t, err) defer resp.Body.Close() - assert.Equal(t, http.StatusForbidden, resp.StatusCode) + // bug bash #22: cross-tenant access returns 404 (not 403) so a non-owner + // can't confirm the deployment exists. + assert.Equal(t, http.StatusNotFound, resp.StatusCode) } func TestDeployCancelDelete_UnknownID_Returns404(t *testing.T) { diff --git a/internal/handlers/nosql.go b/internal/handlers/nosql.go index 5bee61f9..78165eba 100644 --- a/internal/handlers/nosql.go +++ b/internal/handlers/nosql.go @@ -338,7 +338,7 @@ func (h *NoSQLHandler) newNoSQLAuthenticated( Fingerprint: fp, CloudVendor: vendor, CountryCode: country, - ExpiresAt: nil, + ExpiresAt: resourceExpiryForTier(tier), // free→24h TTL, paid→permanent (bug bash #4) CreatedRequestID: requestID, ParentResourceID: parentRootID, }) @@ -527,7 +527,7 @@ func (h *NoSQLHandler) ProvisionForTwinCore(ctx context.Context, in ProvisionFor Fingerprint: in.Fingerprint, CloudVendor: in.CloudVendor, CountryCode: in.CountryCode, - ExpiresAt: nil, + ExpiresAt: resourceExpiryForTier(in.Tier), // free→24h TTL, paid→permanent (bug bash #4) CreatedRequestID: in.RequestID, ParentResourceID: in.ParentRootID, }) diff --git a/internal/handlers/queue.go b/internal/handlers/queue.go index 932ef5b3..eb71dbdf 100644 --- a/internal/handlers/queue.go +++ b/internal/handlers/queue.go @@ -488,7 +488,7 @@ func (h *QueueHandler) newQueueAuthenticated( Fingerprint: fp, CloudVendor: vendor, CountryCode: country, - ExpiresAt: nil, + ExpiresAt: resourceExpiryForTier(tier), // free→24h TTL, paid→permanent (bug bash #4) CreatedRequestID: requestID, }) if err != nil { diff --git a/internal/handlers/resource_ttl.go b/internal/handlers/resource_ttl.go new file mode 100644 index 00000000..566a2214 --- /dev/null +++ b/internal/handlers/resource_ttl.go @@ -0,0 +1,34 @@ +package handlers + +import "time" + +// ephemeralTierTTL is the lifetime of a newly-provisioned resource on an +// ephemeral (unpaid) tier. Per api/plans.yaml, anonymous and free are both +// "claimed-unpaid, 24h TTL, pay-from-day-one" — they share anonymous's exact +// limits and TTL; the only difference is audience. +const ephemeralTierTTL = 24 * time.Hour + +// ephemeralTiers is the set of tiers whose resources auto-expire. Everything +// else (hobby, hobby_plus, pro, growth, team) is a paid tier whose resources +// are permanent until explicitly deleted. +var ephemeralTiers = map[string]bool{ + "anonymous": true, + "free": true, +} + +// resourceExpiryForTier returns the expires_at to stamp on a newly-provisioned +// resource for the given tier: now()+ephemeralTierTTL for an ephemeral tier, +// nil (permanent) for a paid tier. +// +// Centralizes the TTL policy so every authenticated provision handler applies +// it identically. Closes bug bash 2026-06-02 #4: authenticated free-tier +// provisions hardcoded ExpiresAt=nil, so claimed-unpaid resources never +// expired despite plans.yaml's documented 24h TTL — letting a free user keep +// resources indefinitely without paying. Paid tiers are unaffected (nil). +func resourceExpiryForTier(tier string) *time.Time { + if !ephemeralTiers[tier] { + return nil + } + t := time.Now().UTC().Add(ephemeralTierTTL) + return &t +} diff --git a/internal/handlers/resource_ttl_test.go b/internal/handlers/resource_ttl_test.go new file mode 100644 index 00000000..8ebb5ea2 --- /dev/null +++ b/internal/handlers/resource_ttl_test.go @@ -0,0 +1,27 @@ +package handlers + +import ( + "testing" + "time" +) + +func TestResourceExpiryForTier(t *testing.T) { + now := time.Now().UTC() + // Ephemeral tiers get a ~24h expiry. + for _, tier := range []string{"anonymous", "free"} { + got := resourceExpiryForTier(tier) + if got == nil { + t.Fatalf("%s: expected non-nil expiry (24h TTL), got nil", tier) + } + d := got.Sub(now) + if d < 23*time.Hour || d > 25*time.Hour { + t.Errorf("%s: expiry %v from now; want ~24h", tier, d) + } + } + // Paid tiers are permanent (nil). + for _, tier := range []string{"hobby", "hobby_plus", "pro", "growth", "team"} { + if got := resourceExpiryForTier(tier); got != nil { + t.Errorf("%s: expected nil (permanent), got %v", tier, *got) + } + } +} diff --git a/internal/handlers/storage.go b/internal/handlers/storage.go index bc1a7c7f..19468093 100644 --- a/internal/handlers/storage.go +++ b/internal/handlers/storage.go @@ -483,7 +483,7 @@ func (h *StorageHandler) newStorageAuthenticated( Fingerprint: fp, CloudVendor: vendor, CountryCode: country, - ExpiresAt: nil, + ExpiresAt: resourceExpiryForTier(team.PlanTier), // free→24h TTL, paid→permanent (bug bash #4) CreatedRequestID: requestID, }) if err != nil { diff --git a/internal/handlers/vector.go b/internal/handlers/vector.go index 277f6050..17fe1799 100644 --- a/internal/handlers/vector.go +++ b/internal/handlers/vector.go @@ -481,7 +481,7 @@ func (h *VectorHandler) newVectorAuthenticated( Fingerprint: fp, CloudVendor: vendor, CountryCode: country, - ExpiresAt: nil, // permanent + ExpiresAt: resourceExpiryForTier(tier), // free→24h TTL, paid→permanent (bug bash #4) CreatedRequestID: requestID, ParentResourceID: parentRootID, }) diff --git a/internal/handlers/webhook.go b/internal/handlers/webhook.go index 709824cb..e6baf296 100644 --- a/internal/handlers/webhook.go +++ b/internal/handlers/webhook.go @@ -422,7 +422,7 @@ func (h *WebhookHandler) newWebhookAuthenticated( Fingerprint: fp, CloudVendor: vendor, CountryCode: country, - ExpiresAt: nil, + ExpiresAt: resourceExpiryForTier(team.PlanTier), // free→24h TTL, paid→permanent (bug bash #4) CreatedRequestID: requestID, }) if err != nil { diff --git a/internal/models/resource.go b/internal/models/resource.go index d56b3739..a816ebe2 100644 --- a/internal/models/resource.go +++ b/internal/models/resource.go @@ -320,6 +320,7 @@ func GetActiveResourceByFingerprintType(ctx context.Context, db *sql.DB, fingerp AND resource_type = $2 AND env = $3 AND status = 'active' + AND (expires_at IS NULL OR expires_at > NOW()) ORDER BY created_at DESC LIMIT 1 `, fingerprint, resourceType, env) @@ -351,6 +352,7 @@ func GetActiveResourceByFingerprint(ctx context.Context, db *sql.DB, fingerprint AND team_id IS NULL AND env = $2 AND status = 'active' + AND (expires_at IS NULL OR expires_at > NOW()) ORDER BY created_at DESC LIMIT 1 `, fingerprint, env) diff --git a/plans.yaml b/plans.yaml index acbf9f50..3fe7fd78 100644 --- a/plans.yaml +++ b/plans.yaml @@ -425,6 +425,10 @@ plans: alerts: true custom_domains: true sla: true + # Team sits above Growth ($199 vs $99); it gets dedicated infra too + # (bug bash 2026-06-02 #12). Without this, IsDedicatedTier(team)=false + # denied the top tier isolated Postgres/Redis/Mongo. + dedicated: true # team_yearly — $1990/yr ≈ $165.83/mo (~17% off $199 x 12). team_yearly: @@ -465,6 +469,7 @@ plans: alerts: true custom_domains: true sla: true + dedicated: true # mirrors team (bug bash #12) # growth — $99/mo. Sits between Pro ($49) and Team ($199) as the # "unlimited supporting services" step for teams whose Postgres/Redis