Skip to content

Enforce registry RBAC on publish and registry access paths - #463

Open
MUFFANUJ wants to merge 11 commits into
nebari-dev:mainfrom
MUFFANUJ:rbacOnRegistryActions
Open

Enforce registry RBAC on publish and registry access paths#463
MUFFANUJ wants to merge 11 commits into
nebari-dev:mainfrom
MUFFANUJ:rbacOnRegistryActions

Conversation

@MUFFANUJ

Copy link
Copy Markdown
Collaborator

Reference Issues or PRs

Closes #444

What does this implement/fix?

Put a x in the boxes that apply

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds a feature)
  • Breaking change (fix or feature that would cause existing features not to work as expected)
  • Documentation Update
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no API changes)
  • Build related changes
  • Other (please describe):

Testing

  • Did you test the pull request locally?
  • Did you add new tests?

Documentation

API documentation was regenerated to include the new registry_id query parameter and the documented 400/403 responses for publish defaults

Access-centered content checklist

Text styling

  • The content is written with plain language (where relevant).
  • If there are headers, they use the proper header tags (with only one level-one header: H1 or # in markdown).
  • All links describe where they link to (for example, check the Nebari website).
  • This content adheres to the Nebari style guides.

Non-text content

  • All content is represented as text (for example, images need alt text, and videos need captions or descriptive transcripts).
  • If there are emojis, there are not more than three in a row.
  • Don't use flashing GIFs or videos.
  • If the content were to be read as plain text, it still makes sense, and no information is missing.

Any other comments?

This fixes missing registry RBAC enforcement across registry browse, tag lookup, import, publish, publication visibility updates, and publish defaults. Registry read/write checks are now performed in the service layer, denied access is audited, registry listings/publications are filtered by readable registries, and publish defaults can be computed for an explicitly selected registry. The frontend publish dialog now refreshes defaults when the user selects a different registry, and the API docs were regenerated for the new publish-defaults behavior.

@netlify

netlify Bot commented Jul 21, 2026

Copy link
Copy Markdown

Deploy Preview for nebi-docs canceled.

Name Link
🔨 Latest commit 6147baa
🔍 Latest deploy log https://app.netlify.com/projects/nebi-docs/deploys/6a838b51ed268900086016ec

@MUFFANUJ

Copy link
Copy Markdown
Collaborator Author

Currently, I kept "read" / "write" as string literals because that is the existing pattern used across the repo’s RBAC code. I think that constants would be cleaner and safer long-term. We can add shared constants for these actions and replace usages repo-wide, but I avoided doing that in this fix to keep the change minimal and focused on registry RBAC enforcement. If needed i can include that as well here.

@aktech

aktech commented Jul 23, 2026

Copy link
Copy Markdown
Member

Looks like there are some conflicts.

@MUFFANUJ

MUFFANUJ commented Jul 26, 2026

Copy link
Copy Markdown
Collaborator Author

Resolved the conflicts in 91e5e72. Please take a look now!

@aktech

aktech commented Jul 28, 2026

Copy link
Copy Markdown
Member

Looks like the CI is failing after the latest merge.

Comment thread internal/service/workspace_publishing.go

@aktech aktech left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@MUFFANUJ Taking a closer look, this breaks registry access for everyone in team mode. Previously any logged-in user could pull and push from the registries configured by admins. With this PR, nobody (including admins) has access after upgrade until grants are created, and the only way to create them is the admin grant API by hand: there's no UI for it.

Fine-grained access control was parked for later mainly because the management side/UI wasn't there yet. How are you envisioning people will grant permissions? We probably need either a backwards-compatible default (e.g. registries stay open to logged-in users unless marked restricted) or the grant UI as part of this change.

I would suggest we go with ability keeping registries open to logged in users unless marked restricted to unblock this PR and a follow up PR for the management UI, for which we need to coordinate with Smera and Nat.

Maybe something like:


Screenshot 2026-08-11 at 13 16 41

@MUFFANUJ

Copy link
Copy Markdown
Collaborator Author

I would suggest we go with ability keeping registries open to logged in users unless marked restricted to unblock this PR and a follow up PR for the management UI, for which we need to coordinate with Smera and Nat.

That makes sense, will address the UI part on a follw up; this is ready for another look!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[security] Registry read/write RBAC is defined but never enforced on browse/tags/import/publish

2 participants