Skip to content

feat: transient private-registry auth for deploy_component - #1158

Closed
kriszyp wants to merge 8 commits into
mainfrom
kris/registry-deploy-auth-core
Closed

feat: transient private-registry auth for deploy_component#1158
kriszyp wants to merge 8 commits into
mainfrom
kris/registry-deploy-auth-core

Conversation

@kriszyp

@kriszyp kriszyp commented Jun 8, 2026

Copy link
Copy Markdown
Member

Summary

  • Adds optional registryAuth: [{ registry, token, scope? }] to deploy_component, letting a deploy authenticate npm pack/install against a private registry for package=npm:@org/app@x.y.z references.
  • The token is written to a transient 0600 .npmrc (in a 0700 tmpdir), injected via npm_config_userconfig for 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_deployment row.

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_USERCONFIG on peers; this PR is the data-plane half that handles an origin-supplied token without durably persisting it.

Where to focus

  • Security flow (components/operations.js): delete req.registryAuth runs unconditionally before replicateOperation; serverHelpers/serverUtilities.ts strips it from the ops log.
  • Transient .npmrc lifecycle (components/Application.ts): writeTransientNpmrc/cleanupTransientNpmrc, 0600 file / 0700 dir, npm_config_userconfig injection in nonInteractiveSpawn.
  • Two design decisions worth a second opinion:
    1. A scope-less entry sets npm's default 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.
    2. writeTransientNpmrc prepends any inherited npm_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 .npmrc lifecycle/merge, and the deploy_component validator.

🤖 Generated by Claude (Opus 4.7).

kriszyp and others added 2 commits June 7, 2026 18:20
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 gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread components/operations.js Outdated
Comment thread components/Application.ts
Comment thread components/operations.js Outdated
Comment thread components/Application.ts
@claude

claude Bot commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

Reviewed; no blockers found.

@kriszyp
kriszyp requested review from Ethan-Arrowood and heskew June 8, 2026 12:30
kriszyp and others added 2 commits June 8, 2026 07:32
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>
Comment thread components/operationsValidation.js
Comment thread components/Application.ts
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>
@kriszyp
kriszyp marked this pull request as ready for review June 8, 2026 14:59
kriszyp added 2 commits June 8, 2026 15:46
…auth-core

# Conflicts:
#	components/operationsValidation.js
#	unitTests/server/fastifyRoutes/operationsValidation.test.js
@kriszyp

kriszyp commented Jun 9, 2026

Copy link
Copy Markdown
Member Author

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 npm: refs vs https://github.com/… git clones, alongside the existing git+ssh:// path). Two overlaps worth capturing now so the F-* work lands cleanly on top of this:

  1. Spawn hardening (F-5). This PR injects npm_config_userconfig into the shared nonInteractiveSpawn, which runs shell: true (components/Application.ts ~L827/L835). F-5 plans to fork deploy-time spawns into a shell: false nonInteractiveSpawnDeploy precisely to close the credential-bearing-env + shell:true RCE class. When that fork lands, the npmUserconfigPath threading needs to move onto the hardened deploy helper rather than stay on the legacy shell:true one — otherwise private-registry npm pack/install auth gets stranded. (Lower exposure than the git case here — npm_config_userconfig is a file path, not a free-form URL — but same risk class, so it should ride the same fix.)

  2. Shared deployComponent surface (F-4). This PR already does strip-before-replicate (delete req.registryAuth before replicateOperation) and uses the token only locally on the originator. F-4's originator/peer branch + identity-only req.credentials object touches the same components/operations.js#deployComponent, so it'd be good to write F-4 knowing registryAuth is already handled this way — the two should compose rather than collide.

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 nonInteractiveSpawnDeploy fork when it comes up.

— Claude

@kriszyp
kriszyp marked this pull request as draft June 11, 2026 20:03
@kriszyp kriszyp added this to the v5.2 milestone Jun 14, 2026
…auth-core

# Conflicts:
#	components/Application.ts
#	components/operations.js
@kriszyp

kriszyp commented Jul 8, 2026

Copy link
Copy Markdown
Member Author

Superseded by #1717, which carries the full feature — the literal-token form from this PR plus a secret-reference form ({ registry, secret }) that resolves the token from the hdb_secret store (#1550/#1582) instead of embedding it. #1717 is stacked on #1582; closing this to consolidate on that branch.

🤖 Closed by KrAIs on Kris's behalf.

@kriszyp kriszyp closed this Jul 8, 2026
kriszyp pushed a commit that referenced this pull request Jul 8, 2026
… every install

Completes the reference-not-embed story: a provided registry token is no longer a
transient, this-node-only credential — it is ingested into the encrypted, replicated
hdb_secret store and referenced everywhere, so package deploys survive rollback,
reboot, and new peers without the operator re-supplying the token.

- ingestRegistryAuth() (secretOperations): a literal `{ registry, token }` is sealed
  into hdb_secret (via set_secret, custody required) under a derived name
  (deploy.<component>.<registry>) granted to the component, and returned as a
  `{ registry, secret }` reference. Idempotent on rotation. Already-reference entries
  and (no-custody) literal tokens pass through untouched.
- deployComponent: ingests up front, then records references in TWO durable places —
  the component config (applicationConfig.registryAuth, read on every cold install) and
  the hdb_deployment row (registry_auth, the rollback source). req.registryAuth now
  replicates as references, never tokens; peers resolve from their own replicated
  hdb_secret copy (resolveRegistryAuth gains a bounded waitMs to cover the row arriving
  just behind the deploy op). No-custody core stays on the transient #1158 fallback.
- installApplications(): resolves applicationConfig.registryAuth at cold install so a
  fresh/wiped node or new peer authenticates from the store (best-effort — logs and
  installs without auth if custody isn't up yet, rather than blocking boot).
- assertApplicationConfig + deployment row + config type: registryAuth is references
  only; a literal token on disk is rejected.

Fabric NPM_CONFIG_USERCONFIG injection (harper-pro) becomes redundant for auth once
peers resolve from the store — a coordinated harper-pro follow-up removes the token
injection; core keeps userconfig *inheritance* for non-auth npm config (proxy/cafile).

Tests: ingest seal/round-trip/passthrough/no-custody/idempotent, derived-name
sanitization, resolve bounded-wait (times-out-404 + replicates-in-mid-wait),
assertApplicationConfig references-only.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
kriszyp added a commit that referenced this pull request Jul 10, 2026
* feat: transient private-registry auth for deploy_component

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>

* fix: route default registry for scope-less auth and preserve inherited 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>

* harden transient registryAuth token handling on deploy_component

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>

* guard registryAuth against .npmrc line injection; clear in-memory token

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>

* feat: two-tier component secret delivery + env declarations (#1550)

Consumption side of the hdb_secret store (#1554): rows with empty grants
are decrypted and materialized into the real process.env before
components load (pre-existing real env wins); rows with non-empty grants
never touch process.env and are exposed only through the per-component
secrets accessor (import { secrets } from 'harper' / scope.secrets),
which also carries the component's declared global-tier names.

Component configs declare env expectations in an `env:` block (string =
inline literal with .env semantics incl. enc:v1:; object = declaration
satisfied from the store). Unsatisfied required declarations gate that
component's load (missing | ungranted | custody-unavailable) while the
instance keeps running, and the declared-but-unsatisfied set is exposed
(metadata only) via get_components.

Under the vm/compartment loaders the accessor binds exactly via the
per-scope harper module; under the native loader the process-wide export
resolves through a component-load AsyncLocalStorage context and fails
loudly from ambiguous contexts.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix: address Codex review findings on secret delivery

- gate failure no longer installs an ErrorResource at '/' (could clobber
  the root URL space of unrelated components); containment is status +
  log + absent URL space
- env literals apply only after the whole block validates and the gate
  passes (no partial process.env mutation from a gated component)
- load cycles reset per-component declaration state so removed
  components/env blocks don't leave stale accessor names or stale
  unsatisfiedEnv in get_components
- scope.secrets keys by ApplicationScope.name (grants identity), not
  Scope#appName, which diverges on RUN_HDB_APP paths
- env-declaring component loads refresh the store snapshot, so deploy
  validation in a long-lived worker gates against post-boot
  set_secret/grant_secret changes
- secrets proxy rejects preventExtensions/setPrototypeOf (freezing the
  shared proxy would break key-set invariants for all later consumers)

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix: drop cycle-level declaration reset (main-thread reload never reprocesses)

loadedPaths is never cleared in production, so already-loaded components
early-return on main-thread reload cycles — a cycle-level registry wipe
would permanently empty get_components' unsatisfiedEnv after the first
reload. Registries are overwrite-on-reprocess instead; deleted
components' state is unreachable from get_components.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix: address Gemini review findings on secret delivery

- single-flight materializeGlobalSecrets: concurrent component loads
  share one hdb_secret scan (no N-way scan/decrypt herd at boot) and
  two scans can never interleave (older scan can't overwrite newer
  state); sequential callers still get fresh reads
- gate accepts a real env var fallback for a granted-but-undecryptable
  row, matching what the accessor would serve at runtime
- accessor views are null-prototype so Object.prototype names can't
  masquerade as secret values under dynamic access
- document that env literals share process.env cross-component (load-
  order visibility, same as .env today)
- regression test: component binding propagates into native-loader ESM
  top-level evaluation (incl. destructure + post-top-level-await) —
  empirical guard for the ALS/dynamic-import claim (dismissed on
  evidence for our supported Node range)

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix: make secrets proxy enumeration traps inspect-safe outside a binding context

has/ownKeys/getOwnPropertyDescriptor now report an empty object when no
component-load binding is active, so inspectors/serializers (util.inspect,
spread, `in`) can never crash the process; direct property reads stay loud.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* refactor: move SecretsView type into componentSecrets, import in index

Addresses Dawson's review comment: the per-component secrets view type was
defined inline in index.ts; export it as SecretsView from componentSecrets.ts
(next to the accessor it describes) and import it as a type in index.ts.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* feat(secrets): key the process.env delivery tier on the processEnv flag (consumption)

Consumption side of the explicit-tier change (#1582): materialize a row into the
real process.env when `processEnv: true` rather than when `grants` is empty, and
key the declaration gate + accessor on the same flag. A row with neither
processEnv nor grants is now inert (visible to no component) until granted —
omission reads as restrictive, matching the review consensus.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* feat: resolve deploy_component registryAuth from the hdb_secret store (reference, not embed)

registryAuth entries may now name an hdb_secret row (`{ registry, secret }`)
instead of carrying a literal `{ registry, token }`. The token is resolved by
decrypting the referenced row on the deploying (main) thread, where the
operations API dispatches and the Pro secrets component registers custody —
so the credential lives in the replicated, audited secrets store (#1550/#1582)
rather than travelling in the operation body.

- operationsValidation: each entry is `token` XOR `secret` (secret uses the
  set_secret name grammar); the literal-token form is unchanged.
- secretOperations.resolveRegistryAuth(): decrypts secret-backed entries; literal
  tokens pass through untouched (no custody/store needed on that fast path).
  Authority mirrors the accessor model — the secret must be processEnv-global or
  granted to the component being deployed, else 403; missing row → 404; absent
  custody or a decrypt failure fails the deploy loudly.
- operations.deployComponent: resolves before constructing the Application; the
  resolved token gets the same transient handling as a literal token (transient
  .npmrc, stripped from req before replication, kept out of the ops log).

Peers still authenticate via fabric-injected NPM_CONFIG_USERCONFIG; origin-side
resolution keeps cluster behavior identical. Peer-side ref resolution (each peer
decrypting its replicated hdb_secret copy) is a possible follow-up.

Unit coverage: resolveRegistryAuth pass-through/resolve/global/mixed + all four
failure reasons; validator token-XOR-secret cases.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* feat: persist provided registry tokens as hdb_secret refs; resolve on every install

Completes the reference-not-embed story: a provided registry token is no longer a
transient, this-node-only credential — it is ingested into the encrypted, replicated
hdb_secret store and referenced everywhere, so package deploys survive rollback,
reboot, and new peers without the operator re-supplying the token.

- ingestRegistryAuth() (secretOperations): a literal `{ registry, token }` is sealed
  into hdb_secret (via set_secret, custody required) under a derived name
  (deploy.<component>.<registry>) granted to the component, and returned as a
  `{ registry, secret }` reference. Idempotent on rotation. Already-reference entries
  and (no-custody) literal tokens pass through untouched.
- deployComponent: ingests up front, then records references in TWO durable places —
  the component config (applicationConfig.registryAuth, read on every cold install) and
  the hdb_deployment row (registry_auth, the rollback source). req.registryAuth now
  replicates as references, never tokens; peers resolve from their own replicated
  hdb_secret copy (resolveRegistryAuth gains a bounded waitMs to cover the row arriving
  just behind the deploy op). No-custody core stays on the transient #1158 fallback.
- installApplications(): resolves applicationConfig.registryAuth at cold install so a
  fresh/wiped node or new peer authenticates from the store (best-effort — logs and
  installs without auth if custody isn't up yet, rather than blocking boot).
- assertApplicationConfig + deployment row + config type: registryAuth is references
  only; a literal token on disk is rejected.

Fabric NPM_CONFIG_USERCONFIG injection (harper-pro) becomes redundant for auth once
peers resolve from the store — a coordinated harper-pro follow-up removes the token
injection; core keeps userconfig *inheritance* for non-auth npm config (proxy/cafile).

Tests: ingest seal/round-trip/passthrough/no-custody/idempotent, derived-name
sanitization, resolve bounded-wait (times-out-404 + replicates-in-mid-wait),
assertApplicationConfig references-only.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* style: prettier formatting for registry-auth changes

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix: address review — npmrc newline invariant, 5xx for server-state resolve failures

Addresses PR #1717 review comments:

- buildNpmrcContent: enforce the no-CR/LF invariant at the .npmrc write
  boundary so it holds for tokens resolved from hdb_secret rows, not just
  validator-guarded literal tokens (cb1kenobi/Barber AI).
- resolveRegistryAuth: report no-custody as 503 and decrypt failure as 500
  instead of the ClientError default 400 — these are server-state, not
  client-fixable (cb1kenobi/Barber AI).
- resolveRegistryAuth: skip null entries for parity with the fast-path guard
  (gemini-code-assist; no 'SKIP' sentinel exists in this path).
- Tests: newline-guard rejection + status-code assertions on the two
  server-state failures.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* refactor(components): hoist deploy class to top-level scope (#1717)

Hoist the throwaway InvalidRegistryAuthError classes (defined inline on
every assertApplicationConfig call) to named top-level classes,
InvalidRegistryAuthPropertyError and InvalidRegistryAuthEntryError,
matching the existing InvalidInstall*Error convention in the file. Also
fixes prettier formatting on secretOperations.test.js flagged by CI.

* style: fix prettier 3.9 formatting in Application.ts

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
Co-authored-by: Kris Zyp <kris@harperdb.io>
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