fix: regenerate lavamoat policy for Docker compatibility#590
fix: regenerate lavamoat policy for Docker compatibility#590andreabadesso wants to merge 1 commit intomasterfrom
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughUpdates Lavamoat policy configuration adding many new public entries and reorganizing side-channel mappings (yargs ecosystem, ws, express>qs>side-channel, lavamoat resolve entries) and tightens several browser-like globals; README adds Docker and from-source run instructions. (50 words) Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #590 +/- ##
=======================================
Coverage 90.08% 90.08%
=======================================
Files 56 56
Lines 2462 2462
Branches 456 456
=======================================
Hits 2218 2218
Misses 226 226
Partials 18 18 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
76d7e03 to
d39d50b
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
lavamoat/node/policy.json (1)
901-914: Redundant lru-cache references through both dev and prod paths.The
jsonwebtoken>semverentry references bothnodemon>semver>lru-cache(dev path) andjsonwebtoken>semver>lru-cache(prod path). In a production Docker environment without dev dependencies, the nodemon path won't exist.If the policy was generated correctly in the Docker build stage (as per PR objectives), only the production path should be needed. Consider removing the nodemon reference to avoid confusion.
Proposed cleanup
"jsonwebtoken>semver": { "globals": { "console.error": true, "process": true }, "packages": { - "nodemon>semver>lru-cache": true, "jsonwebtoken>semver>lru-cache": true } },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lavamoat/node/policy.json` around lines 901 - 914, The policy lists both the dev and prod paths for lru-cache under "jsonwebtoken>semver"; remove the dev-only reference "nodemon>semver>lru-cache" from the "packages" object of "jsonwebtoken>semver" so only the production path "jsonwebtoken>semver>lru-cache" remains, and then verify there are no other dev-only references to "nodemon>..." elsewhere in the policy (leave "jsonwebtoken>semver>lru-cache" and its child "jsonwebtoken>semver>lru-cache>yallist" intact).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lavamoat/node/policy.json`:
- Around line 331-349: Remove all dev-only dependency entries that start with
the "eslint-plugin-import>" prefix from the policy object (e.g., keys like
"eslint-plugin-import>array-includes>get-intrinsic" and the nested "packages"
entries referencing "eslint-plugin-import>..."), leaving only production
dependency chains such as "express>qs>side-channel>get-intrinsic"; ensure you
delete the entire key/value block for each eslint-plugin-import entry so no
devDependency chains remain in the runtime policy.
---
Nitpick comments:
In `@lavamoat/node/policy.json`:
- Around line 901-914: The policy lists both the dev and prod paths for
lru-cache under "jsonwebtoken>semver"; remove the dev-only reference
"nodemon>semver>lru-cache" from the "packages" object of "jsonwebtoken>semver"
so only the production path "jsonwebtoken>semver>lru-cache" remains, and then
verify there are no other dev-only references to "nodemon>..." elsewhere in the
policy (leave "jsonwebtoken>semver>lru-cache" and its child
"jsonwebtoken>semver>lru-cache>yallist" intact).
d39d50b to
64e5bb6
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
lavamoat/node/policy-override.json (1)
41-243: Consider codifying the merge workflow in CI to avoid policy drift.Given the two-environment generation/merge requirement, a scripted check in CI would make future policy updates safer and reproducible.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lavamoat/node/policy-override.json` around lines 41 - 243, The repo needs a CI check that reproduces the two-environment Lavamoat policy generation and verifies the committed lavamoat/node/policy-override.json is the exact merged output to prevent drift; add a CI job (e.g., policy:verify) that runs the existing policy generation script for both environments (invoke the same generator used to produce policy-override.json), merges them the same way the manual process does, diff/compare the generated merge with lavamoat/node/policy-override.json, and fail the job if there are any differences so future PRs must update the committed file via the generator.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@lavamoat/node/policy-override.json`:
- Around line 41-243: The repo needs a CI check that reproduces the
two-environment Lavamoat policy generation and verifies the committed
lavamoat/node/policy-override.json is the exact merged output to prevent drift;
add a CI job (e.g., policy:verify) that runs the existing policy generation
script for both environments (invoke the same generator used to produce
policy-override.json), merges them the same way the manual process does,
diff/compare the generated merge with lavamoat/node/policy-override.json, and
fail the job if there are any differences so future PRs must update the
committed file via the generator.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3efb5c6f-73e9-4648-8ea7-9d150d41e1fe
📒 Files selected for processing (2)
lavamoat/node/policy-override.jsonlavamoat/node/policy.json
64e5bb6 to
3bae2b1
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lavamoat/node/policy.json`:
- Around line 264-277: The policy currently includes entries for
"express>qs>side-channel>call-bind" and
"express>qs>side-channel>call-bind>call-bind-apply-helpers" (and their
"get-intrinsic" children) but is missing the equivalent dev-path variants under
the eslint-plugin-import tree; add matching entries prefixed with
"eslint-plugin-import>qs>side-channel>..." that mirror the existing package
objects for the express entries (i.e., create
"eslint-plugin-import>qs>side-channel>call-bind",
"eslint-plugin-import>qs>side-channel>call-bind>call-bind-apply-helpers", and
the "get-intrinsic" child mappings), and duplicate these additions at the other
ranges mentioned (near lines 372–377, 414–418, 585–603, and 1026–1039) so the
policy contains both express- and eslint-plugin-import-prefixed paths for
call-bind, call-bind-apply-helpers, set-function-length, es-define-property, and
get-intrinsic.
In `@README.md`:
- Around line 20-25: The README currently shows secrets inline in the docker run
example; update it to demonstrate using an env-file or Docker secrets instead of
embedding HEADLESS_SEED_DEFAULT and HEADLESS_API_KEY directly. Replace the -e
HEADLESS_SEED_DEFAULT="your seed words here" and -e
HEADLESS_API_KEY=your_api_key parts in the hathornetwork/hathor-wallet-headless
docker run example with a note to use --env-file path/to/.env (or reference
Docker secrets) and show the .env keys (HEADLESS_SEED_DEFAULT and
HEADLESS_API_KEY) as placeholders, removing any literal secret values from the
example.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 897082ce-f9d6-4220-ad51-b92b27029aeb
📒 Files selected for processing (2)
README.mdlavamoat/node/policy.json
3562d93 to
9c9052e
Compare
f3b7ed1 to
b9d85f6
Compare
The lavamoat policy was generated against dev node_modules, but both Docker and the recommended production setup use production-only dependencies. This caused runtime policy violations because: 1. Shared transitive packages resolve through different parent chains in dev vs production node_modules. 2. config.js is loaded dynamically via a computed require() in settings.js, so lavamoat never discovers yargs and its deps. Regenerated the policy from production dependencies using a shim that statically requires config.js so lavamoat discovers the full dependency graph. Added README instructions for running from source with production deps to match the policy. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
b9d85f6 to
7897e56
Compare
Acceptance Criteria
make lavamoat_policyregenerates the policy correctlyWhat this PR does
The LavaMoat policy was out of date and the Docker image could not start. Two root causes:
node_modules, but the runtime (Docker and recommended from-source setup) uses production-only deps. npm resolves shared transitive packages through different parent chains in each case, so the policy didn't match.settings.jsloadsconfig.jsvia a dynamicimport(), so lavamoat's static analysis never discoveredyargsand its transitive deps (used inconfig.js.docker).This PR:
policy.jsonfrom production dependencies, using a shim entrypoint that statically requiresconfig.jsso lavamoat sees the full dependency graph.scripts/generate-lavamoat-policy.shand amake lavamoat_policytarget to automate policy regeneration (requires Docker).npm run lavamoat:policyshould no longer be used.Security Checklist