chore: quality backlog from the 2026-07 audit (typed HTTP errors, config validation, untested paths, release hygiene)#119
Open
kurok wants to merge 1 commit into
Open
Conversation
4179500 to
b7ee735
Compare
- Wrap non-2xx HTTP responses in a typed VaultHttpError (part of the VaultError hierarchy) while preserving the legacy message and the statusCode/error field shape, so callers can instanceof-check HTTP failures. - Fail fast with InvalidArgumentsError on missing required auth config: role_id for AppRole, role for IAM and Kubernetes; Token auth is now null-safe when the config object is missing entirely. - Cover previously untested paths: the Kubernetes service-account token read failure, the prototype-pollution guard in deepMerge (now exported for direct testing), Lease#getMetadata, and non-2xx err.error contents including the non-JSON-body fallback. The renewal-failure test now asserts the refresh timer actually re-arms. - Document the decision to keep the unmaintained long-timeout dependency. - Add a release-time check to the publish workflow that CHANGELOG.md contains a release-notes heading for the version being published. Refs #111 Signed-off-by: Yuriy R <22548029+kurok@users.noreply.github.com>
b7ee735 to
6792d05
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implements the actionable in-repo items from the 2026-07 audit quality backlog (#111): typed HTTP errors, fail-fast auth config validation, tests for the previously untested paths, and the in-repo release-hygiene items. Items that are already covered, blocked on another in-flight PR, or maintainer-decision-shaped are listed below and left open on the issue.
Refs #111
Issue checklist status
Untested paths
VaultKubernetesAuthfs.readFileSyncthrow path — new test asserts the fs error propagates and no login request is madeVaultIAMAuthdefault AWS credential-chain path — already covered by the existing "credentials from environment variables" test (constructs without explicit credentials, sofromNodeProviderChain()is built and resolved); no change neededtest/auth.base.test.mjs"reschedules when a renewal fails" — now asserts__refreshTimeoutre-arms and that advancing the clock triggers a second renewal attemptVaultNodeConfig.deepMerge—deepMergeis now exported for direct testing; tests cover__proto__/constructor/prototypeskipping (viaJSON.parse-created own keys), undefined-skip, in-place nested merge, and deep-cloningLease.getMetadata()tests (present,fromResponsemapping, undefined for KV v1)VaultApiClientnon-2xx handling — tests asserterr.errorparsed-JSON contents, the non-JSON raw-text fallback, and the empty-body (undefined) caseCode quality
VaultHttpError extends VaultError, thrown byVaultApiClient.makeRequeston non-2xx. Backward compatible: message stays"<status> - <text>",statusCode/errorkeep their shape, stillinstanceof ErrorInvalidArgumentsErroron missingrole_id(AppRole) /role(IAM, Kubernetes); Token auth now also handles a missing config object. Valid configs behave exactly as beforeVaultBaseAuth.__authTokenpromise/token two-field split — a "consider" refactor of the auth state machine with no behavior change; better done as its own focused change rather than bundled into this batchDependencies & release process
long-timeout— documented the decision to keep it (comment at the require site inVaultBaseAuth.js: unmaintained but tiny, dependency-free, and it works around the 32-bitsetTimeoutlimit that real token TTLs exceed)CHANGELOG.mdhas no# <version>release-notes heading for the version being publishedVaultApiClient— the issue says to do this after the catch/log/rethrow wrapper from Refactor: unify the 4x copy-pasted request pipeline in VaultClient #110, which is being handled in a separate in-flight PRChanges
src/errors.js— addVaultHttpError(statusCode + error fields, legacy message shape)src/VaultApiClient.js— throwVaultHttpErrorinstead of an ad-hoc plainErroron non-2xxsrc/auth/VaultAppRoleAuth.js,src/auth/VaultIAMAuth.js,src/auth/VaultKubernetesAuth.js— fail fast on missing required config;src/auth/VaultTokenAuth.js— null-safe config checksrc/auth/VaultBaseAuth.js— document thelong-timeoutkeep decisionsrc/VaultNodeConfig.js— exportdeepMergeso the security guard is directly testable.github/workflows/publish.yml— release-time CHANGELOG heading check beforenpm publishCHANGELOG.md—# Unreleasedentries for the two user-facing changestest/*— 22 new/extended tests (see checklist above); the only modified existing tests are the two the issue explicitly targets (errorsexport list, renewal reschedule)Type of change
Checklist
npm run lint && npm testpasses locally (unit: 277 passing, up from 255; coverage 98.33% stmts / 96.23% branch / 100% funcs / 98.33% lines, above the 97/93/100/97 gates)# UnreleasedinCHANGELOG.mdSigned-off-by:trailer (git commit -s)