Skip to content

Commit 0ed387a

Browse files
authored
fix: visibility enforcement for local package deps (aspect-build#2357)
### Changes are visible to end-users: no ### Test plan - New test cases added Fixes: aspect-build#2345
1 parent d407a2e commit 0ed387a

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

42 files changed

+848
-16
lines changed

.aspect/workflows/config.yaml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,8 @@ workspaces:
125125
without: true
126126
- buildifier:
127127
without: true
128+
# No test targets. Requires running test.sh.
129+
# e2e/npm_translate_lock_package_visibility:
128130
e2e/npm_translate_lock_subdir_patch:
129131
icon: npm
130132
tasks:

.github/workflows/ci.yaml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@ jobs:
5656
e2e/npm_translate_package_lock
5757
e2e/npm_translate_yarn_lock
5858
e2e/npm_translate_lock_exclude_package_contents
59+
e2e/npm_translate_lock_package_visibility
5960
e2e/package_json_module
6061
e2e/patch_from_repo
6162
e2e/pnpm_lockfiles

MODULE.bazel

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -235,8 +235,16 @@ npm.npm_translate_lock(
235235
},
236236
npmrc = "//:.npmrc",
237237
package_visibility = {
238-
"unused": ["//visibility:private"],
239-
"@mycorp/pkg-a": ["//examples:__subpackages__"],
238+
"unused": ["//npm/private/test:__subpackages__"],
239+
"@mycorp/pkg-a": [
240+
"//examples:__subpackages__",
241+
"//js/private/test/image:__subpackages__",
242+
],
243+
"@mycorp/pkg-d": [
244+
"//examples:__subpackages__",
245+
"//js/private/test/image:__subpackages__",
246+
],
247+
"@mycorp/pkg-e": ["//examples:__subpackages__"],
240248
},
241249
pnpm_lock = "//:pnpm-lock.yaml",
242250
public_hoist_packages = {
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
node_modules
2+
packages/from-local/node_modules
3+
packages/from-root/node_modules
4+
packages/some-dep/node_modules
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
import %workspace%/../../tools/preset.bazelrc
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
../../.bazelversion
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
# Test npmrc file for visibility test
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
load("@npm//:defs.bzl", "npm_link_all_packages")
2+
3+
npm_link_all_packages(name = "node_modules")
4+
5+
# Simple test to make bazel test //... pass
6+
# The real visibility enforcement test is in test.sh
7+
sh_test(
8+
name = "placeholder_test",
9+
srcs = ["test_placeholder.sh"],
10+
)
11+
12+
# Create a simple passing test script
13+
genrule(
14+
name = "generate_placeholder_test",
15+
outs = ["test_placeholder.sh"],
16+
cmd = "echo '#!/bin/bash\necho \"Placeholder test passed\"\nexit 0' > $@",
17+
)
Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
bazel_dep(name = "aspect_rules_js", version = "0.0.0", dev_dependency = True)
2+
local_path_override(
3+
module_name = "aspect_rules_js",
4+
path = "../..",
5+
)
6+
7+
npm = use_extension(
8+
"@aspect_rules_js//npm:extensions.bzl",
9+
"npm",
10+
dev_dependency = True,
11+
)
12+
npm.npm_translate_lock(
13+
name = "npm",
14+
data = [
15+
"//:package.json",
16+
"//packages/from-local:package.json",
17+
"//packages/some-dep:package.json",
18+
],
19+
npmrc = "//:.npmrc",
20+
package_visibility = {
21+
"some-dep": ["//visibility:public"], # Public by default for CI, test.sh will make it private
22+
},
23+
pnpm_lock = "//:pnpm-lock.yaml",
24+
verify_node_modules_ignored = "//:.bazelignore",
25+
)
26+
use_repo(npm, "npm")
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
# Test coverage for package_visibility enforcement in npm_translate_lock
2+
3+
This test validates that `package_visibility` restrictions are properly enforced for workspace packages when using local node_modules references (`:node_modules/package`).
4+
5+
## Bug Description
6+
7+
Previously, workspace packages could bypass `package_visibility` restrictions by referencing packages locally (`:node_modules/some-dep`) instead of from the root (`//:node_modules/some-dep`). This security issue allowed unauthorized access to restricted packages.
8+
9+
## Test Validation
10+
11+
The test attempts to build `//packages/from-local:from_local_lib` which references `:node_modules/some-dep` locally. This should fail with a visibility error because `some-dep` is restricted to `//packages/from-root:__subpackages__` only.
12+
13+
## Expected Behavior
14+
15+
With the fix, local references create aliases that delegate to root targets, ensuring Bazel's visibility system is properly enforced regardless of reference style.

0 commit comments

Comments
 (0)