Release JavaScript GCP KMS Storage v1.1.0 - #1189
Open
mgallego-keeper wants to merge 33 commits into
Open
mgallego-keeper wants to merge 33 commits into
mgallego-keeper wants to merge 33 commits into
Conversation
…1165) * fix(storage/js-gcp): bump axios, form-data, protobufjs, follow-redirects, handlebars, babel - axios -> 1.19.0 (CVE-2026-40175, CVE-2026-42040) - form-data -> 4.0.6 (CVE-2026-12143, CRLF injection via unescaped multipart field names/filenames) - protobufjs -> 7.6.5 (CVE-2026-44292, CVE-2026-44294, CVE-2026-45740, CVE-2026-54269) - real production path via @google-cloud/kms -> google-gax -> protobufjs - follow-redirects -> 1.16.0 (CVE-2026-40895), pulled in as a side effect of the axios bump - handlebars -> 4.7.9 (CVE-2026-33938, CVE-2026-33941) - dev-only via ts-jest - @babel/core -> 7.29.7 (CVE-2026-49356) - dev-only Lockfile-only, no package.json range changes. Cherry-picked and scoped to sdk/javascript/packages/gcp/package-lock.json from 831b7b48, 984fc17e, and efde007d (Sergey Aldoukhov). KSM-1218 * fix(storage/js-gcp): bump brace-expansion, @grpc/grpc-js, flatted, browserslist, baseline-browser-mapping, babel-plugin-transform-modules-systemjs, humanfs-node, js-yaml, picomatch * docs(storage/js-gcp): add changelog entry for KSM-1218 dependency updates --------- Co-authored-by: Sergey Aldoukhov <saldoukhov@gmail.com>
…sions (#1181) * fix(sdk/javascript): check ENOENT before recreating GCP KMS config createConfigFileIfMissing() treated any fs.access failure the same as a missing file, so a transient error (EACCES, EPERM, ESTALE) silently overwrote the real on-disk config with an empty one instead of only recreating it when the file is genuinely missing (ENOENT). Types the catch clause as unknown instead of any, drops the now-unneeded eslint-disable, avoids logging the literal string "undefined" when a thrown value has no message, and reuses the already-resolved configPath instead of recomputing dirname(resolve(...)) a second time. Replaces the fs.access error handling test, which swallowed the rejection with .catch(() => undefined) and only asserted no write happened - a check that would still pass if the propagating throw were replaced with a silent return - with table-driven assertions (EACCES, EPERM, ESTALE, EIO, EBUSY) through the public init() and saveString() entry points, asserting the rejection itself propagates with the right error code. KSM-1370 * fix(sdk/javascript): write GCP KMS config file with restrictive permissions Route all four config-file write sites through a new atomic temp-file- then-rename helper (writeFileAtomicSync in atomicWrite.ts), rather than fs.writeFile(mode: 0o600) followed by fs.chmod. The mode option only takes effect when a file is created, so it was a no-op against a config file already on disk from an earlier SDK version, and the follow-up chmod rewrote the same inode, so a process already holding a read handle on the old file kept reading straight through the rewrite. The atomic rename produces a new inode, closing both gaps. The temp file opens with O_EXCL ("wx" instead of "w"), turning a name collision with another writer's in-flight temp file into an EEXIST instead of a silent truncation, and the containing directory is fsynced after a successful rename so the rename itself survives a crash, not just the file's data (EPERM/EISDIR, e.g. on Windows, is tolerated rather than failing an otherwise-successful write). Adds real-filesystem tests for the properties a mocked-fs assertion can't catch: a stale reader fd not surviving the rewrite, decryptConfig's autosave write reaching the file, saveConfig's own write independently reaching mode 0600 on a pre-existing 0644 file, both writes inside createConfigFileIfMissing routing through writeFileAtomicSync (not just the first), and the temp file still landing at exactly 0600 under a umask that strips owner bits from the initial open call. Bumps to 1.1.0 (not 1.0.1/1.0.2): this is a real behavior change (an already-exposed config file is now actually remediated, not just given a new mode bit), not a no-behavior-change patch. Regenerates package-lock.json, which still self-referenced 1.0.1 after package.json was bumped. Also resolves KSM-1458 (no atomic write anywhere in this file, filed separately during the same security review): the temp-file-then-rename approach this commit adds is exactly the fix that ticket asked for. KSM-1450
…fig writes (#1183) writeFileAtomicSync() (added in PR #1181, KSM-1450 and KSM-1370) writes to a same directory temp file, fsyncs it, then renames it over the real config path. This already gives the atomicity KSM-1458 asks for: a failed or interrupted write can never leave the real config file truncated, since the real path is only ever touched by a single rename once the new content is fully written. This adds the one regression test that property did not yet have. It intercepts fs.writeSync at module resolution, not on the live fs object (fs.writeSync is non configurable in current Node, so jest.spyOn cannot redefine it there), and makes it report fewer bytes than were actually written. It then asserts the original file's content and inode are both unchanged, and that the failed temp file was cleaned up. This needed its own file rather than living in GCPKeyValueStorage.atomicWrite.test.ts, which deliberately does not mock fs at all. jest.mock is hoisted per file, so adding a partial fs mock there would apply to every test in that file, not just this one. KSM-1458
…ption (#1187) loadConfig() decides whether the config file holds plaintext or ciphertext by trying to JSON.parse it. The saveConfig() call that encrypts a plaintext file sat inside that same try block, so a KMS failure while encrypting was caught by the catch meant for a failed parse. The code then set jsonError, logged "given file is encrypted file", and tried to decrypt content it had already parsed as valid JSON. A successful JSON.parse is proof the file is plaintext. Encrypting it is a consequence of that result, not part of computing it, so the encryption now runs in an `if (!jsonError)` block after detection finishes. A KMS outage, a revoked permission, or a disabled key version propagates as the real KMS error instead of being relabeled. Before this change a KMS PERMISSION_DENIED surfaced as "Decryption failed : Invalid header", identical to the error a genuinely corrupt file produces, because decryptBuffer rejected the plaintext for having no blob header. An operator who trusted that message could delete or replace an intact config file. The file is left untouched on disk, and the two cases are now distinguishable. Propagates the original error rather than wrapping it, so the KMS error type, message, and code survive, matching getKeyDetails() and the error handling KSM-847 established in this package. The decrypt path and both existing corrupt-file messages are unchanged: "Decryption failed : Invalid header" for content that is neither JSON nor a valid envelope, and "Failed to parse decrypted config file <path>" for an envelope wrapping a non-JSON payload. KSM-1460
… fails (#1188) changeKey() snapshotted gcpKeyConfig and cryptoClient before switching keys, but not keyType, isAsymmetric, or encryptionAlgorithm. getKeyDetails() had already overwritten those three with the new key's values, so a failure in the saveConfig() that follows left the object holding the old key paired with the new key's algorithm. For two asymmetric keys with different OAEP hashes that combination is unrecoverable. The RSA public key comes from gcpKeyConfig while the OAEP hash comes from encryptionAlgorithm, so the next save wraps the AES data key under the old key's public key using the new key's hash. GCP derives the OAEP hash from the key version itself and never from the decrypt request, so no key can unwrap that blob: not the old key, whose declared hash no longer matches, and not the new key, which was never used. The write itself reports success, so nothing signals the loss until the next read, which loses the client ID, the app key, and the device private key. Reproduced with real RSA keypairs in the new test: both keys fail with an OAEP decoding error before this change, and the old key reads the config back after it. getKeyDetails() also assigned encryptionAlgorithm before the unsupported key-purpose check could throw. It now computes the metadata into locals, validates the purpose, and assigns the three fields only once the key is known to be usable, so a rejected key never mutates state. changeKey() rolls its own state back, but init() calls getKeyDetails() with no rollback at all. KSM-1461
…hanged save (#1186) * fix(sdk/javascript): recreate a deleted GCP KMS config file on an unchanged save saveConfig() compared the in-memory config hash against lastSavedConfigHash and returned early on a match, before it called createConfigFileIfMissing(). If the config file was deleted or replaced while the process kept running, and the in-memory value had not changed, the early return fired before the file was ever checked. saveString() and saveStorage() resolved successfully and wrote nothing, so the caller believed the value was persisted and the loss only became visible at the next restart. The skip now also requires the config file to still be present, through a new configFileExists() helper that does a single fs.access() on the resolved path. The && short-circuit keeps that syscall off the force and changed-hash paths. A missing file makes the guard false, so control falls through to the full save path, which encrypts and writes the real this.config contents. Reordering createConfigFileIfMissing() ahead of the early return would not have been enough on its own. That method writes Buffer.from("{}") and then an encrypted blob of "{}", not the real config, so a deleted file would have been repaired with an empty placeholder and the data would still have been lost. The existence check therefore forces a real save rather than standing in for one. configFileExists() reports false for any fs.access failure, matching fs.existsSync semantics. It only decides whether a save can be skipped, and a save that cannot be skipped goes on to createConfigFileIfMissing(), which already holds the ENOENT-only policy for acting on an access failure and rethrows every other code (KSM-1370). Keeping that policy in one place means a path whose existence cannot be determined now reports the failure instead of resolving silently. Adds five real-filesystem regression tests in their own file. Three delete the config file out from under a live instance and save the same value again, asserting the recreated file decrypts back to the real config and that a fresh instance's init() reads the value back, which is what catches a placeholder-only repair. One pins the unverifiable-path behavior with an ENOTDIR case, chosen over a chmod-based EACCES so it behaves the same for an unprivileged user and for root. One asserts the no-changes skip still avoids the write when the file is present. KSM-1459 * fix(sdk/javascript): stop double-encrypting a recreated GCP KMS config file The recovery path added for KSM-1459 called createConfigFileIfMissing() unconditionally once a deleted file was confirmed missing. That method itself encrypts and writes a "{}" placeholder before returning, and the code right after it immediately encrypted and wrote the real config over that same file. One recovery cost two KMS encrypt calls and two atomic writes for content that was discarded as soon as it landed. saveConfig() now tracks whether the no-changes-detected guard itself already proved the file is missing. When it has, the recovery skips createConfigFileIfMissing() and calls a new ensureConfigDirectoryExists() helper instead, extracted from createConfigFileIfMissing() unchanged, so only the directory-creation half of its work runs. The real encrypt-and- write that already follows in saveConfig() then creates the file directly, at one KMS call and one write. Also tightens the comment above the guard, which read as if the "{}" placeholder were a lasting outcome of skipping this check. It only survives a crash between the two writes; under normal execution it is overwritten within the same call, before saveConfig() ever returns. Adds a regression test that pins the call counts directly, which the existing content-only assertions could not have caught on their own. Addresses review from Stas Schaller on PR #1186. KSM-1459
…esetting it (#1184) loadConfig() treated a zero-byte config file as an empty config: it substituted "{}" for the file contents, which then fell through into the plain-JSON branch, parsed cleanly, and reached saveConfig(). That encrypted the empty object and wrote it back over the damaged file, destroying the client id, app key and device private key with no recovery path and only a warn-level log. A zero-byte file is never a legitimate "no config yet" state. createConfigFileIfMissing() runs first and only creates a file when fs.access reports ENOENT, and the file it creates always holds a real encrypted blob. Zero bytes can therefore only mean a file that already existed was truncated by an interrupted write or corruption. loadConfig() now logs at error level and throws, naming the config path, and leaves the file on disk untouched so it can be restored from a backup. A non-empty plaintext JSON config is still encrypted in place, and an existing encrypted config still loads unchanged. KSM-1455
…#1185) The constructor picked the config path with `keyVaultConfigFileLocation ?? process.env.KSM_CONFIG_FILE ?? this.defaultConfigFileLocation`. `??` falls back only on null and undefined, so an empty string passed straight through and configFileLocation became "". resolve("") is the current working directory, and fs.access on a directory succeeds, so createConfigFileIfMissing() logged the working directory as an existing config file and the failure surfaced later as a confusing empty path. An empty environment variable is a common result of a Docker --env-file or a Kubernetes ConfigMap entry with no value, so this is easy to hit by accident. Routes both fallback sources through a nonBlank() helper that maps a blank value to undefined, so each step falls back the way the JSDoc already promised. The test is value.trim() === "" rather than value === "", which also covers an env file that pads the value instead of emptying it; a non-blank value is returned unchanged, so no legitimate path is trimmed. Adds table-driven regression tests over the empty and whitespace-only forms of both sources, the no-regression cases for non-empty values, and one assertion through createConfigFileIfMissing() that fs.access is called against the resolved default config file and never against process.cwd() - the field value alone can't show that the working directory was being accepted as a config file. KSM-1457
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
Removes the long-lived NPM_TOKEN pulled via ksm-action in favor of npm's trusted publisher OIDC flow (matches secrets-manager-core's publish.npm.yml). Requires the Trusted Publisher entry on npmjs.com to be configured to accept this workflow filename/environment.
stas-schaller
force-pushed
the
release/storage/javascript/gcp-kms/v1.1.0
branch
from
September 22, 2026 18:50
789e68b to
909923c
Compare
Raises the @keeper-security/secrets-manager-core dependency from ^17.3.0 to ^17.6.0 in package.json, package-lock.json, and CHANGELOG.md. Also fixes GCPKeyValueStorage.test.ts fs mock, which stubbed only fs.promises. Core 17.6.0 reads fs.constants at module load time for cache directory symlink protection, so the incomplete mock crashed every test in the file before any test body ran. The mock now forwards the real, static fs.constants alongside its existing mocked fs.promises functions. KSM-1500
…nfig() (#1191) decryptConfig() reads the config file independently of loadConfig() and has its own zero-length check, separate from the one KSM-1455 fixed. A zero-length file logged a warning and resolved to an empty string, the same silent-corruption pattern KSM-1455 already fixed in loadConfig(), just on a different code path. Moves the check outside the read's try/catch, matching loadConfig()'s structure, so throwing here does not get re-wrapped into a generic Failed to load config file error. Now throws, names the config file, and leaves the file untouched, same as loadConfig(). KSM-1486
…1195) Drops cross-spawn, eastasianwidth, gtoken, package-json-from-dist, path-key, shebang-command, shebang-regex, and signal-exit from the resolved dependency tree, all flagged unmaintained by the release's SBOM scan. Verified: npm install, build, lint, and full test suite (113/113) pass unchanged; npm audit reports 0 vulnerabilities.
…#1194) The move to an atomic temp-file-then-rename changed two things that worked in 1.0.0, and the changelog documented neither. Both reach a consumer on a ^1.0.0 range automatically, with no API signature change to warn them. First, the directory holding the config file must now be writable, not only the config file. A write creates a temporary file alongside the config and renames it into place, where an in-place write needed permission on the file alone. A deployment that mounts a writable config file inside a read-only directory now fails with EACCES on every save. Second, a config path that is a symbolic link, or that has a hard-linked peer, is replaced rather than written through, because rename does not follow a symbolic link at its destination. A config symlinked into a shared or externally mounted location silently stops updating the link target. Nothing throws, which makes this the more dangerous of the two. Adds a Changed section for both, placed before Security to match the Keep a Changelog ordering this file already claims to follow, and notes what to check before upgrading. Also records a security improvement the same change delivered that no entry mentioned. Every write previously used fs.writeFile, which follows a symbolic link at its destination, and one of those writes is the decryptConfig() autosave path that writes the configuration in plaintext. Anyone able to create a file in the config file's directory could plant a symbolic link and have the SDK write the decrypted client ID, app key, and device private key to any file the process could write. The rename replaces the link instead of following it. Documentation only. No source change. KSM-1509
…lure (#1192) configFileExists() returned false for any fs.access failure, not only for ENOENT. saveConfig() reads a false result as "the file is gone", sets fileConfirmedMissing, and that flag takes the branch which skips createConfigFileIfMissing() and writes unconditionally. The ENOENT-only guard lives inside createConfigFileIfMissing(), so it never sees this path. A transient access failure therefore overwrote whatever was on disk with the in-memory config. ESTALE is the realistic trigger, routine on NFS and on container volume remounts. The sequence that loses data: one process performs a no-op save, so its hash still matches, fs.access fails with ESTALE, and it writes its stale config over a rotated credential another process just saved. saveString() resolved successfully, so the caller had no indication. Returns false only for ENOENT and rethrows everything else. The call site needs no change, because a thrown error reaches saveConfig()'s outer catch, which logs and rethrows. The method comment claimed the opposite of what the code did and is rewritten to describe the branch it actually feeds. The existing 'rejects instead of skipping when the config file path cannot be checked' test cannot catch this. It produces the failure with ENOTDIR by replacing the parent directory with a regular file, and that same ENOTDIR also fails the temp-file open in the write that follows, so the rejection it observes is the write's and it passes either way. The two new tests inject the failure directly and leave the directory writable, so the write would succeed if it were attempted. Both fail against the previous behavior; a third asserts the ENOENT recovery path still works. KSM-1507
…P KMS write (#1193) fsyncDirectory() wrapped only fsyncSync in its try. The openSync of the directory sat outside it, and the call to fsyncDirectory() itself sits outside any try in writeFileAtomicSync(). The data is fsynced and the rename has already committed by the time it runs, so a failure to open the directory turned a fully successful write into a thrown error. On a directory the process may write and traverse but not read, mode 0300, saveString() rejected with EACCES while the config file on disk already held the new value. lastSavedConfigHash was left unupdated too, so the in-memory state agreed with the false failure. A caller that treats a rejected save as "not persisted" retries or abandons a credential rotation that already landed. The existing comment shows Windows was anticipated, but the tolerance was attached to the wrong call: on Windows the directory open is what fails, because CreateFileW is not given FILE_FLAG_BACKUP_SEMANTICS, so fsyncSync is never reached and the EPERM/EISDIR catch never saw it. The new tests demonstrate this directly: EPERM and EISDIR, the two codes that catch named, both failed against the previous code. Moves the open inside the try and drops the allow-list entirely. Every failure here has the same conclusion, that the bytes are durable and only the directory entry's survival across a power loss is at stake, so none of them justifies reporting a successful write as failed. Full tolerance also covers the codes a network or container filesystem can return that no allow-list anticipated, such as ENOTSUP, EINVAL and EBADF, which the previous code rethrew. The finally block now guards on the descriptor being defined, since a failed open leaves nothing to close. Adds a test file that stubs openSync through jest.mock, because fs.openSync is non-configurable and jest.spyOn cannot redefine it. Six error codes are covered plus a real mode 0300 directory end to end, and one test asserts the directory is still opened and fsynced on platforms that allow it, so the fix cannot degrade into silently dropping the durability step. KSM-1508
Contributor
Author
Cross-PR overlap and merge order for #1196 to #1200This note covers PRs #1196, #1197, #1198, #1199, and #1200. All five target this release branch. It records the file overlap between them and a recommended merge order. I verified every claim below by actually performing the merges, not by reading the diffs alone. File overlap
The one real conflict#1198 and #1200 insert a new The fix is a one-line manual resolution: keep both new lines. Their order in the file does not matter, since neither line depends on the other. Merge orderNone of these five PRs depends on another for correctness. Each closes an independent ticket. Any merge order is safe. I recommend the order below. It lands the CVE fix first. It also handles the one known conflict on purpose, not by accident.
Combined verificationI merged all five branches onto the release branch tip in the order above. I resolved the one |
This was referenced Sep 24, 2026
Merged
…S storage README gaps (KSM-1536) (#1200) CHANGELOG.md was excluded from the published npm tarball, so the only place the v1.1.0 behavior changes were documented never reached a consumer who installs from npm. Added it to package.json's files array. Added a Behavior Notes section to the README covering the Node 20 floor, the directory-write and symlink-replacement requirements, the zero-length-config hard error, and the 0600 file mode. Added an explicit warning to the Decrypt Config section: decryptConfig(true) writes the client ID, app key, and device private key to disk in plaintext, and documented how to re-encrypt (call init() again, which detects and re-encrypts a plaintext config automatically). Documented KSM_CONFIG_FILE and its precedence against the constructor argument. Corrected the IAM role discrepancy between the README and GcpKeyConfig.ts, verified against both the actual GCP KMS API calls this package makes (getCryptoKey, getPublicKey, encrypt, decrypt, asymmetricDecrypt -- no key/keyring-management calls anywhere) and Google's own predefined-role permission tables: removed roles/cloudkms.admin from GcpKeyConfig.ts, which this package never needs, and added roles/cloudkms.viewer to both files, since none of the other three listed roles include cloudkms.cryptoKeys.get, which getKeyDetails() calls via getCryptoKey().
…33) (#1196) The declared floor of ^1.13.5 doesn't guarantee a patched axios for this package's own bearer-token requests in rawEncrypt/rawDecrypt. Six advisories (GHSA-6chq-wfr3-2hj9, GHSA-pf86-5x62-jrwf, GHSA-q8qp-cvcw-x6jj, GHSA-p92q-9vqr-4j8v, GHSA-35jp-ww65-95wh, GHSA-hfxv-24rg-xrqf) were fixed between 1.15.1 and 1.16.0; a consumer who already pins an old axios keeps that vulnerable version through npm's dedupe.
…ate on an unpublished version (KSM-1530) (#1198) build-npm ran npm install while generate-sbom and publish-npm both used npm ci, so the job that runs the release-gating tests could resolve a different dependency tree than the one the SBOM describes and the one that ultimately publishes. Switched build-npm to npm ci. Also ported core's get-version/validate-version jobs: nothing previously gated generate-sbom or publish-npm on the target version being unpublished, so a mistaken or repeated workflow_dispatch would produce an SBOM for a version that will never exist and page a Release Manager for a prod approval that npm would reject anyway. Co-authored-by: Mateo Gallego <mgallego@keepersecurity.com>
…(KSM-1517) (#1199) * test(sdk/javascript): close atomicWrite mutation gaps and tighten zero-length-config assertions in GCP KMS storage (KSM-1517) Mutation testing against the current suite found 3 surviving mutants in writeFileAtomicSync/fsyncDirectory (the O_EXCL-drop and whole-directory- fsync-drop mutants named in the original report are now caught by GCPKeyValueStorage.directoryFsync.test.ts, added since): the temp file's open mode can weaken from 0600 to 0644 without any test noticing (masked by the redundant chmodSecure call), the file's data fsync can be dropped, and the directory's fsync can be dropped while still opening it. Added test/atomicWrite.fsyncCoverage.test.ts, scoping each check to the relevant fd's own open/close lifetime window rather than a bare fd-number match, since fds are reused after close. Also tightened 3 bare rejects.toThrow() assertions in GCPKeyValueStorage.zeroLengthConfig.test.ts to rejects.toThrow(/is empty/), matching the pattern already used by sibling assertions in the same file, so they can't pass for an unrelated rejection reason. No production code changed; atomicWrite.ts was already correct. * test(sdk/javascript): close two remaining weak assertions in GCP KMS storage zero-length coverage (KSM-1517) decryptConfig()'s "names the offending config file" assertion at zeroLengthConfig.test.ts checked only that the error message contained the config path, not that it named the actual empty-file cause. The fallback error from a different failure in the same function also contains the path, so the assertion couldn't tell the two apart. Tightened to /is empty/, matching its three sibling assertions in the same file. The two fs.writeFile-not-called assertions in GCPKeyValueStorage.test.ts were unconditionally true: nothing in src/ calls fs.writeFile anymore, the write path moved to writeFileAtomicSync. Replaced both with a spy on writeFileAtomicSync, the function actually in the call path. Verified against a regression (widening createConfigFileIfMissing's ENOENT-only check to also swallow EACCES): both replaced assertions now fail; the originals could not have caught it. * test(sdk/javascript): assert both path and cause on decryptConfig()'s zero-length rejection, mock the write-not-attempted spies (KSM-1517) The prior commit's /is empty/ pattern alone didn't prove this test's own title claim ("names the offending config file"); adding back a configPath assertion alongside it means neither half can pass against a wrong-cause fallback error on its own. The two writeFileAtomicSync spies added for the fs.writeFile replacement now mock the implementation, so a future regression that does reach the call fails the "not called" assertion cleanly instead of performing a real filesystem write as a side effect.
…necessary tootallnate override (KSM-1534) (#1197) * fix(sdk/javascript): declare Node 20 engines floor and drop unnecessary tootallnate override in GCP KMS storage (KSM-1534) Core 17.6.0 pulls an engines.node >=20 floor into this package's tree, but the package itself declared no engines field, so a consumer on an unsupported Node version got only a silent npm warning instead of a hard failure. Declared the floor explicitly. Also dropped the "@tootallnate/once": "3.0.1" override. It resolved nothing in the current tree (http-proxy-agent is on 7.0.2, which doesn't depend on @tootallnate/once), so it was unnecessary regardless of which version it pinned. * fix(sdk/javascript): raise GCP KMS storage engines floor to Node >=22 (KSM-1534) @google-cloud/kms@^6.2.0 and eight of its own dependencies (google-gax, gcp-metadata, google-logging-utils, proto3-json-serializer, retry-request, teeny-request, google-auth-library, and the package itself) all declare an engines.node floor of >=22. Declaring >=20 here reproduced the exact npm warning-only signal this ticket exists to replace with a confident floor, one Node version later: a Node 20/21 consumer would see this package's own package.json promise >=20 while a dependency two levels down already disagreed. Verified with npm ci: 9 EBADENGINE warnings on Node 20.20.2, zero on Node 22.23.3. * fix(sdk/javascript): update lockfile, CHANGELOG, and CI to match the Node >=22 floor (KSM-1534) Three artifacts still said >=20 after the engines.node bump: the package-lock.json root engines field (regenerated on Node 22), the CHANGELOG entry (moved from Maintenance to Changed, since this drops support for a runtime rather than just tidying dependencies), and test.javascript.storage.gcp.kms.yml's node-version, which was testing exactly the runtime the package now declares unsupported. * docs(sdk/javascript): correct the Node 22 floor entry in the GCP KMS storage CHANGELOG (KSM-1534) The conflict resolution in 46b4768 placed the KSM-1534 entry between the KSM-1450/KSM-1458 symlink entry and the KSM-1514 entry, so the KSM-1514 entry's "Combined with the entry above" pointed at the Node engines entry. Moved the KSM-1534 entry to the end of the Changed list. Rewrote the KSM-1534 entry to describe the released before-and-after state. No released version declared >=20, and 1.0.0, the only published version, installs on Node 20 with no EBADENGINE warning at all (checked with a fresh install on Node 20.20.1). So 1.1.0 drops support that 1.0.0 had, and the entry now says so directly. --------- Co-authored-by: Mateo Gallego <mgallego@keepersecurity.com>
Run the GCP KMS storage tests on pull requests into master and on changes to the workflow file itself. Restrict the job token to contents: read, pin checkout and setup-node to the SHAs the publish workflows use, drop checkout credential persistence, and disable the npm cache to match the publish job.
…lds id-token: write publish-npm previously checked out source, ran npm ci, and rebuilt from scratch in the same job that holds the OIDC token, so any install-lifecycle script in that job's dependency tree could mint a trusted-publishing token. It now downloads the exact artifact build-npm already tested and publishes it unmodified. Extended that artifact to include README.md and CHANGELOG.md, which package.json's files field ships but the upload previously missed. Also pins npm to 11.6.2 (verified >=11.5.1, required for OIDC trusted publishing) instead of floating @latest, and deletes the dead .npmrc left over from before this workflow moved off NPM_TOKEN.
A second init() on the instance that called decryptConfig(true) skips the write, because the config hash did not change. The file stays in plaintext. Point the README and the KSM-1536 changelog entry at a new GCPKeyValueStorage instance, which re-encrypts correctly. KSM-1554 tracks the code fix.
…ly (#1205) * fix(gcp): stop importing google-auth-library directly, fixes Yarn PnP install and ADC getToken() google-auth-library was imported directly in GcpKmsClient.ts but never declared in package.json. npm hid this, since @google-cloud/kms pulls a copy in transitively through google-gax. Yarn's default Plug'n'Play linker (Yarn 2+) and pnpm with hoist=false both refuse it: importing the package failed immediately with "tried to access google-auth-library, but it isn't declared in its dependencies" (KSM-1553). The same standalone JWT object was also the cause of getToken() resolving to undefined when GCPKSMClient was constructed with no arguments (the Application Default Credentials path), since that constructor never set it. A RAW_ENCRYPT_DECRYPT key on that path silently took the wrong crypto path (KSM-1524). getToken() now reads the access token from KMSClient.auth, the GoogleAuth instance @google-cloud/kms already constructs internally, which is set on every construction path. KSM-1553, KSM-1524 * fix(gcp): address review feedback on auth, crypto, and PnP guard - getToken() now catches auth-library errors and rethrows only the message, since the raw error object can carry a refresh token or subject token that the library's own redactor does not strip - removed scopes from the explicit-credential KMS clients, restoring the audience-bound self-signed JWT instead of a scope-bound OAuth token usable against any Google API - a RAW_ENCRYPT_DECRYPT key with a falsy token now throws a named error instead of silently taking the gRPC path - added a real-auth-stack test file (no mocks on google-auth-library or gaxios) so a construction path that silently used ADC instead of the caller's credentials fails a test, not just a mock assertion - added a Yarn PnP install step to CI so an undeclared dependency fails the build instead of reaching a release - corrected README/CHANGELOG claims about Yarn 2 PnP support (never true, unrelated pino issue predating this PR) and pnpm hoisting (must be set in pnpm-workspace.yaml, .npmrc is ignored) KSM-1553, KSM-1524 * fix(gcp): close mutation-coverage gaps and doc inaccuracies from round 2 review real-auth test file now isolates ADC/gcloud/metadata-server reach and covers all three construction routes for token identity and gRPC JWT audience, plus two GCPKeyValueStorage-level tests for raw vs. symmetric key routing. Reverts the npm test script to plain jest. Adds the same Yarn PnP install guard to the publish workflow's build-npm job, since that job (not the PR-triggered test workflow) is what gates a release. Corrects the pnpm claim and removes the inaccurate Security changelog entry (no shipped release had the refresh-token leak). KSM-1553, KSM-1524 * chore(gcp): drop scaffolded test comment labels and a stray ticket ref Given/When/Then labels on single-assertion tests, and a Jira ticket reference in a workflow comment, add nothing a reader can't already see from the code and the commit history. * ci(gcp): run the publish PnP smoke after the upload and wait for the logger worker - publish.npm.storage.gcp.kms.yml: the Plug'n'Play smoke step installs the newest in-range dependencies with no lockfile and runs their code, so it now runs after "Upload NPM package". The artifact that publish-npm ships is fixed before any unpinned code runs; a failure still fails build-npm, which publish-npm needs. - Both smoke scripts no longer call process.exit(0) right after getToken() rejects. The process now ends after the pino-pretty worker loads or fails, so an undeclared dependency reachable only from the worker fails the step again. An unref'd 60 s watchdog turns a hang into a failure. - The raw-key storage test also calls decryptConfig(false), which fetches its own token, and checks that it reaches rawDecrypt with the ADC token. - Correct the smoke comment that said no network access is available. KSM-1553, KSM-1524 --------- Co-authored-by: Mateo Gallego <mgallego@keepersecurity.com>
Since the symlink read protection landed, every save path checks the config path with lstat and rejects a symbolic link before the atomic rename runs. Only a hard-linked peer is still replaced silently. The README and CHANGELOG still said a symlinked path is replaced on write.
This branch is waiting to be deployed
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
2 participants
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
Release branch for
@keeper-security/secrets-manager-gcpv1.1.0. The release hardens how the storage reads and writes its config file, fixes installs under Yarn Plug'n'Play andgetToken()on Application Default Credentials (ADC), and requires Node.js 22.Changes
Bug Fixes
hoist: false(KSM-1553): The package importedgoogle-auth-librarybut did not declare it, so installs failed under these linkers. The package no longer imports it.getToken()on ADC (KSM-1524):getToken()returnedundefinedon the ADC path. ARAW_ENCRYPT_DECRYPTkey then took the wrong crypto path.getToken()now reads the token fromKMSClient.auth, which every construction path sets. ARAW_ENCRYPT_DECRYPTkey with no token now throws a named error.createConfigFileIfMissing()recreates the config file only onENOENT. Other existence-check failures now propagate instead of an overwrite.loadConfig()anddecryptConfig()now throw on a zero-length file and leave it untouched. Before,loadConfig()re-encrypted an empty config over the top.KSM_CONFIG_FILEorkeyVaultConfigFileLocationnow falls back to the next source. Before, it resolved to the current working directory.saveConfig()no longer skips the write when the config file was deleted while the process ran. OnlyENOENTcounts as deleted. Any other access failure rejects the save instead of an overwrite.changeKey()rollback (KSM-1461): A failedchangeKey()now restores the key type and encryption algorithm, not only the key config.init()(KSM-1516): Every public method exceptinit()rejects withGCPKeyValueStorageErroruntilinit()completes.loadConfig()rejects a parsed config that is not an object of string values, before any write.loadConfig()anddecryptConfig()refuse a symlinked config path. On POSIX, the read also checks file owner and mode, and a new config directory gets mode 0700. On Windows, the read checks neither owner nor mode, anddecryptConfig()does not refuse a symlink. The README and CHANGELOG say so.axiosfloor to^1.20.0, so npm dedupe cannot keep an unpatched version.Maintenance
@google-cloud/kmsfrom^5.2.1to^6.2.0. The 6.x line drops eight transitive dependencies from the production tree (KSM-1510).@tootallnate/onceoverride (KSM-1534).decryptConfig(true)plaintext warning,KSM_CONFIG_FILEdocs, and corrected IAM roles.CHANGELOG.mdnow ships in the npm tarball (KSM-1536, KSM-1509).atomicWritemutation gaps (KSM-1517).build-npmjob usesnpm ci, and new jobs stop a dispatch for a version that npm already has (KSM-1530).build-npmtested, so no install script runs while the job holdsid-token: write. The job uses the npm that Node 24 bundles, and the unused.npmrcis removed.masterand on changes to its own file. Its token iscontents: read, and its actions are SHA-pinned.build-npmjob now install the packed tarball under Yarn Plug'n'Play and callgetToken()offline. An undeclared dependency now fails the build (KSM-1553).Breaking Changes
engines.nodeas>=22, because@google-cloud/kms6.x requires it. On Node 20 or 21, npm prints a warning, and--engine-strictfails the install. Upgrade the runtime to Node 22 before you upgrade this package.EACCES.keyVaultConfigFileLocationorKSM_CONFIG_FILEat the real file.getToken()rejects withGCPKeyValueStorageError(KSM-1524). On failure,getToken()no longer passes on the auth-library error object, because that object can hold a refresh token. A caller that readserr.statusorerr.codefrom a rejectedgetToken()call no longer gets them.Related Issues