feat(registry): enforce per-package access policy from YAML#12043
Merged
Conversation
Derive PackagePolicies from each packages[*] entry's access/publish tokens in from_yaml/from_default_yaml, instead of hardcoding registry_mock_defaults — configured ACLs were silently ignored before, so a user-defined private scope got no protection. - Add the $anonymous AccessRule: admits only unauthenticated callers; an authenticated caller falls outside the group and gets 403. - Unknown access tokens now fail config parsing rather than silently mis-enforcing; named groups remain unsupported (tracked separately). - Programmatic Config::proxy / Config::static_serve keep registry_mock_defaults. Refs #11973.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📜 Recent review details⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
📝 WalkthroughWalkthroughConfig parsing now maps serde failures to RegistryError::InvalidConfig and compiles PackagePolicies from YAML package ChangesYAML-Driven Access Policy System
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #12043 +/- ##
==========================================
+ Coverage 88.73% 88.81% +0.07%
==========================================
Files 234 234
Lines 30461 30594 +133
==========================================
+ Hits 27031 27173 +142
+ Misses 3430 3421 -9 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
`mis-enforce` tripped the `typos` check (read as `mis` → `miss`/`mist`).
Review Summary by QodoWire per-package access policies from YAML config
WalkthroughsDescription• Compile per-package access policies from YAML packages block instead of hardcoding defaults • Add $anonymous access rule for unauthenticated-only access • Fail config parsing on unknown access tokens instead of silently mis-enforcing • Enforce compiled policies at request time via updated enforce_access logic Diagramflowchart LR
YAML["YAML packages block<br/>access/publish tokens"]
PARSE["parse_access_rule<br/>with defaults"]
BUILD["build_policies<br/>creates PackagePolicies"]
ENFORCE["enforce_access<br/>checks rule vs caller"]
RESPONSE["200/401/403<br/>response"]
YAML -->|"each entry"| PARSE
PARSE -->|"AccessRule"| BUILD
BUILD -->|"PackagePolicies"| ENFORCE
ENFORCE -->|"match rule<br/>& auth state"| RESPONSE
File Changes1. registry/crates/pnpm-registry/src/config.rs
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
registry/crates/pnpm-registry/tests/policy.rs (1)
70-82: ⚡ Quick winConsider asserting authenticated access succeeds for
@secret/*.This test only verifies the anonymous-denied (401) and
$all-fallthrough (404) cases. A regression where$authenticatedwrongly rejected every caller would still pass here. Adding the positive path rounds out the matrix.♻️ Add positive-case assertion
// `@secret/*` requires auth: an anonymous read is 401. assert_eq!(status_of(app.clone(), get("/@secret/thing")).await, StatusCode::UNAUTHORIZED); + // An authenticated caller passes the access check (404 = allowed but absent). + let token = add_user_and_get_token(&app, "bob", "secret").await; + assert_eq!( + status_of(app.clone(), get_auth("/@secret/thing", &token)).await, + StatusCode::NOT_FOUND + ); // A `**` ($all) package is readable anonymously — it's just absent // on disk, so 404, which proves the access check passed. assert_eq!(status_of(app, get("/lodash")).await, StatusCode::NOT_FOUND);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@registry/crates/pnpm-registry/tests/policy.rs` around lines 70 - 82, The test authenticated_access_token_from_yaml_gates_anonymous_reads currently only checks anonymous 401 and public 404; add a positive authenticated assertion to ensure $authenticated allows access: after building app via router(config), perform a GET to "/@secret/thing" using the existing test helpers (e.g., status_of and get) but include a valid authentication header/token (use the same auth mechanism used elsewhere in tests) and assert the response is not UNAUTHORIZED (expected NOT_FOUND if package is absent) to confirm authenticated callers pass the access check; reference the test function name authenticated_access_token_from_yaml_gates_anonymous_reads and helpers config_from_yaml, router, status_of, and get to locate where to add this assertion.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@registry/crates/pnpm-registry/src/config.rs`:
- Around line 540-543: build_policies() currently validates access and publish
but ignores access.unpublish, allowing unknown tokens to slip through; call
parse_access_rule on access.unpublish (e.g. let _unpublish_rule =
parse_access_rule(access.unpublish.as_deref(), AccessRule::None)? ) to validate
the unpublish token and propagate any parse errors before creating the
PackagePolicy (keep using PackagePolicy::new(pattern, access_rule, publish_rule)
if the API doesn't accept an unpublish rule).
---
Nitpick comments:
In `@registry/crates/pnpm-registry/tests/policy.rs`:
- Around line 70-82: The test
authenticated_access_token_from_yaml_gates_anonymous_reads currently only checks
anonymous 401 and public 404; add a positive authenticated assertion to ensure
$authenticated allows access: after building app via router(config), perform a
GET to "/@secret/thing" using the existing test helpers (e.g., status_of and
get) but include a valid authentication header/token (use the same auth
mechanism used elsewhere in tests) and assert the response is not UNAUTHORIZED
(expected NOT_FOUND if package is absent) to confirm authenticated callers pass
the access check; reference the test function name
authenticated_access_token_from_yaml_gates_anonymous_reads and helpers
config_from_yaml, router, status_of, and get to locate where to add this
assertion.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 7b3918b8-7d9f-48a9-87ed-f9689592aeea
📒 Files selected for processing (5)
registry/crates/pnpm-registry/src/config.rsregistry/crates/pnpm-registry/src/error.rsregistry/crates/pnpm-registry/src/policy.rsregistry/crates/pnpm-registry/src/server.rsregistry/crates/pnpm-registry/tests/policy.rs
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Lint and Test (windows-latest)
- GitHub Check: Lint and Test (macos-latest)
- GitHub Check: Lint and Test (ubuntu-latest)
- GitHub Check: Code Coverage
🧰 Additional context used
📓 Path-based instructions (1)
registry/crates/**/*.rs
📄 CodeRabbit inference engine (registry/AGENTS.md)
Follow the pacquet code-style guide (../pacquet/CODE_STYLE_GUIDE.md) for Rust-level conventions including imports, naming, ownership, error handling, and test layout
Files:
registry/crates/pnpm-registry/src/error.rsregistry/crates/pnpm-registry/tests/policy.rsregistry/crates/pnpm-registry/src/policy.rsregistry/crates/pnpm-registry/src/server.rsregistry/crates/pnpm-registry/src/config.rs
🔇 Additional comments (7)
registry/crates/pnpm-registry/src/error.rs (1)
35-37: LGTM!Also applies to: 52-59, 176-176
registry/crates/pnpm-registry/src/policy.rs (1)
14-16: LGTM!Also applies to: 24-28, 37-38, 173-195
registry/crates/pnpm-registry/src/config.rs (1)
9-10: LGTM!Also applies to: 69-75, 232-236, 451-463, 470-471, 549-554, 611-612, 1286-1387
registry/crates/pnpm-registry/src/server.rs (2)
1298-1305: LGTM!
1327-1342: LGTM!registry/crates/pnpm-registry/tests/policy.rs (2)
16-68: LGTM!
84-97: LGTM!
build_policies validated access/publish but let unknown unpublish tokens slip through. Validate it as well (defaulting to the publish rule when absent) so the fail-closed behavior is consistent.
|
Actionable comments posted: 0 |
This was referenced May 28, 2026
zkochan
added a commit
that referenced
this pull request
May 29, 2026
* feat(registry): support multi-token access lists with usernames Model each packages[*] access/publish permission as a list of tokens (verdaccio's space-separated groups) instead of a single rule, and match it against the caller's identity. Tokens are the built-in pseudo-groups ($all/$authenticated/$anonymous and their @/bare aliases) or a name; a name matches an authenticated caller whose username equals it — verdaccio's per-user access, where the htpasswd backend contributes the username and richer groups would come from a group-providing auth backend. - AccessRule (single enum) -> AccessList (Vec<AccessToken>) + Identity. - Accept both a space-separated string and a YAML sequence for each permission, like verdaccio. - Bare names are now valid tokens (drops the InvalidAccessRule stopgap from #12043 that rejected them). - enforce_access: anonymous denial -> 401, authenticated denial -> 403. Refs #11973. * docs(registry): note the bare authenticated/anonymous token aliases AccessToken::from also accepts the unprefixed `authenticated` / `anonymous` forms; document them on the variants so they aren't misread as usernames.
zkochan
added a commit
that referenced
this pull request
May 29, 2026
* feat(registry): support multi-token access lists with usernames Model each packages[*] access/publish permission as a list of tokens (verdaccio's space-separated groups) instead of a single rule, and match it against the caller's identity. Tokens are the built-in pseudo-groups ($all/$authenticated/$anonymous and their @/bare aliases) or a name; a name matches an authenticated caller whose username equals it — verdaccio's per-user access, where the htpasswd backend contributes the username and richer groups would come from a group-providing auth backend. - AccessRule (single enum) -> AccessList (Vec<AccessToken>) + Identity. - Accept both a space-separated string and a YAML sequence for each permission, like verdaccio. - Bare names are now valid tokens (drops the InvalidAccessRule stopgap from #12043 that rejected them). - enforce_access: anonymous denial -> 401, authenticated denial -> 403. Refs #11973. * docs(registry): note the bare authenticated/anonymous token aliases AccessToken::from also accepts the unprefixed `authenticated` / `anonymous` forms; document them on the variants so they aren't misread as usernames.
4 tasks
4 tasks
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
Wires
pnpm-registry's per-package access policy from the YAML config instead of hard-coding it, and adds the$anonymousaccess rule.Until now
from_yaml/from_default_yamlignored eachpackages[*].access/.publishtoken and always installedPackagePolicies::registry_mock_defaults(). So a config like:parsed fine but was silently unenforced —
@acme/*stayed world-readable. This closes that gap: configured ACLs now actually gate requests.What's in
from_yaml_strbuildsPackagePoliciesfrom thepackages:block in declared order (first match wins). Missingaccessdefaults to$all, missingpublishto$authenticated— the same safe fallback applied to unmatched packages.$anonymousrule. NewAccessRule::Anonymous: admits only unauthenticated callers. An authenticated caller is outside that group, so it gets 403 (matching verdaccio, where$anonymousis absent from a logged-in user's groups).$all/$authenticated/$anonymous(e.g. a named group, not modeled yet) now fails config parsing rather than silently mis-enforcing.Config::proxy/Config::static_servestill useregistry_mock_defaults, so the bundled/test behaviour is identical.Notes / scope
unpublishis parsed but still folds intopublishat enforcement time (separate item).config.yamlalready carries the@private/*and@pnpm.e2e/needs-authrules, sofrom_default_yamlreproduces the previous protections (covered by a regression test).Test plan
$anonymous, unknown-token error, and bundled-config protections preserved; plusAccessRuleparsing/rejection.tests/policy.rs) drives the realfrom_yaml→ router → enforce path: an$authenticatedscope 401s anonymous reads; an$anonymousscope admits anonymous (404) and 403s an authenticated caller.static_serve).cargo test,cargo clippy --deny warnings,cargo dylint(perfectionist),cargo fmt --check,cargo doc -D warningsall clean.Refs #11973 (Access control → "Wire policy fully from YAML" + "
$anonymousgroup").Written by an agent (Claude Code, claude-opus-4-8).
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests