From c1004adffdcbebea77ed374a548f3ed0d8fcf47b Mon Sep 17 00:00:00 2001 From: Aviad Hahami Date: Sun, 11 Oct 2026 11:04:27 +0000 Subject: [PATCH] fix: restore generateRootStatus and repair addKubernetesRole/certLogin 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 --- features.md | 5 ++ index.d.ts | 1 + src/commands.js | 129 ++++++++++++++++++++++++++++++++++++++---------- test/unit.js | 100 +++++++++++++++++++++++++++++++++++++ 4 files changed, 209 insertions(+), 26 deletions(-) diff --git a/features.md b/features.md index 60c9013..264a4ad 100644 --- a/features.md +++ b/features.md @@ -87,6 +87,11 @@ `POST /auth/{{mount_point}}{{^mount_point}}approle{{/mount_point}}/role/{{role_name}}/secret-id` +## vault.generateRootStatus + +`GET /sys/generate-root/attempt` + + ## vault.generateRootInit `PUT /sys/generate-root/attempt` diff --git a/index.d.ts b/index.d.ts index d9b197e..b085f33 100644 --- a/index.d.ts +++ b/index.d.ts @@ -105,6 +105,7 @@ declare namespace NodeVault { init(options?: Option): Promise; unseal(options?: Option): Promise; seal(options?: Option): Promise; + generateRootStatus(options?: Option): Promise; generateRootInit(options?: Option): Promise; generateRootCancel(options?: Option): Promise; generateRootUpdate(options?: Option): Promise; diff --git a/src/commands.js b/src/commands.js index efffce7..c6c0160 100644 --- a/src/commands.js +++ b/src/commands.js @@ -208,32 +208,72 @@ module.exports = { path: '/auth/{{mount_point}}{{^mount_point}}{{kubernetesPath}}{{/mount_point}}/role/{{ role_name }}', schema: { req: { - name: { - type: 'string', - }, - bound_cidrs: { - type: 'array', - }, - bound_service_account_names: { - type: 'array', - }, - bound_service_account_namespaces: { - type: 'array', - }, - ttl: { - type: 'integer', - }, - max_ttl: { - type: 'integer', - }, - policies: { - type: 'array', - }, - num_uses: { - type: 'integer', - }, - period: { - type: 'integer', + type: 'object', + properties: { + bound_service_account_names: { + type: ['array', 'string'], + }, + bound_service_account_namespaces: { + type: ['array', 'string'], + }, + bound_service_account_namespace_selector: { + type: 'string', + }, + audience: { + type: 'string', + }, + alias_name_source: { + type: 'string', + }, + alias_metadata: { + type: ['object', 'array'], + }, + token_policies: { + type: ['array', 'string'], + }, + token_ttl: { + type: ['integer', 'string'], + }, + token_max_ttl: { + type: ['integer', 'string'], + }, + token_explicit_max_ttl: { + type: ['integer', 'string'], + }, + token_period: { + type: ['integer', 'string'], + }, + token_bound_cidrs: { + type: ['array', 'string'], + }, + token_num_uses: { + type: 'integer', + }, + token_no_default_policy: { + type: 'boolean', + }, + token_type: { + type: 'string', + }, + // Deprecated aliases of the token_* fields; Vault still accepts them. + policies: { + type: ['array', 'string'], + }, + ttl: { + type: ['integer', 'string'], + }, + max_ttl: { + type: ['integer', 'string'], + }, + period: { + type: ['integer', 'string'], + }, + bound_cidrs: { + type: ['array', 'string'], + }, + num_uses: { + type: 'integer', + }, }, }, }, @@ -341,6 +381,38 @@ module.exports = { } }, }, + generateRootStatus: { + method: 'GET', + path: '/sys/generate-root/attempt', + schema: { + res: { + type: 'object', + properties: { + started: { + type: 'boolean', + }, + nonce: { + type: 'string', + }, + progress: { + type: 'integer', + minimum: 0, + }, + required: { + type: 'integer', + minimum: 1, + }, + pgp_fingerprint: { + type: 'string', + }, + complete: { + type: 'boolean', + }, + }, + required: ['started', 'nonce', 'progress', 'required', 'pgp_fingerprint', 'complete'], + }, + }, + }, generateRootInit: { method: 'PUT', path: '/sys/generate-root/attempt', @@ -759,6 +831,11 @@ module.exports = { schema: { req: { type: 'object', + properties: { + name: { + type: 'string', + }, + }, }, res: tokenResponse, }, diff --git a/test/unit.js b/test/unit.js index 67906db..024d915 100644 --- a/test/unit.js +++ b/test/unit.js @@ -960,6 +960,106 @@ describe('node-vault', () => { }); }); + describe('auth and sys commands', () => { + it('should send addKubernetesRole settings as the request body', (done) => { + const settings = { + bound_service_account_names: 'app', + bound_service_account_namespaces: ['default'], + audience: 'vault', + token_policies: ['read-only'], + token_ttl: '1h', + token_bound_cidrs: ['10.0.0.0/8'], + }; + vault.addKubernetesRole({ role_name: 'my-role', ...settings }) + .then(() => { + request.should.have.calledOnce(); + const options = request.firstCall.args[0]; + options.method.should.equal('POST'); + options.path.should.equal('/auth/kubernetes/role/my-role'); + options.json.should.deep.equal(settings); + done(); + }) + .catch(done); + }); + + it('should still send the deprecated addKubernetesRole field names', (done) => { + const settings = { policies: 'read-only,audit', ttl: 3600, num_uses: 5 }; + vault.addKubernetesRole({ role_name: 'my-role', ...settings }) + .then(() => { + request.firstCall.args[0].json.should.deep.equal(settings); + done(); + }) + .catch(done); + }); + + it('should reject addKubernetesRole settings of the wrong type without a request', (done) => { + vault.addKubernetesRole({ role_name: 'my-role', token_num_uses: 'many' }) + .then(() => done(new Error('expected a validation error'))) + .catch((err) => { + err.message.should.contain('Invalid type'); + request.should.not.have.been.called(); + done(); + }) + .catch(done); + }); + + it('should not send per-call requestOptions as the certLogin body', (done) => { + response.body = { auth: { client_token: 'cert-token' } }; + vault.certLogin({ requestOptions: { cert: 'client-cert', key: 'client-key' } }) + .then(() => { + const options = request.firstCall.args[0]; + options.path.should.equal('/auth/cert/login'); + expect(options.json).to.be.undefined(); + options.cert.should.equal('client-cert'); + done(); + }) + .catch(done); + }); + + it('should send the certLogin role name as the request body', (done) => { + response.body = { auth: { client_token: 'cert-token' } }; + vault.certLogin({ name: 'web' }) + .then(() => { + request.should.have.calledOnce(); + const options = request.firstCall.args[0]; + options.method.should.equal('POST'); + options.path.should.equal('/auth/cert/login'); + options.json.should.deep.equal({ name: 'web' }); + vault.token.should.equal('cert-token'); + done(); + }) + .catch(done); + }); + + it('should call generateRootStatus with correct path and method', (done) => { + const params = { + method: 'GET', + path: '/sys/generate-root/attempt', + }; + vault.generateRootStatus() + .then(assertRequest(request, params, done)) + .catch(done); + }); + }); + + describe('command definitions', () => { + // extendOptions only forwards args named in schema.req/query.properties, + // so a schema without them silently breaks its command. + it('should declare an object schema with properties for every req and query', () => { + const malformed = []; + Object.keys(vault.commands).forEach((name) => { + const { schema } = vault.commands[name]; + ['req', 'query'].forEach((part) => { + const s = schema && schema[part]; + if (s && (s.type !== 'object' || !s.properties || typeof s.properties !== 'object')) { + malformed.push(`${name}.schema.${part}`); + } + }); + }); + malformed.should.deep.equal([]); + }); + }); + describe('commands export', () => { it('should expose commands object on client', () => { vault.commands.should.be.an('object');