Add per-build type LavaMoat policies#12702
Conversation
| # Generate LavaMoat policies for the extension background script for each build | ||
| # type. | ||
| concurrently --kill-others-on-fail -n main,beta,flask \ | ||
| "WRITE_AUTO_POLICY=1 yarn dist" \ | ||
| "WRITE_AUTO_POLICY=1 yarn dist --build-type beta" \ | ||
| "WRITE_AUTO_POLICY=1 yarn dist --build-type flask" |
There was a problem hiding this comment.
Note: Your device may board the Struggle Bus when running this locally.
Builds ready [e7876fb]Page Load Metrics (730 ± 14 ms)
|
|
this sounds like hell for the security engineers charged with reviewing policies and policy changes |
README.md
Outdated
| * Unfortunately, `yarn lavamoat:auto` will behave inconsistently on different platforms. macOS and Windows users may see extraneous changes relating to optional dependencies. | ||
| * The build system LavaMoat policy file (`lavamoat/build-system/policy.json`) | ||
| * Run `yarn lavamoat:build:auto` to re-generate this policy file. | ||
| * This only applies if the external modules imported by the build system or its dependencies have changed. Keep in mind that some modules may be imported dynamically, and may elude LavaMoat's static analysis. If this occurs, debug the build system with [`ndb`](https://www.npmjs.com/package/ndb) to determine which packages are dynamically imported, add them to the "useless" imports in `development/build/index.js`, and re-run the auto policy script. |
There was a problem hiding this comment.
This advice seems... overly specific? Dynamic imports aren't the only Lavamoat-related problem people are likely to run up against, nor is it specific to the build system. Other problems include dynamic global usage, attempts to mutate frozen globals, and incorrect policy generation due to multiple copies of a dependency being in the dependency tree.
| # type. | ||
| # ATTN: This may tax your device when running it locally. | ||
| concurrently --kill-others-on-fail -n main,beta,flask \ | ||
| "WRITE_AUTO_POLICY=1 yarn dist" \ |
There was a problem hiding this comment.
Is this safe to run concurrently? All three of these builds will write to the dist directory, so I'd expect these to be overwriting each other.
There was a problem hiding this comment.
Hm. For the purposes of validating the LavaMoat config, it doesn't matter what's in the dist directory or whether files are being overwritten. The build processes just need to complete normally. We could really use a --no-emit option, actually.
My question is whether this could interfere with any of the other build scripts we run, if they also write or read from dist?
There was a problem hiding this comment.
My concern was that LavaMoat itself might be generating the policy files from-disk. So the version of the build it was generating a policy for might be reliant on this race condition of which build finishes when.
But on second thought... that's not how any of this works. LavaMoat runs on the JavaScript while it's still in stream form, before it gets written to disk. At least I'm pretty sure. So yeah you're probably right.
And I agree, a --no-emit option would be useful here.
There was a problem hiding this comment.
im not sure if:
cleanis safe (removes and creates the dir)- inpage/contentscript is safe (creates the inpage bundle and puts it in the contentscript bundle)
but if it works it works
Co-authored-by: Mark Stacey <markjstacey@gmail.com>
Gudahtt
left a comment
There was a problem hiding this comment.
LGTM! Though regarding that README comment, I think that could be improved by moving that debugging advice to a separate bullet point or section. So that it can be expanded later, and inclusive of more than just the build system policy.
Builds ready [bb28367]Page Load Metrics (744 ± 19 ms)
|
This PR adds one LavaMoat background script policy or each build type. It also renames the build system policy directory from
nodetobuild-systemto make its purpose more clear. Each build type has the originalpolicy-override.jsonformainbuilds.We need to maintain separate policies for each build type because each type will produce different bundles with different internal and external modules.
In addition to the LavaMoat-related changes, this PR updates the lint scripts to no longer lint the auto-generated LavaMoat policy files. This should save off a significant amount of time from our JSON linting (which we do with
prettierdirectly).Closes #12690