feat: transient private-registry auth for deploy_component#1158
feat: transient private-registry auth for deploy_component#1158kriszyp wants to merge 7 commits into
Conversation
Add an optional `registryAuth` array to deploy_component carrying private npm registry tokens. The deploying node materializes a per-deploy 0600 `.npmrc` (in a 0700 temp dir) that `npm pack`/`npm install` authenticate against, then removes it; the token is held only in memory and that transient file. The token is stripped from the request before replication and from the operations log, so it never persists to config, hdb_deployment, the replication channel, or logs. Peers reinstall the package via their own fabric-injected NPM_CONFIG_USERCONFIG. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…d npmrc Addresses two Codex review findings on the transient registry-auth path: - A scope-less registryAuth entry now also emits a default `registry=` line so an unscoped package spec (npm:my-private-app) and its transitive deps resolve against the supplied private registry instead of silently falling back to npmjs (the token would otherwise never be used). Scoped entries still route only their @scope. - writeTransientNpmrc now prepends any inherited npm_config_userconfig (e.g. a fabric-injected file with cluster registries, proxy, or cafile) and appends the transient auth last so it wins on conflict, instead of clobbering those settings. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces transient private-registry authentication for deploying components by writing credentials to a temporary .npmrc file during installation and cleaning them up immediately afterward. The code reviewer provided valuable feedback to enhance security and robustness, including deleting sensitive credentials immediately after instantiation to prevent leakage on failure, wrapping the cleanup directory removal in a try-catch block to avoid masking deployment errors, and adding defensive checks to prevent directory leaks if the initialization is called multiple times.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
|
Reviewed; no blockers found. |
Address cross-model (gemini) review findings on the private-registry deploy auth path, all defense-in-depth for the invariant that the token never survives into a log/error path or replication: - operations.js: strip req.registryAuth immediately after the Application ctor captures it, instead of after loadComponent. The prior strip ran only on the success path, leaking the token in req if prepareApplication/loadComponent threw. Removes the now-redundant later delete. - Application.cleanupTransientNpmrc: wrap rm in try/catch so a failure (e.g. a Windows file lock) can't mask the original deploy error or skip broadcastDeployEnd; state is always cleared in finally. - Application.writeTransientNpmrc: clean up a prior temp dir if called twice, so the earlier 0700 dir + token file isn't leaked. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
Address claude[bot] review findings on the private-registry deploy auth path: - operationsValidation.js: forbid CR/LF in registry and token. Both are written verbatim into the transient line-based .npmrc, so a super_user could otherwise inject arbitrary npm config lines (redirect scopes/registries, set other keys). Uses a newline guard rather than a strict URI validator because registry also accepts bare hosts and //host/ forms. Adds tests for both injection paths plus a bare-host case to pin that the guard doesn't over-restrict. - Application.cleanupTransientNpmrc: also clear this.registryAuth so the plaintext token array can't surface in a later heap dump or error serialization of the Application instance. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…auth-core # Conflicts: # components/operationsValidation.js # unitTests/server/fastifyRoutes/operationsValidation.test.js
|
FAB-356 alignment note (cc @heskew, since you own FAB-356 and review here) This PR and the GitHub-App private-repo design share the same core invariant — keep the deploy credential out of the replicated operation body / WAL — so they're complementary, not competing (npm
Nothing actionable for this PR; just flagging so the F-1/F-4/F-5 work accounts for the npm path. Happy to pair on the — Claude |
1 participant
Summary
registryAuth: [{ registry, token, scope? }]todeploy_component, letting a deploy authenticatenpm pack/installagainst a private registry forpackage=npm:@org/app@x.y.zreferences..npmrc(in a 0700 tmpdir), injected vianpm_config_userconfigfor the deploy's npm spawns, and removed on completion. It is stripped from the operation before replication and from the operations log — so it never persists on disk, reaches peers, lands in logs, or enters the package reference /harperdb-config.yaml/hdb_deploymentrow.Context / why
Deploy-by-reference (vs. payload) is cleanly recorded, revertible, and ships no blob — but a private registry needs npm auth, which previously had to be hand-placed per host. This adds a managed, transient credential path. In the cluster, the Harper fabric injects
NPM_CONFIG_USERCONFIGon peers; this PR is the data-plane half that handles an origin-supplied token without durably persisting it.Where to focus
components/operations.js):delete req.registryAuthruns unconditionally beforereplicateOperation;serverHelpers/serverUtilities.tsstrips it from the ops log..npmrclifecycle (components/Application.ts):writeTransientNpmrc/cleanupTransientNpmrc, 0600 file / 0700 dir,npm_config_userconfiginjection innonInteractiveSpawn.registry=(so an unscoped spec resolves against the private registry) — which requires that registry to serve/proxy public deps. Scoped entries route only their@scope.writeTransientNpmrcprepends any inheritednpm_config_userconfig(e.g. a fabric-injected file) so its proxy/cafile/other registries survive, appending the transient auth last (npm last-value-wins).Paired with the harper-pro wrapper PR (log redaction + this core bump). Unit coverage added for
buildNpmrcContent, the.npmrclifecycle/merge, and thedeploy_componentvalidator.🤖 Generated by Claude (Opus 4.7).