Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions internal/dispatch/gcf/provisioner.go
Original file line number Diff line number Diff line change
Expand Up @@ -410,6 +410,26 @@ func (p *Provisioner) EnsureOrgInMint(ctx context.Context, expectedURL string, o
}

allowedOrgs := trafficEnvVars["ALLOWED_ORGS"]

// Defense-in-depth: if ALLOWED_ORGS is empty but ROLE_APP_IDS has
// role-only entries, the mint has been bootstrapped and should have
// enrolled orgs. Empty ALLOWED_ORGS in this state indicates env var
// data loss (e.g., stale read from a diverged revision), not a first

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[low] error-message-consistency

Error message uses multi-line string concatenation with '+' operator, inconsistent with other fmt.Errorf calls in this file which use single-line format strings.

Suggested fix: Consolidate into a single-line format string.

// enrollment. Abort to prevent silently unenrolling all existing orgs.
if allowedOrgs == "" {
var roleAppIDMap map[string]string

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[medium] fail-open

json.Unmarshal error is silently discarded. If ROLE_APP_IDS contains malformed JSON, roleAppIDMap stays nil, RoleOnlyAppIDs returns nil, and the guard is bypassed. This is a fail-open on parse failure in a guard designed to detect data corruption.

Suggested fix: Check the error from json.Unmarshal. If ROLE_APP_IDS is non-empty but fails to parse, return an error (fail closed).

if raw := trafficEnvVars["ROLE_APP_IDS"]; raw != "" {
_ = json.Unmarshal([]byte(raw), &roleAppIDMap)
}
roleOnly := mintcore.RoleOnlyAppIDs(roleAppIDMap)
if len(roleOnly) > 0 {
return fmt.Errorf(
"data inconsistency: ALLOWED_ORGS is empty but ROLE_APP_IDS has %d configured roles; "+
"this suggests env var data loss — run 'fullsend mint status --project=%s' to investigate",
len(roleOnly), p.cfg.ProjectID)
}
}

orgPresent := false
for _, o := range strings.Split(allowedOrgs, ",") {
if strings.EqualFold(strings.TrimSpace(o), org) {
Expand Down
64 changes: 63 additions & 1 deletion internal/dispatch/gcf/provisioner_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2416,6 +2416,66 @@ func TestEnsureOrgInMint_ProceedsOnFirstEnrollment(t *testing.T) {
assert.Equal(t, "new-org", fake.lastUpdateServiceEnvVars["ALLOWED_ORGS"])
}

func TestEnsureOrgInMint_DataInconsistencyGuard(t *testing.T) {
// When ALLOWED_ORGS is empty but ROLE_APP_IDS has role-only entries,
// the mint has been bootstrapped — empty ALLOWED_ORGS is data loss.
fake := newFakeGCFClient()
fake.functionInfo = &FunctionInfo{
URI: "https://mint.example.com",
EnvVars: map[string]string{},
}
fake.trafficEnvVars = map[string]string{
"ALLOWED_ORGS": "",
"ROLE_APP_IDS": `{"coder":"111","reviewer":"222"}`,
}

p := NewProvisioner(Config{ProjectID: "proj1", Region: "us-central1"}, fake)
err := p.EnsureOrgInMint(context.Background(), "https://mint.example.com", "new-org")
require.Error(t, err)
assert.Contains(t, err.Error(), "data inconsistency")
assert.Contains(t, err.Error(), "2 configured roles")
assert.Contains(t, err.Error(), "proj1")
assert.NotContains(t, fake.calls, "UpdateServiceEnvVars")
}

func TestEnsureOrgInMint_DataInconsistencyGuard_NoRoleAppIDs(t *testing.T) {
// When ALLOWED_ORGS is empty and ROLE_APP_IDS is also empty,
// this is a genuine first enrollment — no error.
fake := newFakeGCFClient()
fake.functionInfo = &FunctionInfo{
URI: "https://mint.example.com",
EnvVars: map[string]string{},
}
fake.trafficEnvVars = map[string]string{
"ALLOWED_ORGS": "",
"ROLE_APP_IDS": "",
}

p := NewProvisioner(Config{ProjectID: "proj1", Region: "us-central1"}, fake)
err := p.EnsureOrgInMint(context.Background(), "https://mint.example.com", "new-org")
require.NoError(t, err)
assert.Contains(t, fake.calls, "UpdateServiceEnvVars")
}

func TestEnsureOrgInMint_DataInconsistencyGuard_OnlyLegacyKeys(t *testing.T) {
// When ALLOWED_ORGS is empty and ROLE_APP_IDS has only legacy org/role
// keys (containing "/"), RoleOnlyAppIDs filters them out — no guard trigger.
fake := newFakeGCFClient()
fake.functionInfo = &FunctionInfo{
URI: "https://mint.example.com",
EnvVars: map[string]string{},
}
fake.trafficEnvVars = map[string]string{
"ALLOWED_ORGS": "",
"ROLE_APP_IDS": `{"acme/coder":"111"}`,
}

p := NewProvisioner(Config{ProjectID: "proj1", Region: "us-central1"}, fake)
err := p.EnsureOrgInMint(context.Background(), "https://mint.example.com", "new-org")
require.NoError(t, err)
assert.Contains(t, fake.calls, "UpdateServiceEnvVars")
}

func TestRegisterPerRepoWIF_AddsNewRepo(t *testing.T) {
fake := newFakeGCFClient()
fake.functionInfo = &FunctionInfo{
Expand Down Expand Up @@ -2993,12 +3053,14 @@ func TestMarshalRoleAppIDs_SortsKeys(t *testing.T) {
}

func TestEnsureOrgInMint_DerivesAllowedRolesWhenEmpty(t *testing.T) {
// ALLOWED_ORGS populated, ALLOWED_ROLES empty — should derive roles
// from ROLE_APP_IDS and proceed.
fake := newFakeGCFClient()
fake.functionInfo = &FunctionInfo{
URI: "https://mint.example.com",
}
fake.trafficEnvVars = map[string]string{
"ALLOWED_ORGS": "",
"ALLOWED_ORGS": "existing-org",
"ROLE_APP_IDS": `{"coder":"100","triage":"200"}`,
}

Expand Down
Loading