feat(codegen/digitalocean): vault-wireable token auth #13

Merged

stack72 merged 1 commit from worktree-473 into main 2026-05-29 00:05:21 +00:00

Conversation 2 Commits 1 Files changed 56 +2342 -354

stack72 commented 2026-05-28 23:51:28 +00:00

Owner

Copy link

Adds an optional, sensitive token global argument to the @swamp/digitalocean codegen so the API token can be sourced from a vault instead of only the DO_API_TOKEN environment variable. Mirrors the Hetzner Cloud work in #12 (swamp-club #471).

What changed

• token global arg — optional, sensitive (z.meta({ sensitive: true })), takes precedence over DO_API_TOKEN, wireable with a vault.get(...) expression. Injected unless a resource already has a real token property (collision guard). Threaded through the _lib auth path and every helper call site (CRUD, action, sub-resource, discovery, readiness/action polling); never written into a request body.
• getToken(explicitToken?) prefers the arg over the env var; the single validatedToken is replaced with a Set so each distinct token (env or per-model) is validated exactly once against GET /v2/account. The no-token error names both sources.
• Pipeline mirrors the token injection into newFieldNames so upgrade diffing sees the emitted field — preventing a spurious Removed: token entry on the next regeneration.
• Docs — DigitalOcean auth section in the shared README generator now documents both auth paths; codegen/designs/digitalocean.md lib behaviors updated.
• Regenerated model/digitalocean — 45 models + _lib + README + manifest bumped to 2026.05.29.1, each model gaining an additive Added: token upgrade entry (identity upgradeAttributes — backward compatible).

Verification

• Idempotent regeneration (zero-diff second run, including after fmt).
• 156 codegen tests pass, incl. new DigitalOcean lib tests (token precedence, per-token validation via /v2/account, token-never-in-body, error paths, DELETE threading) and generator tests (sensitive arg in both schemas, threading into CRUD/action/sub-resource methods, collision guard).
• deno check / lint / fmt --check / install --frozen clean across codegen/ and model/digitalocean/.
• All 45 models verified: version 2026.05.29.1, exactly one new upgrade entry reading Added: token, upgrades array grew by +1 (history preserved).

Note: z.meta({ sensitive: true }) is honored by swamp-core in run logs, reports, and data storage (not swamp model get output, but a vault-sourced arg is stored there as the vault.get(...) expression, so no secret leaks on the recommended path).

Closes swamp-club #473.

🤖 Generated with Claude Code

Adds an optional, sensitive `token` global argument to the @swamp/digitalocean codegen so the API token can be sourced from a vault instead of only the `DO_API_TOKEN` environment variable. Mirrors the Hetzner Cloud work in #12 (swamp-club #471). ## What changed - **`token` global arg** — optional, sensitive (`z.meta({ sensitive: true })`), takes precedence over `DO_API_TOKEN`, wireable with a `vault.get(...)` expression. Injected unless a resource already has a real `token` property (collision guard). Threaded through the `_lib` auth path and every helper call site (CRUD, action, sub-resource, discovery, readiness/action polling); never written into a request body. - **`getToken(explicitToken?)`** prefers the arg over the env var; the single `validatedToken` is replaced with a `Set` so each distinct token (env or per-model) is validated exactly once against `GET /v2/account`. The no-token error names both sources. - **Pipeline** mirrors the `token` injection into `newFieldNames` so upgrade diffing sees the emitted field — preventing a spurious `Removed: token` entry on the next regeneration. - **Docs** — DigitalOcean auth section in the shared README generator now documents both auth paths; `codegen/designs/digitalocean.md` lib behaviors updated. - **Regenerated `model/digitalocean`** — 45 models + `_lib` + README + manifest bumped to `2026.05.29.1`, each model gaining an additive `Added: token` upgrade entry (identity `upgradeAttributes` — backward compatible). ## Verification - Idempotent regeneration (zero-diff second run, including after `fmt`). - 156 codegen tests pass, incl. new DigitalOcean lib tests (token precedence, per-token validation via `/v2/account`, token-never-in-body, error paths, DELETE threading) and generator tests (sensitive arg in both schemas, threading into CRUD/action/sub-resource methods, collision guard). - `deno check` / `lint` / `fmt --check` / `install --frozen` clean across `codegen/` and `model/digitalocean/`. - All 45 models verified: version `2026.05.29.1`, exactly one new upgrade entry reading `Added: token`, upgrades array grew by +1 (history preserved). Note: `z.meta({ sensitive: true })` is honored by swamp-core in run logs, reports, and data storage (not `swamp model get` output, but a vault-sourced arg is stored there as the `vault.get(...)` expression, so no secret leaks on the recommended path). Closes swamp-club #473. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

stack72 added 1 commit 2026-05-28 23:51:28 +00:00

feat(codegen/digitalocean): vault-wireable token auth ... e02e01d0b1

Adds an optional, sensitive `token` global argument to the
@swamp/digitalocean codegen so the API token can be sourced from a vault
instead of only the DO_API_TOKEN environment variable. Mirrors the
Hetzner Cloud work in #12 (swamp-club #471).

## What changed

- **`token` global arg** — optional, sensitive (`z.meta({ sensitive: true })`),
  takes precedence over `DO_API_TOKEN`, wireable with a `vault.get(...)`
  expression. Injected unless a resource already has a real `token` property
  (collision guard). Threaded through the `_lib` auth path and every helper
  call site (CRUD, action, sub-resource, discovery, readiness/action polling);
  never written into a request body.
- **`getToken(explicitToken?)`** prefers the arg over the env var; the single
  `validatedToken` is replaced with a `Set` so each distinct token (env or
  per-model) is validated exactly once against `GET /v2/account`. The no-token
  error names both sources.
- **Pipeline** mirrors the `token` injection into `newFieldNames` so upgrade
  diffing sees the emitted field — preventing a spurious `Removed: token`
  entry on the next regeneration.
- Docs: DigitalOcean auth section in the shared README generator now documents
  both auth paths; `codegen/designs/digitalocean.md` lib behaviors updated.
- `model/digitalocean` regenerated: 45 models + `_lib` + README + manifest
  bumped to `2026.05.29.1`, each model gaining an additive `Added: token`
  upgrade entry (identity `upgradeAttributes` — backward compatible).

## Verification

- Idempotent regeneration (zero-diff second run, including after fmt).
- 156 codegen tests pass, incl. new DigitalOcean lib tests (token precedence,
  per-token validation via /v2/account, token-never-in-body, error paths,
  DELETE threading) and generator tests (sensitive arg in both schemas,
  threading into CRUD/action/sub-resource methods, collision guard).
- `deno check` / `lint` / `fmt --check` / `install --frozen` clean across
  `codegen/` and `model/digitalocean/`.
- All 45 models verified: version `2026.05.29.1`, exactly one new upgrade entry
  reading `Added: token`, upgrades array grew by +1 (history preserved).

Note: `z.meta({ sensitive: true })` is honored by swamp-core in run logs,
reports, and data storage (not `swamp model get` output, but a vault-sourced
arg is stored there as the `vault.get(...)` expression, so no secret leaks on
the recommended path).

Closes swamp-club #473.

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

stack72 commented 2026-05-29 00:02:39 +00:00

Author

Owner

Copy link

Code Review

Blocking Issues

None found.

---

Suggestions

1. remove() doesn't drain the 404 response body (libGenerator.ts ~line 181)

   tryRead() explicitly drains the response body on 404 (await resp.text()) to avoid resource leaks with real HTTP connections. remove() doesn't — it just reads resp.status and returns without consuming the body for either the 404 or the 204/200 success case. DELETE 204 is fine (empty body), but DELETE 404 returns an error JSON payload that goes unconsumed. This is inconsistent with tryRead and subResourceUpdate (which both drain). Low-severity in practice, but worth aligning.

2. tryFindByField, createAndPollAction, and pollResourceReady have no direct tests (libGenerator_test.ts)

   These three new runtime helpers are added to the generated _lib/digitalocean.ts and exercised via snapshot-tested generated code, but libGenerator_test.ts only has tests for getToken, create, read, and remove. tryFindByField in particular has non-trivial pagination logic (loop over pages using meta.total, array-finding heuristic) that would benefit from direct test cases covering: found on first page, found on second page, not found, 404 on the list endpoint itself.

3. Minor inconsistency: get method for child resources uses context.globalArgs.token where g is already in scope (extensionModelGenerator.ts ~line 362)

   When generating the get method for child resources (endpointLine is non-null), the generator emits const g = context.globalArgs; then the dynamic endpoint assignment, but the subsequent read() call uses context.globalArgs.token instead of g.token. Every other method that defines g uses g.token. Functionally correct (same object), but inconsistent.

4. checkExists list-filter passes "" if the naming field is undefined (extensionModelGenerator.ts ~line 296)

   When listFilterField is "label" and g.label is undefined, the generated code calls tryFindByField(..., "label", g.label?.toString() ?? "", ...). Passing an empty string means it searches for resources with an empty label rather than skipping the check. For "name" the same applies. In practice a user who calls checkExists: true without providing a name has already made an error, but the empty-string search could return a spurious match. A guard like if (!g.${listFilterField}) throw new Error(...) before the tryFindByField call would make the failure mode explicit.

## Code Review ### Blocking Issues None found. --- ### Suggestions 1. **`remove()` doesn't drain the 404 response body (`libGenerator.ts` ~line 181)** `tryRead()` explicitly drains the response body on 404 (`await resp.text()`) to avoid resource leaks with real HTTP connections. `remove()` doesn't — it just reads `resp.status` and returns without consuming the body for either the 404 or the 204/200 success case. DELETE 204 is fine (empty body), but DELETE 404 returns an error JSON payload that goes unconsumed. This is inconsistent with `tryRead` and `subResourceUpdate` (which both drain). Low-severity in practice, but worth aligning. 2. **`tryFindByField`, `createAndPollAction`, and `pollResourceReady` have no direct tests (`libGenerator_test.ts`)** These three new runtime helpers are added to the generated `_lib/digitalocean.ts` and exercised via snapshot-tested generated code, but `libGenerator_test.ts` only has tests for `getToken`, `create`, `read`, and `remove`. `tryFindByField` in particular has non-trivial pagination logic (loop over pages using `meta.total`, array-finding heuristic) that would benefit from direct test cases covering: found on first page, found on second page, not found, 404 on the list endpoint itself. 3. **Minor inconsistency: `get` method for child resources uses `context.globalArgs.token` where `g` is already in scope (`extensionModelGenerator.ts` ~line 362)** When generating the `get` method for child resources (`endpointLine` is non-null), the generator emits `const g = context.globalArgs;` then the dynamic endpoint assignment, but the subsequent `read()` call uses `context.globalArgs.token` instead of `g.token`. Every other method that defines `g` uses `g.token`. Functionally correct (same object), but inconsistent. 4. **`checkExists` list-filter passes `""` if the naming field is undefined (`extensionModelGenerator.ts` ~line 296)** When `listFilterField` is `"label"` and `g.label` is `undefined`, the generated code calls `tryFindByField(..., "label", g.label?.toString() ?? "", ...)`. Passing an empty string means it searches for resources with an empty label rather than skipping the check. For `"name"` the same applies. In practice a user who calls `checkExists: true` without providing a name has already made an error, but the empty-string search could return a spurious match. A guard like `if (!g.${listFilterField}) throw new Error(...)` before the `tryFindByField` call would make the failure mode explicit.

stack72 commented 2026-05-29 00:03:12 +00:00

Author

Owner

Copy link

Adversarial Review

This PR adds vault-wireable token auth to the DigitalOcean codegen pipeline. Every generated model gets an optional token global argument (marked sensitive) that overrides DO_API_TOKEN, and every helper call in the generated code threads that token through. The shared lib (_lib/digitalocean.ts) switches from a single cached token to a Set of validated tokens. The README and design doc are updated.

Critical / High

None found.

Medium

1. libGenerator.ts (generated _lib/digitalocean.ts), remove() function, line ~187 of the template: The remove() helper returns { existed: resp.status !== 404 } but does not drain the response body on 404. Every other path (non-ok handled by request(), 204 has no body, etc.) drains the body, but a 404 on DELETE leaks the undrained response. In practice this is likely harmless because DigitalOcean's 404 responses are small and the connection won't be reused, but it's inconsistent with how tryRead handles 404 (which calls await resp.text()). The read(), update(), and subResourceUpdate() helpers all drain their responses.

   Breaking scenario: If Deno's HTTP client enforces strict body consumption for connection reuse, a 404 on DELETE could log a warning or leak a connection.

   Suggested fix: Add if (resp.status === 404) { await resp.text(); } before the return, or unconditionally drain with await resp.text(); before checking status.

2. libGenerator.ts (generated _lib/digitalocean.ts), subResourceUpdate() function, line ~199 of the template: When request() returns a 404 for the sub-resource endpoint, subResourceUpdate() silently drains the body and returns void without throwing. The caller then calls read() to re-read the parent resource. If the sub-resource path returned 404 because the parent resource doesn't exist, this will just produce a confusing error from the read() call ("Resource not found") instead of a clear "sub-resource update failed" error.

   Breaking scenario: User calls resize on a database cluster that has been deleted. Gets "Resource not found: GET /v2/databases/..." instead of an error about the resize operation itself.

   Suggested fix: Check resp.status === 404 in subResourceUpdate() and throw with a descriptive error.

3. extensionModelGenerator.ts:296: In the checkExists list-filter branch, the generated code does g.${listFilterField}?.toString() ?? "". When listFilterField is "name" and g.name is undefined (user forgot to set it), this searches for a resource with name === "", finds nothing, and proceeds to create a resource with an empty/missing name — which will likely fail with a confusing API error. The direct-lookup branch (line 285) has the same pattern but uses g.${namingField} directly without the ?.toString() ?? "" fallback, so it would pass undefined to tryRead, which would call /endpoint/undefined and get a 404 — also wrong but at least wouldn't create a bad resource.

   Breaking scenario: User sets checkExists: true but forgets to set name in globalArgs. The check passes (no resource named ""), then create fails with a DigitalOcean validation error about missing name — but only after wasting the checkExists API call.

   Suggested fix: This is an edge case since name is typically required in GlobalArgsSchema and Zod would reject the input before execute runs. LOW risk in practice.

Low

1. libGenerator.ts, validatedTokens Set (line ~19 of generated lib): The validatedTokens Set is module-level and grows unboundedly. In a long-running process that receives many distinct tokens (e.g., a service processing requests for multiple tenants), this set would grow without bound. In practice this is unlikely to matter since DigitalOcean models typically use 1-2 tokens, but it's technically a memory leak.

2. libGenerator.ts, pollAction() and pollResourceReady() timing: Both polling functions use fixed delays (5s and 10s respectively) with 60 max attempts. For pollResourceReady this means up to 10 minutes of polling, during which the token validation cache is relied upon. If the token is rotated during a long poll, the cached validation would still allow requests but the API would reject them with 401, which request() would throw as a generic API error. This is a known limitation rather than a bug.

3. pipeline.ts:435-437: The comment explains why token is added to newFieldNames — to prevent spurious "Removed: token" upgrade entries. The logic is correct but relies on the assumption that the field name collision check in extensionModelGenerator.ts (line 129) uses nameOnly === "token" to match. If a future refactor changes how the collision is detected, these two sites could drift. The code is correct as-is.

Verdict

PASS — The PR is a clean, systematic threading of an optional token parameter through all DO API helpers and generated model methods. The token handling follows the same pattern already established in the Hetzner provider (validated once per distinct token, sent only in the Authorization header, marked sensitive). Tests are thorough — covering token precedence, validation caching, collision guards, and per-method threading. The medium findings are pre-existing patterns carried from the original code, not regressions introduced by this PR.

## Adversarial Review This PR adds vault-wireable `token` auth to the DigitalOcean codegen pipeline. Every generated model gets an optional `token` global argument (marked sensitive) that overrides `DO_API_TOKEN`, and every helper call in the generated code threads that token through. The shared lib (`_lib/digitalocean.ts`) switches from a single cached token to a `Set` of validated tokens. The README and design doc are updated. ### Critical / High None found. ### Medium 1. **`libGenerator.ts` (generated `_lib/digitalocean.ts`), `remove()` function, line ~187 of the template**: The `remove()` helper returns `{ existed: resp.status !== 404 }` but does not drain the response body on 404. Every other path (non-ok handled by `request()`, 204 has no body, etc.) drains the body, but a 404 on DELETE leaks the undrained response. In practice this is likely harmless because DigitalOcean's 404 responses are small and the connection won't be reused, but it's inconsistent with how `tryRead` handles 404 (which calls `await resp.text()`). The `read()`, `update()`, and `subResourceUpdate()` helpers all drain their responses. **Breaking scenario**: If Deno's HTTP client enforces strict body consumption for connection reuse, a 404 on DELETE could log a warning or leak a connection. **Suggested fix**: Add `if (resp.status === 404) { await resp.text(); }` before the return, or unconditionally drain with `await resp.text();` before checking status. 2. **`libGenerator.ts` (generated `_lib/digitalocean.ts`), `subResourceUpdate()` function, line ~199 of the template**: When `request()` returns a 404 for the sub-resource endpoint, `subResourceUpdate()` silently drains the body and returns `void` without throwing. The caller then calls `read()` to re-read the parent resource. If the sub-resource path returned 404 because the parent resource doesn't exist, this will just produce a confusing error from the `read()` call ("Resource not found") instead of a clear "sub-resource update failed" error. **Breaking scenario**: User calls `resize` on a database cluster that has been deleted. Gets "Resource not found: GET /v2/databases/..." instead of an error about the resize operation itself. **Suggested fix**: Check `resp.status === 404` in `subResourceUpdate()` and throw with a descriptive error. 3. **`extensionModelGenerator.ts:296`**: In the `checkExists` list-filter branch, the generated code does `g.${listFilterField}?.toString() ?? ""`. When `listFilterField` is `"name"` and `g.name` is undefined (user forgot to set it), this searches for a resource with `name === ""`, finds nothing, and proceeds to create a resource with an empty/missing name — which will likely fail with a confusing API error. The direct-lookup branch (line 285) has the same pattern but uses `g.${namingField}` directly without the `?.toString() ?? ""` fallback, so it would pass `undefined` to `tryRead`, which would call `/endpoint/undefined` and get a 404 — also wrong but at least wouldn't create a bad resource. **Breaking scenario**: User sets `checkExists: true` but forgets to set `name` in globalArgs. The check passes (no resource named ""), then create fails with a DigitalOcean validation error about missing name — but only after wasting the checkExists API call. **Suggested fix**: This is an edge case since `name` is typically required in GlobalArgsSchema and Zod would reject the input before `execute` runs. LOW risk in practice. ### Low 1. **`libGenerator.ts`, `validatedTokens` Set (line ~19 of generated lib)**: The `validatedTokens` Set is module-level and grows unboundedly. In a long-running process that receives many distinct tokens (e.g., a service processing requests for multiple tenants), this set would grow without bound. In practice this is unlikely to matter since DigitalOcean models typically use 1-2 tokens, but it's technically a memory leak. 2. **`libGenerator.ts`, `pollAction()` and `pollResourceReady()` timing**: Both polling functions use fixed delays (5s and 10s respectively) with 60 max attempts. For `pollResourceReady` this means up to 10 minutes of polling, during which the token validation cache is relied upon. If the token is rotated during a long poll, the cached validation would still allow requests but the API would reject them with 401, which `request()` would throw as a generic API error. This is a known limitation rather than a bug. 3. **`pipeline.ts:435-437`**: The comment explains why `token` is added to `newFieldNames` — to prevent spurious "Removed: token" upgrade entries. The logic is correct but relies on the assumption that the field name collision check in `extensionModelGenerator.ts` (line 129) uses `nameOnly === "token"` to match. If a future refactor changes how the collision is detected, these two sites could drift. The code is correct as-is. ### Verdict **PASS** — The PR is a clean, systematic threading of an optional `token` parameter through all DO API helpers and generated model methods. The token handling follows the same pattern already established in the Hetzner provider (validated once per distinct token, sent only in the Authorization header, marked sensitive). Tests are thorough — covering token precedence, validation caching, collision guards, and per-method threading. The medium findings are pre-existing patterns carried from the original code, not regressions introduced by this PR.

stack72 merged commit edf96b43a6 into main 2026-05-29 00:05:21 +00:00

stack72 referenced this pull request from a commit 2026-05-29 00:05:22 +00:00

feat(codegen/digitalocean): vault-wireable token auth (#13)

stack72 deleted branch worktree-473 2026-05-29 00:05:22 +00:00

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!13

No description provided.