feat(codegen/hetzner): vault-wireable token auth and paginated list method #12

Merged
stack72 merged 1 commit from worktree-471 into main 2026-05-28 20:50:37 +00:00
Owner

Adds vault-wireable auth and discovery to the Hetzner Cloud codegen.

What changed

  • token global arg — optional, sensitive (z.meta({ sensitive: true })), takes precedence over HETZNER_API_TOKEN, wireable with ${{ vault.get(...) }}. Threaded through the _lib auth path (each distinct token validated once via a Set cache); never written into a request body.
  • list method — generated for every collection-GET resource. Discovers by an optional Hetzner label selector, follows meta.pagination.next_page to exhaustion, and writes one state resource per item (factory), returning a count. swamp does not enforce required global args at instantiation, so list works on the existing models with no schema relaxation — CRUD validation is untouched.
  • All changes in codegen/hetzner + codegen/shared; model/hetzner-cloud regenerated (manifest + 11 models → 2026.05.28.1, upgrade entry Added: token). Fixed a latent upgrade-diff bug: the pipeline now mirrors the generator's token injection into newFieldNames, preventing a spurious Removed: token entry on the next regeneration.

Verification

  • Idempotent regeneration (zero-diff second run).
  • 23 codegen tests pass (token precedence, per-token validation, pagination, query encoding, error paths, token-never-in-body, list emission, CRUD validation preserved).
  • deno check / lint / fmt --check / install --frozen clean across codegen/ and model/hetzner-cloud/.

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 #471. Sibling follow-ups for env-var-only auth: #473 (DigitalOcean), #474 (AWS), #475 (GCP), #476 (Cloudflare).

🤖 Generated with Claude Code

Adds vault-wireable auth and discovery to the Hetzner Cloud codegen. ## What changed - **`token` global arg** — optional, sensitive (`z.meta({ sensitive: true })`), takes precedence over `HETZNER_API_TOKEN`, wireable with `${{ vault.get(...) }}`. Threaded through the `_lib` auth path (each distinct token validated once via a `Set` cache); never written into a request body. - **`list` method** — generated for every collection-GET resource. Discovers by an optional Hetzner label selector, follows `meta.pagination.next_page` to exhaustion, and writes one `state` resource per item (factory), returning a count. swamp does not enforce required global args at instantiation, so `list` works on the existing models with no schema relaxation — CRUD validation is untouched. - All changes in `codegen/hetzner` + `codegen/shared`; `model/hetzner-cloud` regenerated (manifest + 11 models → `2026.05.28.1`, upgrade entry `Added: token`). Fixed a latent upgrade-diff bug: the pipeline now mirrors the generator's `token` injection into `newFieldNames`, preventing a spurious `Removed: token` entry on the next regeneration. ## Verification - Idempotent regeneration (zero-diff second run). - 23 codegen tests pass (token precedence, per-token validation, pagination, query encoding, error paths, token-never-in-body, list emission, CRUD validation preserved). - `deno check` / `lint` / `fmt --check` / `install --frozen` clean across `codegen/` and `model/hetzner-cloud/`. 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 #471. Sibling follow-ups for env-var-only auth: #473 (DigitalOcean), #474 (AWS), #475 (GCP), #476 (Cloudflare). 🤖 Generated with [Claude Code](https://claude.com/claude-code)
feat(codegen/hetzner): vault-wireable token auth and paginated list method
All checks were successful
CI / codegen - lint (pull_request) Successful in 46s
CI / codegen - lockfile up to date (pull_request) Successful in 49s
CI / Adversarial Code Review (pull_request) Successful in 3m52s
CI / Claude Code Review (pull_request) Successful in 5m27s
CI / Merge Gate (pull_request) Successful in 25s
CI / workflows/gcs-bootstrap - lockfile up to date (pull_request) Has been skipped
CI / cve/dirtyfrag - check (pull_request) Has been skipped
CI / cve/dirtyfrag - fmt (pull_request) Has been skipped
CI / cve/dirtyfrag - lint (pull_request) Has been skipped
CI / cve/dirtyfrag - test (pull_request) Has been skipped
CI / cve/mini-shai-hulud - check (pull_request) Has been skipped
CI / cve/mini-shai-hulud - fmt (pull_request) Has been skipped
CI / cve/mini-shai-hulud - lint (pull_request) Has been skipped
CI / cve/mini-shai-hulud - test (pull_request) Has been skipped
CI / cve/dirtyfrag - lockfile up to date (pull_request) Has been skipped
CI / cve/mini-shai-hulud - lockfile up to date (pull_request) Has been skipped
CI / Dependency Audit (pull_request) Successful in 4m3s
CI / aws models - lockfiles up to date (pull_request) Successful in 1m9s
CI / gcp models - lockfiles up to date (pull_request) Successful in 1m7s
CI / aws models - sample check (pull_request) Successful in 1m22s
CI / model/digitalocean - lockfile up to date (pull_request) Successful in 1m25s
CI / codegen - fmt (pull_request) Successful in 1m17s
CI / model/hetzner-cloud - lockfile up to date (pull_request) Successful in 1m19s
CI / model/digitalocean - check (pull_request) Successful in 1m31s
CI / CI Security Review (pull_request) Has been skipped
CI / cloudflare models - lockfiles up to date (pull_request) Successful in 1m20s
CI / codegen - check (pull_request) Successful in 1m23s
CI / model/hetzner-cloud - check (pull_request) Successful in 1m31s
CI / gcp models - sample check (pull_request) Successful in 1m43s
CI / cloudflare models - sample check (pull_request) Successful in 1m37s
46ee631216
Add an optional, sensitive `token` global argument to every Hetzner model.
It takes precedence over HETZNER_API_TOKEN so the token can be wired with a
${{ vault.get(...) }} expression instead of a shell env var. The token is
threaded through the shared _lib auth path (getToken validates each distinct
token once via a Set cache) and is never written into a request body.

Add a paginated `list` method for every resource with a collection GET. It
discovers resources by an optional Hetzner label selector, follows
meta.pagination.next_page to exhaustion, and writes one `state` resource per
item (factory), returning a count. swamp does not enforce required global args
at instantiation, so list works on the existing models with no schema change.

All changes are in codegen/hetzner + codegen/shared; model/hetzner-cloud is
regenerated (manifest + 11 models -> 2026.05.28.1, upgrade entry "Added: token").

Closes swamp-club #471.

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

Adversarial Review

Medium

  1. Incomplete token collision guard — extensionModelGenerator.ts:81-83 + :171

    The collision guard correctly skips injecting the auth-specific token field into GlobalArgsSchema when a resource already has a property named "token":

    const hasTokenProp = globalArgsProps.some((p) => p.nameOnly === "token");
    

    But the generated method bodies unconditionally reference g.token as the auth argument regardless of hasTokenProp:

    `const result = await create("${endpoint}", body, g.token) as ResourceData;`
    

    If a future Hetzner resource had a property named token (e.g., a webhook token string), the guard would suppress the auth-specific injection, so g.token would resolve to the resource property value. That value would be:

    • Written into the request body (body.token = g.token), and
    • Passed as the token auth argument to create(), attempting API authentication with the resource's property value.

    Breaking example: A hypothetical webhooks resource with createProperties: { token: { type: "string" } } would use the webhook's token string for Bearer auth, resulting in a 401 from the Hetzner API.

    Suggested fix: When hasTokenProp is true, emit undefined instead of g.token in the auth argument position (falling back to the env var), or skip emitting the auth argument entirely. Something like:

    const tokenExpr = hasTokenProp ? "undefined" : "g.token";
    // then use tokenExpr in all method body lines
    

    This is MEDIUM because no current Hetzner resource has a token property, and a collision would be caught during regeneration review. But the guard's intent is to handle this case, and it's currently incomplete.

  2. listAll pagination does not detect cycles — libGenerator.ts:188-211

    The loop assigns page = nextPage from the API response on each iteration. If the API returned a next_page value that points to an already-visited page (e.g., a bug where page 3's response says next_page: 1), the function would re-fetch pages in a cycle, accumulating duplicate items up to maxPages * perPage = 50,000 entries before the bounded guard breaks out.

    Breaking example: A Hetzner API bug or proxy misbehavior returns { servers: [...], meta: { pagination: { next_page: 1 } } } on every page. Result: 50,000 duplicate items returned instead of the real ~50, consuming memory and writing duplicate state resources.

    Suggested fix: Track visited page numbers in a Set<number> and break with a warning if a page is revisited:

    const visited = new Set<number>();
    // ...
    if (visited.has(nextPage)) { console.warn(...); return items; }
    visited.add(nextPage);
    

    This is MEDIUM because the bounded guard prevents an infinite loop, but the duplicate-accumulation behavior could still be surprising.

Low

  1. list method always advertises label selector support — extensionModelGenerator.ts:360

    The generated list description always says "optionally filtered by a Hetzner label selector" and the method always passes label_selector as a query parameter. Not all Hetzner resources support label selectors (e.g., DNS zones use a different filtering model). The Hetzner API would ignore the unknown parameter, so this is functionally harmless, but the description could be misleading.

  2. validatedTokens Set grows unboundedly — libGenerator.ts:15

    The Set<string> caching validated tokens is never pruned. In a long-running process that rotates through many tokens (e.g., a CI runner using short-lived vault tokens), the Set would grow without bound. In practice, Hetzner API tokens are long-lived and the number of distinct tokens per process is small, so this is unlikely to matter.

  3. No test for listAll with empty collection response — libGenerator_test.ts

    The test suite covers single-page, multi-page, and error responses, but doesn't test the case where the API returns an empty collection (e.g., { servers: [], meta: { pagination: { next_page: null } } }). unwrapList would return [] correctly via the array check, so this is just a coverage gap, not a bug.

Verdict

PASS — The code is well-structured, tests are thorough, and the auth-threading and pagination implementation are solid. The collision guard incompleteness (finding #1) is the most concerning item but is currently theoretical since no Hetzner resource has a token property. The pagination cycle detection gap (finding #2) is bounded by the page guard. Neither rises to blocking severity.

## Adversarial Review ### Medium 1. **Incomplete token collision guard — `extensionModelGenerator.ts:81-83` + `:171`** The collision guard correctly skips injecting the auth-specific `token` field into `GlobalArgsSchema` when a resource already has a property named `"token"`: ```ts const hasTokenProp = globalArgsProps.some((p) => p.nameOnly === "token"); ``` But the generated method bodies **unconditionally** reference `g.token` as the auth argument regardless of `hasTokenProp`: ```ts `const result = await create("${endpoint}", body, g.token) as ResourceData;` ``` If a future Hetzner resource had a property named `token` (e.g., a webhook token string), the guard would suppress the auth-specific injection, so `g.token` would resolve to the resource property value. That value would be: - Written into the request body (`body.token = g.token`), **and** - Passed as the `token` auth argument to `create()`, attempting API authentication with the resource's property value. **Breaking example:** A hypothetical `webhooks` resource with `createProperties: { token: { type: "string" } }` would use the webhook's token string for Bearer auth, resulting in a 401 from the Hetzner API. **Suggested fix:** When `hasTokenProp` is true, emit `undefined` instead of `g.token` in the auth argument position (falling back to the env var), or skip emitting the auth argument entirely. Something like: ```ts const tokenExpr = hasTokenProp ? "undefined" : "g.token"; // then use tokenExpr in all method body lines ``` This is **MEDIUM** because no current Hetzner resource has a `token` property, and a collision would be caught during regeneration review. But the guard's intent is to handle this case, and it's currently incomplete. 2. **`listAll` pagination does not detect cycles — `libGenerator.ts:188-211`** The loop assigns `page = nextPage` from the API response on each iteration. If the API returned a `next_page` value that points to an already-visited page (e.g., a bug where page 3's response says `next_page: 1`), the function would re-fetch pages in a cycle, accumulating duplicate items up to `maxPages * perPage = 50,000` entries before the bounded guard breaks out. **Breaking example:** A Hetzner API bug or proxy misbehavior returns `{ servers: [...], meta: { pagination: { next_page: 1 } } }` on every page. Result: 50,000 duplicate items returned instead of the real ~50, consuming memory and writing duplicate `state` resources. **Suggested fix:** Track visited page numbers in a `Set<number>` and break with a warning if a page is revisited: ```ts const visited = new Set<number>(); // ... if (visited.has(nextPage)) { console.warn(...); return items; } visited.add(nextPage); ``` This is **MEDIUM** because the bounded guard prevents an infinite loop, but the duplicate-accumulation behavior could still be surprising. ### Low 3. **`list` method always advertises label selector support — `extensionModelGenerator.ts:360`** The generated `list` description always says "optionally filtered by a Hetzner label selector" and the method always passes `label_selector` as a query parameter. Not all Hetzner resources support label selectors (e.g., DNS zones use a different filtering model). The Hetzner API would ignore the unknown parameter, so this is functionally harmless, but the description could be misleading. 4. **`validatedTokens` Set grows unboundedly — `libGenerator.ts:15`** The `Set<string>` caching validated tokens is never pruned. In a long-running process that rotates through many tokens (e.g., a CI runner using short-lived vault tokens), the Set would grow without bound. In practice, Hetzner API tokens are long-lived and the number of distinct tokens per process is small, so this is unlikely to matter. 5. **No test for `listAll` with empty collection response — `libGenerator_test.ts`** The test suite covers single-page, multi-page, and error responses, but doesn't test the case where the API returns an empty collection (e.g., `{ servers: [], meta: { pagination: { next_page: null } } }`). `unwrapList` would return `[]` correctly via the array check, so this is just a coverage gap, not a bug. ### Verdict **PASS** — The code is well-structured, tests are thorough, and the auth-threading and pagination implementation are solid. The collision guard incompleteness (finding #1) is the most concerning item but is currently theoretical since no Hetzner resource has a `token` property. The pagination cycle detection gap (finding #2) is bounded by the page guard. Neither rises to blocking severity.
Author
Owner

Code Review

Blocking Issues

None.

Model files under model/hetzner-cloud/ are all auto-generated (confirmed by // Auto-generated headers and the presence of corresponding codegen/hetzner/ changes). No hand-edits detected. CLAUDE.md rules are otherwise followed: named exports only, no any in hand-written code (deno-lint-ignore comments are correctly scoped to OpenAPI-parsing functions), npm dependencies in the generated import use an exact version (npm:zod@4.3.6). Tests use withFetchQueue/withFetchRouter local stubs (no live cloud calls), and all env vars are restored in finally blocks.


Suggestions

  1. remove() leaves the 404 response body unconsumed (libGenerator.ts, generated remove() function)

    When request() returns a 404, remove() returns immediately without draining the body:

    if (resp.status !== 422) {
      return { existed: resp.status !== 404 };  // 404 body never read
    }
    

    Compare with tryRead(), which correctly calls await resp.text() before returning null on 404. For real HTTP connections, an unconsumed body prevents keep-alive reuse. Consider mirroring tryRead():

    if (resp.status !== 422) {
      if (resp.status === 404) await resp.text();
      return { existed: resp.status !== 404 };
    }
    
  2. HetznerResource.identifyingField is set but never consumed (pipeline.ts line 481, extensionModelGenerator.ts)

    mergeResourceOperations() sets identifyingField: postBody.name ? "name" : "id", but the generator never reads resource.identifyingField — it calls resolveNamingField(resource) independently, which also handles the label case that identifyingField misses. The field is dead code. The type definition, assignment, and the label-naming gap are all harmless right now, but if any future caller reads identifyingField expecting parity with resolveNamingField, it would get a wrong value for label-named resources (e.g. placement_groups).

  3. listAll() silently returns [] on a 404 (libGenerator.ts, generated listAll())

    request() returns 404 responses rather than throwing. listAll() then calls resp.json() on the 404 body, unwrapList() finds no array-valued key, and the function returns an empty list without any error signal. A caller that hits a misconfigured or non-existent endpoint would receive [] and no indication of the problem. Adding an explicit status check after request() (analogous to how update() checks for 404) would make the failure visible. This is low-probability against the real Hetzner API (collection endpoints return 200 + [] for empty lists) but can silently bite during development or if the endpoint path changes.

  4. libGenerator_test.ts has no tests for read(), update(), or tryRead()

    The lib test file covers remove() (seven cases), getToken() (two cases), and listAll() (three cases). The read() 404→throw path, update() 404→throw path, and tryRead() 200→unwrap/404→null semantics have no dedicated tests. The behavior assertions in extensionModelGenerator_test.ts touch read and tryRead indirectly by checking generated source strings, but those don't execute the runtime logic. Given the depth of coverage elsewhere in this PR, adding a few short read/update/tryRead test cases to libGenerator_test.ts would complete the picture.

  5. Nested-object indentation in generated Zod schemas is inconsistent (extensionModelGenerator.ts generateFullFidelityZod/generateSimplifiedZod, visible in snapshot)

    Both generators hard-code " " (four-space) for nested object fields and " " (two-space) for the closing }), producing output like:

    public_net: z.object({
      ipv4: z.object({
        ip: z.string().optional(),   // 4-space indent
    }).optional(),                   // 2-space close
    

    The inconsistency is cosmetic and doesn't affect runtime behaviour, but since these files pass through deno fmt, it would be cleaned up on format — running deno fmt on the output directory before committing snapshots would make the snapshots reflect the formatted output.

## Code Review ### Blocking Issues None. Model files under `model/hetzner-cloud/` are all auto-generated (confirmed by `// Auto-generated` headers and the presence of corresponding `codegen/hetzner/` changes). No hand-edits detected. CLAUDE.md rules are otherwise followed: named exports only, no `any` in hand-written code (`deno-lint-ignore` comments are correctly scoped to OpenAPI-parsing functions), npm dependencies in the generated import use an exact version (`npm:zod@4.3.6`). Tests use `withFetchQueue`/`withFetchRouter` local stubs (no live cloud calls), and all env vars are restored in `finally` blocks. --- ### Suggestions 1. **`remove()` leaves the 404 response body unconsumed** (`libGenerator.ts`, generated `remove()` function) When `request()` returns a 404, `remove()` returns immediately without draining the body: ```ts if (resp.status !== 422) { return { existed: resp.status !== 404 }; // 404 body never read } ``` Compare with `tryRead()`, which correctly calls `await resp.text()` before returning `null` on 404. For real HTTP connections, an unconsumed body prevents keep-alive reuse. Consider mirroring `tryRead()`: ```ts if (resp.status !== 422) { if (resp.status === 404) await resp.text(); return { existed: resp.status !== 404 }; } ``` 2. **`HetznerResource.identifyingField` is set but never consumed** (`pipeline.ts` line 481, `extensionModelGenerator.ts`) `mergeResourceOperations()` sets `identifyingField: postBody.name ? "name" : "id"`, but the generator never reads `resource.identifyingField` — it calls `resolveNamingField(resource)` independently, which also handles the `label` case that `identifyingField` misses. The field is dead code. The type definition, assignment, and the `label`-naming gap are all harmless right now, but if any future caller reads `identifyingField` expecting parity with `resolveNamingField`, it would get a wrong value for label-named resources (e.g. `placement_groups`). 3. **`listAll()` silently returns `[]` on a 404** (`libGenerator.ts`, generated `listAll()`) `request()` returns 404 responses rather than throwing. `listAll()` then calls `resp.json()` on the 404 body, `unwrapList()` finds no array-valued key, and the function returns an empty list without any error signal. A caller that hits a misconfigured or non-existent endpoint would receive `[]` and no indication of the problem. Adding an explicit status check after `request()` (analogous to how `update()` checks for 404) would make the failure visible. This is low-probability against the real Hetzner API (collection endpoints return `200 + []` for empty lists) but can silently bite during development or if the endpoint path changes. 4. **`libGenerator_test.ts` has no tests for `read()`, `update()`, or `tryRead()`** The lib test file covers `remove()` (seven cases), `getToken()` (two cases), and `listAll()` (three cases). The `read()` 404→throw path, `update()` 404→throw path, and `tryRead()` 200→unwrap/404→null semantics have no dedicated tests. The behavior assertions in `extensionModelGenerator_test.ts` touch `read` and `tryRead` indirectly by checking generated source strings, but those don't execute the runtime logic. Given the depth of coverage elsewhere in this PR, adding a few short `read`/`update`/`tryRead` test cases to `libGenerator_test.ts` would complete the picture. 5. **Nested-object indentation in generated Zod schemas is inconsistent** (`extensionModelGenerator.ts` `generateFullFidelityZod`/`generateSimplifiedZod`, visible in snapshot) Both generators hard-code `" "` (four-space) for nested object fields and `" "` (two-space) for the closing `})`, producing output like: ```ts public_net: z.object({ ipv4: z.object({ ip: z.string().optional(), // 4-space indent }).optional(), // 2-space close ``` The inconsistency is cosmetic and doesn't affect runtime behaviour, but since these files pass through `deno fmt`, it would be cleaned up on format — running `deno fmt` on the output directory before committing snapshots would make the snapshots reflect the formatted output.
stack72 deleted branch worktree-471 2026-05-28 20:50:38 +00:00
Sign in to join this conversation.
No reviewers
No labels
No milestone
No project
No assignees
1 participant
Notifications
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!12
No description provided.