diff --git a/internal/handlers/deploy_ttl.go b/internal/handlers/deploy_ttl.go index 691ba59b..6341c58c 100644 --- a/internal/handlers/deploy_ttl.go +++ b/internal/handlers/deploy_ttl.go @@ -143,6 +143,19 @@ func (h *DeployHandler) SetTTL(c *fiber.Ctx) error { return respondError(c, fiber.StatusNotFound, "not_found", "Deployment not found") } + // Bug-bash #3/#5/#6: a permanent deploy must not be silently flipped back to + // an expiring TTL, and a terminal (expired/deleted/stopped) deploy can't have + // its lifecycle changed. Guard both before the write (the model adds a + // defense-in-depth WHERE clause for the permanent case too). + if d.TTLPolicy == models.DeployTTLPolicyPermanent { + return respondError(c, fiber.StatusConflict, "already_permanent", + "This deployment is permanent — it has no TTL to set. (Downgrading a permanent deploy is support-only.)") + } + if models.IsDeploymentTerminal(d.Status) { + return respondError(c, fiber.StatusConflict, "invalid_state", + fmt.Sprintf("Cannot set a TTL on a %s deployment", d.Status)) + } + if team.PlanTier == "anonymous" { // B7-P1-7 (see MakePermanent above): emit `claim_required` so an // agent branching on error code routes the user to the free claim diff --git a/internal/handlers/deploy_ttl_guards_test.go b/internal/handlers/deploy_ttl_guards_test.go new file mode 100644 index 00000000..96e279c1 --- /dev/null +++ b/internal/handlers/deploy_ttl_guards_test.go @@ -0,0 +1,102 @@ +package handlers_test + +// deploy_ttl_guards_test.go — bug-bash #3/#5/#6: SetTTL must refuse to flip a +// permanent deploy back to an expiring TTL (409 already_permanent) and refuse a +// terminal deploy (409 invalid_state); the model's WHERE-guard is the +// defense-in-depth backstop for the permanent case. Reuses the +// seedDeploy/patchEnvApp/requireCoverageDB harness in deploy_stack_coverage_test.go. + +import ( + "context" + "database/sql" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/google/uuid" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "instant.dev/internal/models" + "instant.dev/internal/testhelpers" +) + +// TestSetTTL_PermanentRejected: make a deploy permanent, then SetTTL → 409 +// already_permanent, and the row stays permanent (expires_at NULL). +func TestSetTTL_PermanentRejected(t *testing.T) { + requireCoverageDB(t) + db, cleanDB := testhelpers.SetupTestDB(t) + defer cleanDB() + teamIDStr := testhelpers.MustCreateTeamDB(t, db, "pro") + teamID := uuid.MustParse(teamIDStr) + deployID, _ := seedDeploy(t, db, teamID, "healthy", "pro") + jwt := testhelpers.MustSignSessionJWT(t, uuid.NewString(), teamIDStr, "ttlperm@example.com") + app, _ := patchEnvApp(t, db) + + // Make it permanent first. + mp := httptest.NewRequest(http.MethodPost, "/api/v1/deployments/"+deployID.String()+"/make-permanent", nil) + mp.Header.Set("Authorization", "Bearer "+jwt) + mpResp, err := app.Test(mp, 5000) + require.NoError(t, err) + require.Equal(t, http.StatusOK, mpResp.StatusCode) + _ = mpResp.Body.Close() + + // SetTTL must now 409. + req := httptest.NewRequest(http.MethodPost, "/api/v1/deployments/"+deployID.String()+"/ttl", + strings.NewReader(`{"hours":48}`)) + req.Header.Set("Authorization", "Bearer "+jwt) + req.Header.Set("Content-Type", "application/json") + resp, err := app.Test(req, 5000) + require.NoError(t, err) + defer resp.Body.Close() + assert.Equal(t, http.StatusConflict, resp.StatusCode, "SetTTL on a permanent deploy must 409") + + // Row stayed permanent. + var policy string + var expiresAt sql.NullTime + require.NoError(t, db.QueryRow(`SELECT ttl_policy, expires_at FROM deployments WHERE id=$1`, deployID).Scan(&policy, &expiresAt)) + assert.Equal(t, "permanent", policy) + assert.False(t, expiresAt.Valid, "expires_at must stay NULL") +} + +// TestSetTTL_TerminalRejected: a terminal (expired) deploy → 409 invalid_state. +func TestSetTTL_TerminalRejected(t *testing.T) { + requireCoverageDB(t) + db, cleanDB := testhelpers.SetupTestDB(t) + defer cleanDB() + teamIDStr := testhelpers.MustCreateTeamDB(t, db, "pro") + teamID := uuid.MustParse(teamIDStr) + deployID, _ := seedDeploy(t, db, teamID, "expired", "pro") + jwt := testhelpers.MustSignSessionJWT(t, uuid.NewString(), teamIDStr, "ttlterm@example.com") + app, _ := patchEnvApp(t, db) + + req := httptest.NewRequest(http.MethodPost, "/api/v1/deployments/"+deployID.String()+"/ttl", + strings.NewReader(`{"hours":48}`)) + req.Header.Set("Authorization", "Bearer "+jwt) + req.Header.Set("Content-Type", "application/json") + resp, err := app.Test(req, 5000) + require.NoError(t, err) + defer resp.Body.Close() + assert.Equal(t, http.StatusConflict, resp.StatusCode, "SetTTL on a terminal deploy must 409") +} + +// TestSetDeploymentTTL_PermanentGuard: the model WHERE-guard leaves a permanent +// row untouched even if called directly (the race backstop, #6). +func TestSetDeploymentTTL_PermanentGuard(t *testing.T) { + requireCoverageDB(t) + db, cleanDB := testhelpers.SetupTestDB(t) + defer cleanDB() + teamID := uuid.MustParse(testhelpers.MustCreateTeamDB(t, db, "pro")) + deployID, _ := seedDeploy(t, db, teamID, "healthy", "pro") + _, err := db.Exec(`UPDATE deployments SET ttl_policy='permanent', expires_at=NULL WHERE id=$1`, deployID) + require.NoError(t, err) + + // Direct model call must NOT flip the permanent row. + require.NoError(t, models.SetDeploymentTTL(context.Background(), db, deployID, 24)) + var policy string + var expiresAt sql.NullTime + require.NoError(t, db.QueryRow(`SELECT ttl_policy, expires_at FROM deployments WHERE id=$1`, deployID).Scan(&policy, &expiresAt)) + assert.Equal(t, "permanent", policy, "WHERE guard must leave a permanent deploy untouched") + assert.False(t, expiresAt.Valid) +} diff --git a/internal/handlers/helpers.go b/internal/handlers/helpers.go index 40aae9f5..ab12c21b 100644 --- a/internal/handlers/helpers.go +++ b/internal/handlers/helpers.go @@ -516,6 +516,9 @@ var codeToAgentAction = map[string]errorCodeMeta{ "install_conflict": { AgentAction: "Tell the user this GitHub installation is already linked to a different InstaNode team. Uninstall the InstaNode app from GitHub and re-install under the intended team, or contact support to move it — see https://instanode.dev/docs/deploy.", }, + "already_permanent": { + AgentAction: "Tell the user this deployment is already permanent and has no TTL to set. Downgrading a permanent deploy to an expiring one is support-only — see https://instanode.dev/docs/deploy.", + }, "state_unavailable": { AgentAction: "Tell the user the GitHub install could not be started due to a transient backend error. Retry shortly — see https://instanode.dev/status.", }, diff --git a/internal/models/deployment.go b/internal/models/deployment.go index 4a5bb466..87b97fab 100644 --- a/internal/models/deployment.go +++ b/internal/models/deployment.go @@ -740,6 +740,10 @@ func ElevateDeploymentTiersByTeam(ctx context.Context, db *sql.DB, teamID uuid.U // warning cycle again instead of skipping reminders that fired earlier. func SetDeploymentTTL(ctx context.Context, db *sql.DB, id uuid.UUID, hours int) error { expiresAt := time.Now().UTC().Add(time.Duration(hours) * time.Hour) + // `ttl_policy != 'permanent'` is defense-in-depth (the handler already + // rejects a permanent deploy with 409): if a concurrent MakePermanent races + // this UPDATE, the WHERE clause prevents silently un-permanenting the deploy + // — the row simply isn't touched (bug-bash #6). _, err := db.ExecContext(ctx, ` UPDATE deployments SET expires_at = $1, @@ -747,7 +751,7 @@ func SetDeploymentTTL(ctx context.Context, db *sql.DB, id uuid.UUID, hours int) reminders_sent = 0, last_reminder_at = NULL, updated_at = now() - WHERE id = $2 + WHERE id = $2 AND ttl_policy != 'permanent' `, expiresAt, id) if err != nil { return fmt.Errorf("models.SetDeploymentTTL: %w", err)