feat(devicepolicy): enforce npm secure-registry policy via managed ~/.npmrc block#174
Draft
raysubham wants to merge 5 commits into
Draft
feat(devicepolicy): enforce npm secure-registry policy via managed ~/.npmrc block#174raysubham wants to merge 5 commits into
raysubham wants to merge 5 commits into
Conversation
….npmrc block
Add the package_config#npm device-policy lane: converge a StepSecurity-owned
block in the console user's ~/.npmrc so npm (and the pnpm / yarn v1 / bun tools
that read the same file) resolve packages through the tenant's secure registry,
alongside the existing VS Code ide_extension lane.
The target is a file the agent may touch as root (macOS LaunchDaemon) against a
user-controlled home, so every file operation goes through Go 1.26 os.Root with
explicit symlink-chain resolution, post-open identity re-checks, and metadata
changes on open handles (never by path) to defeat symlink/entry-swap races.
Supporting pieces:
- an INI classifier that mirrors npm's own key/value parsing (spaced forms,
inline comments, single/double-quoted and JSON-escaped keys) and fails
closed on INI sections, bare CRs, and coercible non-string quoted keys it
cannot safely reason about;
- a cross-process reconciliation lock (flock / LockFileEx) serializing
scheduled and manual cycles per guarded file;
- a per-uid ownership state store kept out of the shared unlocked cache;
- reconciler seams (Converged / Render / ProbeExpected / RestoreSnapshot /
OwnsByMarker / State / ...) that leave the settings.json path byte-identical
when unset;
- a content-aware MDM probe, transactional write/clear with snapshot rollback,
and bounded, identity-checked backups.
Gated behind the existing device-policy feature gate.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
… convergence Remove the bespoke cross-process reconciliation lock (npmrc_lock.go, AcquireReconcileLock via flock/LockFileEx) from the package_config#npm enforce path. The process-wide singleton lock (internal/lock, acquired inside telemetry.Run) already serializes the scan phase across every invocation and privilege mode; only the enforce step runs outside it, and that overlap is atomic-write-safe: each ~/.npmrc write is a temp+rename of a deterministically rendered block, so concurrent cycles never observe a torn file. While a policy is stable both cycles render identical bytes; only a policy transition (a key rotation, or an enforce racing a clear) that interleaves with a concurrent cycle can briefly leave the superseded value, reconverged on the next cycle (eventual consistency). This matches the VS Code ide_extension lane, which enforces the same way with no lock. - delete npmrc_lock.go and the flock / LockFileEx helpers in the platform files; - set the reconciler seams directly in runPackageConfigEnforce; - correct the state-store comments that credited the removed lock; - drop the four lock lifecycle / contention tests. Also clear the residual gosec findings in npmrc.go the way the file already handles cleanup errors: _ = on error-path Close() calls, and a #nosec G115 justification on the POSIX uid conversion.
TestFileStateStoreRoundTrip asserted the state file's mode is exactly 0600, but Windows has no POSIX permission bits: os.Chmod only toggles the write bit and Stat reports 0666 for any writable file, so the check could never hold on Windows (the store's own logic is unaffected — the state file round-trips fine). Guard the assertion with runtime.GOOS != "windows", matching the existing settings_writer_test.go idiom; the rest of the round-trip still runs on every platform.
The package_config#npm enforcement lane is ready to ship, so remove its dedicated feature gate and run it unconditionally. The gate was default-on already, so runtime behavior is unchanged; the lane stays dormant in production until the backend returns a package_config policy (an absent policy is a no-op, never a wipe). The IDE-extension lane keeps its own FeatureDevicePolicy gate. Removes the FeaturePackageConfigPolicy const, its enabled-map entry, and the gate check in runPackageConfigEnforce.
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
Adds the
package_config#npmdevice-policy enforcement lane: the agent converges a StepSecurity-owned managed block in the console user's~/.npmrcso npm (and the pnpm / yarn v1 / bun tools that read the same file) resolve packages through the tenant's secure registry. It runs alongside the existing VS Codeide_extensionlane and reuses the same reconciler, driven through nil-able seams so the settings.json path stays byte-identical.What's included
~/.npmrcmanaged-block writer (npmrc.go,npmrc_unix.go,npmrc_windows.go) — operates on a user-owned tree the agent may touch as root (macOS LaunchDaemon). Every file op goes through Go 1.26os.Rootwith explicit symlink-chain resolution, post-open identity re-checks (Lstat + SameFile), and metadata changes on open handles only — never by path — to defeat symlink / directory-entry-swap races. Transactional write/clear with snapshot rollback, and bounded, identity-checked, 0600 token-bearing backups.[section]headers, bare\rline breaks, and single-quoted non-string JSON keys npm would coerce into an override.telemetry.Runreleases the process-wide singleton lock, so two cycles can overlap only in the write step. Each write is an atomic temp+rename of a deterministically rendered block: identical bytes while the policy is stable, and only a self-healing stale-value window if a policy transition (key rotation / enforce-vs-clear) interleaves with a concurrent cycle, reconverged next cycle. Same model as the lock-free VS Code lane.statestore.go) kept out of the shared device-policy cache so an IDE reconcile in another process can't drop the npm record — separate files make that cross-process lost update structurally impossible without a lock.reconcile.go:Converged/Render/ProbeExpected/RestoreSnapshot/OwnsByMarker/State/WriterInitErr) — every seam at its zero value reproduces the settings.json behavior byte-for-byte.~/.npmrcis user-writable, a bare marker isn't proof; the probe verifies the MDM lane's block is present, effective (last-wins), and correctly owned.Security model
A user who can plant symlinks or swap directory entries mid-operation must not be able to steer a root-owned write, a root read, or a token-bearing backup outside their own regular
~/.npmrc. Token material (<api_key>::dev:<serial>) is never logged, and offboarding removes every managed block — including duplicates — so no live token survives a clear.Testing
go test ./...— all packages pass, including-race.go vet+gofmtclean; builds green on darwin / linux / windows.Notes
package_configpolicy (which ships after this agent release), so the reconciler no-ops until then — an absent or transient policy is always a no-op, never a wipe. The IDE lane keeps its ownFeatureDevicePolicygate.npmrc_lock.go, flock / LockFileEx); it was removed (commit62d9e78) once it was clear the telemetry singleton lock already serializes the scan phase and the enforce overlap is atomic-write-safe. Accepted residual: a narrow, self-healing stale-value window when a policy transition overlaps a concurrent cycle — bounded to one lost cycle, never a corrupt file. Removing the lock also cleared its gosec findings; the remainingnpmrc.gocleanup-error and uid-conversion findings are handled in-convention (_ =/#nosec G115).Opening as a draft for review.