Skip to content

Commit 3f0f87c

Browse files
fix(deploy): guard SetTTL against permanent + terminal deploys (bug-bash #3/#5/#6) (#228)
SetTTL unconditionally overwrote ttl_policy/expires_at, so a user could flip a deployment they'd made permanent back to an expiring TTL, and could set a TTL on an expired/deleted/stopped deploy. Both violate the lifecycle contract. - handler: reject ttl_policy=='permanent' with 409 already_permanent and a terminal status with 409 invalid_state, before the write. - model: SetDeploymentTTL UPDATE now carries `AND ttl_policy != 'permanent'` — defense-in-depth so a MakePermanent racing the UPDATE can't silently un-permanent the deploy (the row just isn't touched). - agent_action entry for already_permanent. Tests (DB-gated): make-permanent→SetTTL→409 (row stays permanent), terminal deploy→SetTTL→409, and the model WHERE-guard leaves a permanent row untouched on a direct call. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 0e107ad commit 3f0f87c

4 files changed

Lines changed: 123 additions & 1 deletion

File tree

internal/handlers/deploy_ttl.go

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,19 @@ func (h *DeployHandler) SetTTL(c *fiber.Ctx) error {
143143
return respondError(c, fiber.StatusNotFound, "not_found", "Deployment not found")
144144
}
145145

146+
// Bug-bash #3/#5/#6: a permanent deploy must not be silently flipped back to
147+
// an expiring TTL, and a terminal (expired/deleted/stopped) deploy can't have
148+
// its lifecycle changed. Guard both before the write (the model adds a
149+
// defense-in-depth WHERE clause for the permanent case too).
150+
if d.TTLPolicy == models.DeployTTLPolicyPermanent {
151+
return respondError(c, fiber.StatusConflict, "already_permanent",
152+
"This deployment is permanent — it has no TTL to set. (Downgrading a permanent deploy is support-only.)")
153+
}
154+
if models.IsDeploymentTerminal(d.Status) {
155+
return respondError(c, fiber.StatusConflict, "invalid_state",
156+
fmt.Sprintf("Cannot set a TTL on a %s deployment", d.Status))
157+
}
158+
146159
if team.PlanTier == "anonymous" {
147160
// B7-P1-7 (see MakePermanent above): emit `claim_required` so an
148161
// agent branching on error code routes the user to the free claim
Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,102 @@
1+
package handlers_test
2+
3+
// deploy_ttl_guards_test.go — bug-bash #3/#5/#6: SetTTL must refuse to flip a
4+
// permanent deploy back to an expiring TTL (409 already_permanent) and refuse a
5+
// terminal deploy (409 invalid_state); the model's WHERE-guard is the
6+
// defense-in-depth backstop for the permanent case. Reuses the
7+
// seedDeploy/patchEnvApp/requireCoverageDB harness in deploy_stack_coverage_test.go.
8+
9+
import (
10+
"context"
11+
"database/sql"
12+
"net/http"
13+
"net/http/httptest"
14+
"strings"
15+
"testing"
16+
17+
"github.com/google/uuid"
18+
"github.com/stretchr/testify/assert"
19+
"github.com/stretchr/testify/require"
20+
21+
"instant.dev/internal/models"
22+
"instant.dev/internal/testhelpers"
23+
)
24+
25+
// TestSetTTL_PermanentRejected: make a deploy permanent, then SetTTL → 409
26+
// already_permanent, and the row stays permanent (expires_at NULL).
27+
func TestSetTTL_PermanentRejected(t *testing.T) {
28+
requireCoverageDB(t)
29+
db, cleanDB := testhelpers.SetupTestDB(t)
30+
defer cleanDB()
31+
teamIDStr := testhelpers.MustCreateTeamDB(t, db, "pro")
32+
teamID := uuid.MustParse(teamIDStr)
33+
deployID, _ := seedDeploy(t, db, teamID, "healthy", "pro")
34+
jwt := testhelpers.MustSignSessionJWT(t, uuid.NewString(), teamIDStr, "ttlperm@example.com")
35+
app, _ := patchEnvApp(t, db)
36+
37+
// Make it permanent first.
38+
mp := httptest.NewRequest(http.MethodPost, "/api/v1/deployments/"+deployID.String()+"/make-permanent", nil)
39+
mp.Header.Set("Authorization", "Bearer "+jwt)
40+
mpResp, err := app.Test(mp, 5000)
41+
require.NoError(t, err)
42+
require.Equal(t, http.StatusOK, mpResp.StatusCode)
43+
_ = mpResp.Body.Close()
44+
45+
// SetTTL must now 409.
46+
req := httptest.NewRequest(http.MethodPost, "/api/v1/deployments/"+deployID.String()+"/ttl",
47+
strings.NewReader(`{"hours":48}`))
48+
req.Header.Set("Authorization", "Bearer "+jwt)
49+
req.Header.Set("Content-Type", "application/json")
50+
resp, err := app.Test(req, 5000)
51+
require.NoError(t, err)
52+
defer resp.Body.Close()
53+
assert.Equal(t, http.StatusConflict, resp.StatusCode, "SetTTL on a permanent deploy must 409")
54+
55+
// Row stayed permanent.
56+
var policy string
57+
var expiresAt sql.NullTime
58+
require.NoError(t, db.QueryRow(`SELECT ttl_policy, expires_at FROM deployments WHERE id=$1`, deployID).Scan(&policy, &expiresAt))
59+
assert.Equal(t, "permanent", policy)
60+
assert.False(t, expiresAt.Valid, "expires_at must stay NULL")
61+
}
62+
63+
// TestSetTTL_TerminalRejected: a terminal (expired) deploy → 409 invalid_state.
64+
func TestSetTTL_TerminalRejected(t *testing.T) {
65+
requireCoverageDB(t)
66+
db, cleanDB := testhelpers.SetupTestDB(t)
67+
defer cleanDB()
68+
teamIDStr := testhelpers.MustCreateTeamDB(t, db, "pro")
69+
teamID := uuid.MustParse(teamIDStr)
70+
deployID, _ := seedDeploy(t, db, teamID, "expired", "pro")
71+
jwt := testhelpers.MustSignSessionJWT(t, uuid.NewString(), teamIDStr, "ttlterm@example.com")
72+
app, _ := patchEnvApp(t, db)
73+
74+
req := httptest.NewRequest(http.MethodPost, "/api/v1/deployments/"+deployID.String()+"/ttl",
75+
strings.NewReader(`{"hours":48}`))
76+
req.Header.Set("Authorization", "Bearer "+jwt)
77+
req.Header.Set("Content-Type", "application/json")
78+
resp, err := app.Test(req, 5000)
79+
require.NoError(t, err)
80+
defer resp.Body.Close()
81+
assert.Equal(t, http.StatusConflict, resp.StatusCode, "SetTTL on a terminal deploy must 409")
82+
}
83+
84+
// TestSetDeploymentTTL_PermanentGuard: the model WHERE-guard leaves a permanent
85+
// row untouched even if called directly (the race backstop, #6).
86+
func TestSetDeploymentTTL_PermanentGuard(t *testing.T) {
87+
requireCoverageDB(t)
88+
db, cleanDB := testhelpers.SetupTestDB(t)
89+
defer cleanDB()
90+
teamID := uuid.MustParse(testhelpers.MustCreateTeamDB(t, db, "pro"))
91+
deployID, _ := seedDeploy(t, db, teamID, "healthy", "pro")
92+
_, err := db.Exec(`UPDATE deployments SET ttl_policy='permanent', expires_at=NULL WHERE id=$1`, deployID)
93+
require.NoError(t, err)
94+
95+
// Direct model call must NOT flip the permanent row.
96+
require.NoError(t, models.SetDeploymentTTL(context.Background(), db, deployID, 24))
97+
var policy string
98+
var expiresAt sql.NullTime
99+
require.NoError(t, db.QueryRow(`SELECT ttl_policy, expires_at FROM deployments WHERE id=$1`, deployID).Scan(&policy, &expiresAt))
100+
assert.Equal(t, "permanent", policy, "WHERE guard must leave a permanent deploy untouched")
101+
assert.False(t, expiresAt.Valid)
102+
}

internal/handlers/helpers.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -516,6 +516,9 @@ var codeToAgentAction = map[string]errorCodeMeta{
516516
"install_conflict": {
517517
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.",
518518
},
519+
"already_permanent": {
520+
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.",
521+
},
519522
"state_unavailable": {
520523
AgentAction: "Tell the user the GitHub install could not be started due to a transient backend error. Retry shortly — see https://instanode.dev/status.",
521524
},

internal/models/deployment.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -740,14 +740,18 @@ func ElevateDeploymentTiersByTeam(ctx context.Context, db *sql.DB, teamID uuid.U
740740
// warning cycle again instead of skipping reminders that fired earlier.
741741
func SetDeploymentTTL(ctx context.Context, db *sql.DB, id uuid.UUID, hours int) error {
742742
expiresAt := time.Now().UTC().Add(time.Duration(hours) * time.Hour)
743+
// `ttl_policy != 'permanent'` is defense-in-depth (the handler already
744+
// rejects a permanent deploy with 409): if a concurrent MakePermanent races
745+
// this UPDATE, the WHERE clause prevents silently un-permanenting the deploy
746+
// — the row simply isn't touched (bug-bash #6).
743747
_, err := db.ExecContext(ctx, `
744748
UPDATE deployments
745749
SET expires_at = $1,
746750
ttl_policy = 'custom',
747751
reminders_sent = 0,
748752
last_reminder_at = NULL,
749753
updated_at = now()
750-
WHERE id = $2
754+
WHERE id = $2 AND ttl_policy != 'permanent'
751755
`, expiresAt, id)
752756
if err != nil {
753757
return fmt.Errorf("models.SetDeploymentTTL: %w", err)

0 commit comments

Comments
 (0)