Skip to content

fix: restore generateRootStatus and repair addKubernetesRole/certLogin schemas - #312

Merged
aviadhahami merged 1 commit into
masterfrom
fix/broken-commands
Oct 11, 2026
Merged

aviadhahami merged 1 commit into
masterfrom
fix/broken-commands

Conversation

@aviadhahami

Copy link
Copy Markdown
Collaborator

What was broken

Command Symptom (every release since 0.10.0)
addKubernetesRole Any call with arguments rejects with TypeError: Cannot convert undefined or null to object, before a request is sent. No role can be created.
certLogin Same TypeError for any argument, including per-call requestOptions with the client certificate. Only a bare certLogin() with TLS settings in the client config works.
generateRootStatus vault.generateRootStatus is not a function. The command was dropped by the 0.10.0 lint merge (#225), and 0.11.0 then removed its type declaration as dangling.

Root cause of the first two: extendOptions builds the request body from schema.req.properties, and neither schema had that key:

  • addKubernetesRole's fields sat directly under req.
  • certLogin's req was a bare { type: 'object' }.

Changes

  • addKubernetesRole: the schema now declares the role parameters from Vault's own OpenAPI spec for auth/kubernetes/role/{name}:

    • the current token_* fields, audience, alias_name_source, alias_metadata and bound_service_account_namespace_selector;
    • the deprecated aliases it already listed (policies, ttl, max_ttl, period, num_uses, bound_cidrs).

    List fields accept an array or a comma-separated string, and durations accept seconds or a string such as "1h", exactly as Vault does. This uses the same type: [...] union tokenRenew already uses.

  • Why not just re-nest the legacy fields: undeclared keys are never sent. A schema with only the legacy names would make calls that use token_bound_cidrs, token_num_uses or audience succeed while silently creating a role without those restrictions.

  • name is gone from the schema. The role name is role_name in the path, and Vault overwrote a name in the body anyway.

  • certLogin: declares the optional name, which is the certificate role to match.

  • generateRootStatus: restored exactly as in 0.9.22 (GET /sys/generate-root/attempt), together with its index.d.ts declaration. features.md is regenerated.

  • Tests:

    • 7 new unit tests: request bodies, the deprecated names, type rejection without a request, per-call requestOptions kept out of the body, generateRootStatus.
    • A guard test that every command's req/query schema is { type: 'object', properties: {...} }. It would have caught both schema bugs.

Verification

  • Unit tests: all 7 new tests fail on master and pass here (106 passing). Lint is unchanged (0 errors).
  • Real Vault, both vault:1.13.3 (the repo's image) and hashicorp/vault:2.1.1. Each command was driven from this branch and from master:
Scenario master this branch
addKubernetesRole with the current fields, string forms included ('app,worker', token_ttl: '1h', token_bound_cidrs, audience, …) TypeError, nothing sent role created; read back with every field set ('1h' → 3600, strings → arrays)
addKubernetesRole with the legacy payload from example/auth_kubernetes.js TypeError role created (token_policies: ["app"], token_ttl: 3600)
addKubernetesRole on a second mount via mount_point TypeError role created on that mount
certLogin({ name: 'web' }) over mutual TLS (roles web, db) TypeError logs in; tokenLookupSelf shows web's policies (db → db's)
certLogin({ name: 'nope' }) / self-signed client cert TypeError Vault 400 (the audit log shows {"name":"nope"} reached Vault)
generateRootStatus() across init → status → cancel → status not a function started true → false with the same nonce; responses validate against the restored schema

Behaviour notes

  • addKubernetesRole arguments are now validated like every other command's, so a wrongly typed value (e.g. token_num_uses: 'many') rejects before any request is sent.
  • Only declared parameters are sent. That is how every generated command works, and the schema now covers Vault's full parameter list.

Not in this PR (pre-existing, found while verifying)

  • Private keys can end up in request bodies. Schema-less POST/PUT commands (e.g. mount) send all arguments as the JSON body, per-call requestOptions included, so a client key/passphrase reaches Vault in the body.
  • certLogin hard-codes /auth/cert/login. Unlike the other login commands, it has no mount_point.
  • example/auth_kubernetes.js builds the config path with single quotes ('auth/${kubernetesPath}/config', a literal string).
  • .github/workflows/write-features.yaml filters on src/commands/**, which never matches src/commands.js, so features.md never regenerates on its own.

🤖 Generated with Claude Code

…n schemas

addKubernetesRole and certLogin threw "TypeError: Cannot convert undefined
or null to object" whenever they were given any argument, and have done so
since both were added in 0.10.0 (first published in 0.9.23-canary.1).
extendOptions builds the request body from schema.req.properties, and
neither schema had one: addKubernetesRole's fields sat directly under req,
and certLogin's req was a bare { type: 'object' }.

- addKubernetesRole: the schema now declares the role parameters of Vault's
  Kubernetes auth API, per its OpenAPI spec: the current token_*, audience,
  alias_name_source, alias_metadata and namespace-selector fields, plus the
  deprecated aliases it already had (policies, ttl, max_ttl, period,
  num_uses, bound_cidrs). As in Vault, list fields take an array or a
  comma-separated string and durations take seconds or a duration string.
  Undeclared keys are never sent, so declaring only the legacy names would
  have dropped token_bound_cidrs, audience and the rest without an error.
  The unused `name` field is gone: the role name is role_name in the path.
- certLogin: declare the optional `name` (the certificate role to match).
- generateRootStatus (GET /sys/generate-root/attempt): restored exactly as
  it was in 0.9.22. The 0.10.0 lint merge (#225) dropped it, and 0.11.0
  then removed its dangling type declaration, which is restored too.
- features.md regenerated (adds generateRootStatus).

New unit tests cover the three commands, and a guard test checks that every
command's req/query schema declares type 'object' with properties, which
would have caught both schema bugs.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@aviadhahami
aviadhahami merged commit c800393 into master Oct 11, 2026
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant