fix(codegen/gcp): forward colliding credential fields in _buildGcpCredentials (#1110) #101

Merged

stack72 merged 1 commit from fix/gcp-project-credential-forwarding into main 2026-07-13 16:18:56 +00:00

Conversation 2 Commits 1 Files changed 12 +59 -30

stack72 commented 2026-07-13 15:58:14 +00:00

Owner

Copy link

Summary

• Fix _buildGcpCredentials in the GCP codegen template to forward g.<name> for credential fields that collide with domain properties, instead of hardcoding undefined
• Regenerate 6 affected models across 4 GCP services (storage/buckets, sqladmin/instances+users+databases, vmmigration/targetprojects, analyticsadmin/properties_firebaselinks)
• Update collision guard documentation in codegen/designs/gcp.md

Problem

Every GCP model declares project as a credential field for getCredentials(). When project also exists as a domain property (API query parameter), the collision guard correctly prevents injecting it as a duplicate GlobalArgsSchema entry — but _buildGcpCredentials then hardcodes project: undefined, silently dropping the user's configured project before credential resolution.

With access-token auth, this left GCP_PROJECT / GOOGLE_CLOUD_PROJECT env as the only working project source, even when project was explicitly set in the model configuration.

Note: The issue reports all 11 storage models are affected, but only buckets has the project domain property collision within storage. Across all GCP, 6 models in 4 services are affected.

Fix

Changed the else branch in the credential field loop (codegen/gcp/extensionModelGenerator.ts) from:

lines.push(`    ${f.name}: undefined,`);

to always forward:

lines.push(`    ${f.name}: g.${f.name} as string | undefined,`);

This is safe because the value already exists in g as a domain property — we're just forwarding it to credential resolution where it's needed.

Verification

• Filtered regeneration: project: undefined → project: g.project in exactly 6 models
• Full unfiltered regeneration (265 services): 6 changed, 2210 unchanged — no collateral damage
• Second run: 0 changed — idempotent
• All quality gates pass (type check, lint, format, lockfile)

Test plan

• CI passes for all 4 affected service directories (storage, sqladmin, vmmigration, analyticsadmin)
• Codegen verify job passes (idempotency)
• Manual: configure a @swamp/gcp/storage/buckets model with project: my-project-id and accessToken from vault, no GCP_PROJECT env — create should succeed

Closes swamp-club#1110

🤖 Generated with Claude Code

## Summary - Fix `_buildGcpCredentials` in the GCP codegen template to forward `g.<name>` for credential fields that collide with domain properties, instead of hardcoding `undefined` - Regenerate 6 affected models across 4 GCP services (storage/buckets, sqladmin/instances+users+databases, vmmigration/targetprojects, analyticsadmin/properties_firebaselinks) - Update collision guard documentation in `codegen/designs/gcp.md` ## Problem Every GCP model declares `project` as a credential field for `getCredentials()`. When `project` also exists as a domain property (API query parameter), the collision guard correctly prevents injecting it as a duplicate `GlobalArgsSchema` entry — but `_buildGcpCredentials` then hardcodes `project: undefined`, silently dropping the user's configured project before credential resolution. With access-token auth, this left `GCP_PROJECT` / `GOOGLE_CLOUD_PROJECT` env as the only working project source, even when `project` was explicitly set in the model configuration. **Note:** The issue reports all 11 storage models are affected, but only `buckets` has the `project` domain property collision within storage. Across all GCP, 6 models in 4 services are affected. ## Fix Changed the `else` branch in the credential field loop (codegen/gcp/extensionModelGenerator.ts) from: ```ts lines.push(` ${f.name}: undefined,`); ``` to always forward: ```ts lines.push(` ${f.name}: g.${f.name} as string | undefined,`); ``` This is safe because the value already exists in `g` as a domain property — we're just forwarding it to credential resolution where it's needed. ## Verification - Filtered regeneration: `project: undefined` → `project: g.project` in exactly 6 models - Full unfiltered regeneration (265 services): 6 changed, 2210 unchanged — no collateral damage - Second run: 0 changed — idempotent - All quality gates pass (type check, lint, format, lockfile) ## Test plan - [ ] CI passes for all 4 affected service directories (storage, sqladmin, vmmigration, analyticsadmin) - [ ] Codegen verify job passes (idempotency) - [ ] Manual: configure a `@swamp/gcp/storage/buckets` model with `project: my-project-id` and `accessToken` from vault, no `GCP_PROJECT` env — create should succeed Closes swamp-club#1110 🤖 Generated with [Claude Code](https://claude.com/claude-code)

stack72 added 1 commit 2026-07-13 15:58:15 +00:00

fix(codegen/gcp): forward colliding credential fields in _buildGcpCredentials (#1110) ... 22f106af23

When a GCP credential field name (e.g. project) collided with a domain
property, _buildGcpCredentials hardcoded it to undefined instead of
reading the value from globalArgs. This meant the project global arg was
silently dropped before credential resolution, forcing users to fall
back to GCP_PROJECT env var even when project was explicitly configured.

Always forward g.<name> for all credential fields regardless of whether
they were injected as separate GlobalArgsSchema entries or already exist
as domain properties.

Affects 6 models across 4 GCP services: storage/buckets,
sqladmin/instances+users+databases, vmmigration/targetprojects,
analyticsadmin/properties_firebaselinks.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

stack72 commented 2026-07-13 16:15:19 +00:00

Author

Owner

Copy link

Adversarial Review

Medium

1. codegen/gcp/extensionModelGenerator.ts:338-340 — _credentialKeys is dead code in GCP generated output.
   The codegen emits const _credentialKeys = new Set([...]) (line 340), but no GCP generated code ever references _credentialKeys.has(). Unlike the AWS codegen (which uses _credentialKeys to filter credential fields out of desiredState when building request bodies), the GCP codegen never generates code that reads from this set. The variable is declared and populated but unused. This is pre-existing (not introduced by this PR), but worth noting since the comment on line 337 says "Credential key set for filtering globalArgs when building request bodies" — that filtering doesn't happen in GCP models. This could cause linting warnings in generated code depending on tooling. Not a correctness issue.

2. codegen/gcp/extensionModelGenerator.ts:347-350 — For non-colliding services, the behavior is unchanged but the comment is slightly misleading.
   The comment on line 348-349 says "even when not injected as a separate global arg, the value exists as a domain property." For non-colliding services (the common case), the credential fields ARE injected as separate global args and do NOT exist as domain properties. The comment is accurate only for the collision case. The code is correct in both cases — g.fieldName reads the right value regardless — but the comment could mislead a future maintainer into thinking all credential fields always exist as domain properties.

Low

1. Generated model files — version bump with "No schema changes" upgrade entry.
   The generated models bump from their previous version to 2026.07.13.1 and add upgrade entries with description: "No schema changes". Technically the _buildGcpCredentials function did change (from project: undefined to project: g.project), which is a behavioral change. The "No schema changes" description is accurate in the narrow sense (the Zod schemas didn't change), but a reader reviewing upgrade history might not realize there was a behavioral credential-resolution change. This is a documentation nuance, not a correctness issue.

Verdict

PASS — The core fix is correct and well-scoped. Before this change, GCP services where project collided with a domain property (e.g. sqladmin, storage) would pass project: undefined to _buildGcpCredentials, causing credential resolution to ignore the user-provided project ID and fall through to environment variables. Now the colliding value is properly forwarded. The design doc update accurately reflects the new behavior. No critical or high-severity issues found.

## Adversarial Review ### Medium 1. **`codegen/gcp/extensionModelGenerator.ts:338-340` — `_credentialKeys` is dead code in GCP generated output.** The codegen emits `const _credentialKeys = new Set([...])` (line 340), but no GCP generated code ever references `_credentialKeys.has()`. Unlike the AWS codegen (which uses `_credentialKeys` to filter credential fields out of `desiredState` when building request bodies), the GCP codegen never generates code that reads from this set. The variable is declared and populated but unused. This is pre-existing (not introduced by this PR), but worth noting since the comment on line 337 says "Credential key set for filtering globalArgs when building request bodies" — that filtering doesn't happen in GCP models. This could cause linting warnings in generated code depending on tooling. Not a correctness issue. 2. **`codegen/gcp/extensionModelGenerator.ts:347-350` — For non-colliding services, the behavior is unchanged but the comment is slightly misleading.** The comment on line 348-349 says "even when not injected as a separate global arg, the value exists as a domain property." For non-colliding services (the common case), the credential fields ARE injected as separate global args and do NOT exist as domain properties. The comment is accurate only for the collision case. The code is correct in both cases — `g.fieldName` reads the right value regardless — but the comment could mislead a future maintainer into thinking all credential fields always exist as domain properties. ### Low 1. **Generated model files — version bump with "No schema changes" upgrade entry.** The generated models bump from their previous version to `2026.07.13.1` and add upgrade entries with `description: "No schema changes"`. Technically the `_buildGcpCredentials` function did change (from `project: undefined` to `project: g.project`), which is a behavioral change. The "No schema changes" description is accurate in the narrow sense (the Zod schemas didn't change), but a reader reviewing upgrade history might not realize there was a behavioral credential-resolution change. This is a documentation nuance, not a correctness issue. ### Verdict **PASS** — The core fix is correct and well-scoped. Before this change, GCP services where `project` collided with a domain property (e.g. sqladmin, storage) would pass `project: undefined` to `_buildGcpCredentials`, causing credential resolution to ignore the user-provided project ID and fall through to environment variables. Now the colliding value is properly forwarded. The design doc update accurately reflects the new behavior. No critical or high-severity issues found.

stack72 commented 2026-07-13 16:15:22 +00:00

Author

Owner

Copy link

Code Review

Blocking Issues

None.

Suggestions

1. Upgrade description could be more specific. All six model files add a "No schema changes" upgrade entry for 2026.07.13.1. The Zod schema shapes didn't change, but _buildGcpCredentials now returns a non-undefined project value — which is a meaningful runtime behavior change. Consider using a description like "Fix: project domain property now forwarded to credential resolution" so operators reading upgrade history understand why the version was bumped. That said, "No schema changes" is the established pattern in this repo for codegen-driven bumps, so this is a minor style note.

---

Correctness analysis:

The codegen fix is correct. For services where project is a domain property (and thus excluded from injectedCredFields), the old generator emitted project: undefined in _buildGcpCredentials, silently discarding the user-supplied project ID that was present in g (global args) via the domain-property entry in GlobalArgsSchema. The fix removes the conditional guard and always reads g.<field> for all credential fields. Since project IS present in GlobalArgsSchema for these services (via zodResult.inputSchemaBody), g.project is populated at runtime and correctly flows into credential resolution.

The _credentialKeys set is still built from injectedCredFields only (e.g. ["accessToken", "credentialsJson"] for sqladmin services), so project is not stripped from request bodies — correct, because it's a domain property the API needs.

All six regenerated model files show the expected diff (project: undefined → project: g.project as string | undefined) with matching version bumps and upgrade entries. The design doc update accurately describes the new forwarding behavior. No hand-edits to model files outside of codegen output; no any types or default exports introduced.

## Code Review ### Blocking Issues None. ### Suggestions 1. **Upgrade description could be more specific.** All six model files add a `"No schema changes"` upgrade entry for `2026.07.13.1`. The Zod schema shapes didn't change, but `_buildGcpCredentials` now returns a non-`undefined` `project` value — which is a meaningful runtime behavior change. Consider using a description like `"Fix: project domain property now forwarded to credential resolution"` so operators reading upgrade history understand why the version was bumped. That said, "No schema changes" is the established pattern in this repo for codegen-driven bumps, so this is a minor style note. --- **Correctness analysis:** The codegen fix is correct. For services where `project` is a domain property (and thus excluded from `injectedCredFields`), the old generator emitted `project: undefined` in `_buildGcpCredentials`, silently discarding the user-supplied project ID that was present in `g` (global args) via the domain-property entry in `GlobalArgsSchema`. The fix removes the conditional guard and always reads `g.<field>` for all credential fields. Since `project` IS present in `GlobalArgsSchema` for these services (via `zodResult.inputSchemaBody`), `g.project` is populated at runtime and correctly flows into credential resolution. The `_credentialKeys` set is still built from `injectedCredFields` only (e.g. `["accessToken", "credentialsJson"]` for sqladmin services), so `project` is not stripped from request bodies — correct, because it's a domain property the API needs. All six regenerated model files show the expected diff (`project: undefined` → `project: g.project as string | undefined`) with matching version bumps and upgrade entries. The design doc update accurately describes the new forwarding behavior. No hand-edits to model files outside of codegen output; no `any` types or default exports introduced.

stack72 merged commit 102409059a into main 2026-07-13 16:18:55 +00:00

stack72 deleted branch fix/gcp-project-credential-forwarding 2026-07-13 16:18:56 +00:00

stack72 referenced this pull request from a commit 2026-07-13 16:18:57 +00:00

fix(codegen/gcp): forward colliding credential fields in _buildGcpCredentials (#1110) (#101)

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels

No items

No labels

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

swamp-club/swamp-extensions!101

No description provided.