Allow configuring disk sizes per template en 1038 v2#3288
Conversation
Separate the default template free-space target from the maximum total root filesystem size that a team may build. Add nullable default_free_disk_size_mb and max_disk_size_mb columns to tiers, plus extra_max_disk_size_mb to add-ons. Keep extra_disk_mb as the add-on contribution to default free space and add constraints preventing invalid negative or inverted tier values. Keep the existing team_limits view unchanged for current consumers and introduce team_limits_v2 with the new effective default and maximum fields. Preserve compatibility with legacy add-on writers by treating a null extra maximum as equal to extra_disk_mb. Leave data population and runtime adoption to later rollout steps so this commit remains an expand-only schema change. Regenerate the database models and add focused migration coverage for the new columns, the unchanged V1 view, the V2 contract, security invoker behavior, and effective tier-plus-add-on calculations. Part of: EN-1038
Read template disk defaults and maximums from team_limits_v2, and use the new default when registering builds. Propagate the build-start maximum through the internal TemplateCreate RPC for both build endpoints. At build completion, compare the exact final rootfs size with the maximum, emit disk-limit telemetry, and reject oversized builds when build-enforce-max-disk-size is enabled. Keep missing or invalid maximums fail-open and document the updated build flow.
❌ 16 Tests Failed:
View the top 3 failed test(s) by shortest run time
To view more test analytics, go to the Test Analytics Dashboard |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| (tier.max_ram_mb + a.extra_max_ram_mb) AS max_ram_mb, | ||
| (tier.disk_mb + a.extra_disk_mb) AS disk_mb, | ||
| (tier.events_ttl_days + a.extra_events_ttl_days) AS events_ttl_days, | ||
| (tier.default_free_disk_size_mb + a.extra_disk_mb)::bigint AS default_free_disk_size_mb, |
There was a problem hiding this comment.
🔒 Agentic Security Review
Severity: HIGH
team_limits_v2.default_free_disk_size_mb is computed from nullable tiers.default_free_disk_size_mb (NULL + extra_disk_mb stays NULL), while downstream auth queries scan this column into non-null int64 fields. For teams on existing tier rows without a backfill/default, that scan can fail and break team resolution on authenticated request paths.
Impact: authenticated traffic for affected teams can fail in auth/team-loading paths (availability/DoS) until data is backfilled or the view expression is made NULL-safe.
Reviewed by Cursor Security Reviewer for commit c1406f8. Configure here.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces team-level disk entitlements and maximum disk size limits for template builds. It adds database migrations for new columns on tiers and addons, creates a new team_limits_v2 view, and updates the orchestrator and template-manager to enforce the resolved maximum disk size limit during builds under a new feature flag. The review feedback highlights a critical issue where the generated database model fields DefaultFreeDiskSizeMb and MaxDiskSizeMb are typed as int64 instead of *int64, which will cause runtime database scan errors when encountering NULL values. Additionally, the feedback suggests safely dereferencing these fields in newTeamLimits to prevent potential nil-pointer panics.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| DefaultFreeDiskSizeMb int64 | ||
| MaxDiskSizeMb int64 |
There was a problem hiding this comment.
The fields DefaultFreeDiskSizeMb and MaxDiskSizeMb are generated as int64 instead of *int64. Since the corresponding database columns default_free_disk_size_mb and max_disk_size_mb are nullable and do not have default values, fetching any team on an existing tier (where these columns are NULL) will result in a runtime database scan error: sql: Scan error on column index ..., name ...: converting NULL to int64 is unsupported.
Please update these fields to be pointers (*int64) to safely handle NULL values from the database.
| DefaultFreeDiskSizeMb int64 | |
| MaxDiskSizeMb int64 | |
| DefaultFreeDiskSizeMb *int64 | |
| MaxDiskSizeMb *int64 |
| return &TeamLimits{ | ||
| SandboxConcurrency: int64(teamLimits.ConcurrentSandboxes), | ||
| BuildConcurrency: int64(teamLimits.ConcurrentTemplateBuilds), | ||
| MaxLengthHours: teamLimits.MaxLengthHours, | ||
| MaxVcpu: int64(teamLimits.MaxVcpu), | ||
| MaxRamMb: int64(teamLimits.MaxRamMb), | ||
| DiskMb: int64(teamLimits.DiskMb), | ||
| EventsTTLDays: int64(teamLimits.EventsTtlDays), | ||
| SandboxConcurrency: int64(teamLimits.ConcurrentSandboxes), | ||
| BuildConcurrency: int64(teamLimits.ConcurrentTemplateBuilds), | ||
| MaxLengthHours: teamLimits.MaxLengthHours, | ||
| MaxVcpu: int64(teamLimits.MaxVcpu), | ||
| MaxRamMb: int64(teamLimits.MaxRamMb), | ||
| DiskMb: int64(teamLimits.DiskMb), | ||
| DefaultFreeDiskSizeMb: teamLimits.DefaultFreeDiskSizeMb, | ||
| MaxDiskSizeMb: teamLimits.MaxDiskSizeMb, | ||
| EventsTTLDays: int64(teamLimits.EventsTtlDays), | ||
| } |
There was a problem hiding this comment.
To support the nullable DefaultFreeDiskSizeMb and MaxDiskSizeMb fields (which should be pointers *int64 in authqueries.TeamLimitsV2), update newTeamLimits to safely dereference them and default to 0 if they are nil. This prevents potential nil-pointer dereference panics and ensures backward compatibility for existing tiers.
var defaultFreeDiskSizeMb int64
if teamLimits.DefaultFreeDiskSizeMb != nil {
defaultFreeDiskSizeMb = *teamLimits.DefaultFreeDiskSizeMb
}
var maxDiskSizeMb int64
if teamLimits.MaxDiskSizeMb != nil {
maxDiskSizeMb = *teamLimits.MaxDiskSizeMb
}
return &TeamLimits{
SandboxConcurrency: int64(teamLimits.ConcurrentSandboxes),
BuildConcurrency: int64(teamLimits.ConcurrentTemplateBuilds),
MaxLengthHours: teamLimits.MaxLengthHours,
MaxVcpu: int64(teamLimits.MaxVcpu),
MaxRamMb: int64(teamLimits.MaxRamMb),
DiskMb: int64(teamLimits.DiskMb),
DefaultFreeDiskSizeMb: defaultFreeDiskSizeMb,
MaxDiskSizeMb: maxDiskSizeMb,
EventsTTLDays: int64(teamLimits.EventsTtlDays),
}
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| SELECT | ||
| t.id, | ||
| tier.max_length_hours, | ||
| (tier.concurrent_instances + a.extra_concurrent_sandboxes) AS concurrent_sandboxes, | ||
| (tier.concurrent_template_builds + a.extra_concurrent_template_builds) AS concurrent_template_builds, | ||
| (tier.max_vcpu + a.extra_max_vcpu) AS max_vcpu, | ||
| (tier.max_ram_mb + a.extra_max_ram_mb) AS max_ram_mb, | ||
| (tier.disk_mb + a.extra_disk_mb) AS disk_mb, | ||
| (tier.events_ttl_days + a.extra_events_ttl_days) AS events_ttl_days, | ||
| (tier.default_free_disk_size_mb + a.extra_disk_mb)::bigint AS default_free_disk_size_mb, | ||
| (tier.max_disk_size_mb + a.extra_max_disk_size_mb)::bigint AS max_disk_size_mb |
There was a problem hiding this comment.
🔴 Migration 20260714091414 adds tiers.default_free_disk_size_mb and tiers.max_disk_size_mb as nullable bigint columns with no DEFAULT and no backfill for existing rows. Because the team_limits_v2 view (20260714091415) does not COALESCE the tier-side value before adding the addon delta, every pre-existing tier yields NULL for these two columns, and since the auth-path generated model (packages/db/pkg/auth/queries/models.go) types them as non-pointer int64 while get_team.sql.go scans directly into that field, pgx will error scanning NULL into it. This breaks GetTeamWithTierByAPIKey/ByTeamID/ByTeamAndUser/GetTeamsWithUsersTeamsWithTier for every existing team as soon as this migration deploys, taking down API-key auth and team lookups system-wide until every tier row is manually backfilled.
Extended reasoning...
The bug: Migration 20260714091414_add_template_disk_entitlements.sql adds tiers.default_free_disk_size_mb and tiers.max_disk_size_mb as bigint columns with no DEFAULT and no backfill UPDATE for existing rows. The only guardrails are CHECK constraints (>= 0, > 0, <= max), and in SQL a CHECK evaluates to UNKNOWN (and therefore passes) when the operand is NULL. Every tier row that existed before this migration — i.e. every tier in every production/self-hosted deployment — therefore keeps these two columns as NULL after the migration runs. Nothing else in this PR (or in packages/db/migrations/) issues a backfill for them.
Where it manifests: The very next migration, 20260714091415_create_team_limits_v2.sql, defines the team_limits_v2 view as:
(tier.default_free_disk_size_mb + a.extra_disk_mb)::bigint AS default_free_disk_size_mb,
(tier.max_disk_size_mb + a.extra_max_disk_size_mb)::bigint AS max_disk_size_mbThe addon-side operand (a.extra_disk_mb, a.extra_max_disk_size_mb) is wrapped in COALESCE(..., 0) inside the lateral join, but the tier-side operand is not. In SQL, NULL + n = NULL, so for any team on a non-backfilled tier this view produces NULL in both columns — even when the team has zero addons.
Why the generated code can't absorb a NULL: The sqlc.yaml override comment even acknowledges the nullability problem ("sqlc does not infer nullability for these view expressions") and forces these two columns to *int64 — but that override only applies to the testutils sqlc config, not the authqueries config used by the real auth path. Concretely:
packages/db/pkg/testutils/queries/models.go:DefaultFreeDiskSizeMb *int64,MaxDiskSizeMb *int64(correct, pointer).packages/db/pkg/auth/queries/models.go(TeamLimitsV2, used by real auth):DefaultFreeDiskSizeMb int64,MaxDiskSizeMb int64(non-pointer).
And get_team.sql.go scans directly into the non-pointer field in all four generated queries: GetTeamWithTierByAPIKey, GetTeamWithTierByTeamAndUser, GetTeamWithTierByTeamID, GetTeamsWithUsersTeamsWithTier (e.g. &i.TeamLimitsV2.DefaultFreeDiskSizeMb). pgx v5 returns a scan error when the destination is a non-pointer int64 and the column value is SQL NULL.
This non-pointer typing isn't just a stale codegen artifact that a re-run would silently fix either: packages/auth/pkg/types/teams.go's newTeamLimits assigns teamLimits.DefaultFreeDiskSizeMb straight into TeamLimits.DefaultFreeDiskSizeMb (also int64, no conversion), so the code depends on it staying a plain int64 to compile.
Step-by-step proof:
- Deploy runs migration
20260714091414: existing tier rowbase_v1(from migration20231220094836) getsdefault_free_disk_size_mb = NULL,max_disk_size_mb = NULL. TheCHECKconstraints pass because the operands areNULL. - Migration
20260714091415createsteam_limits_v2. For any team onbase_v1, evaluating the view:tier.default_free_disk_size_mbisNULL,a.extra_disk_mbisCOALESCE(SUM(...), 0)=0(no addons) →NULL + 0 = NULL. Same formax_disk_size_mb. - A request hits an authenticated endpoint.
authStoreImpl.GetTeamByHashedAPIKeycallss.authDB.Read.GetTeamWithTierByAPIKey(ctx, hashedKey), which runs the generated query and executesrow.Scan(..., &i.TeamLimitsV2.DefaultFreeDiskSizeMb, &i.TeamLimitsV2.MaxDiskSizeMb). - pgx tries to write SQL
NULLinto a non-pointerint64destination and returns an error (e.g. "cannot scan NULL into *int64" is fine, but this is a non-pointer int64, which errors as an invalid Scan target). The error propagates up throughGetTeamByHashedAPIKey→fmt.Errorf("failed to get team from API key: %w", err). - Every API-key-authenticated request for that team now fails at the auth layer. Because there is no backfill, this affects every pre-existing team as soon as the migration deploys — not just an edge case.
Why this isn't caught by the PR's own test: disk_entitlements_migration_test.go inserts a brand-new tier row with default_free_disk_size_mb and max_disk_size_mb explicitly populated (8000/30000), so it never exercises the NULL/pre-existing-tier path that production will hit.
Fix: Either (a) add COALESCE(tier.default_free_disk_size_mb, <sentinel>) / COALESCE(tier.max_disk_size_mb, <sentinel>) in the view (matching whatever sentinel makes sense, e.g. falling back to the legacy disk_mb), or (b) add a backfill UPDATE ... WHERE default_free_disk_size_mb IS NULL (and same for max_disk_size_mb) followed by SET NOT NULL, consistent with how prior tier-column additions in this repo (e.g. migration 20240219190940) handle backfill. Either fix should be paired with correcting the authqueries sqlc override so the Go type reflects the DB's actual nullability guarantee.
1 participant
No description provided.