* fix(replication): preserve artifact type for multi-tier accessories
In getAccessoryArts, reassigning art when fetching accessory details
shadowed the parent artifact and caused sub-accessories (such as signatures
of an SBOM) to inherit Type "UNKNOWN" from the accessory artifact rather
than the subject artifact's type (e.g. "IMAGE"). Consequently, when a
replication policy configured a resource filter (e.g. "image"), the
sub-accessories were dropped by the artifact type filter.
Fix this by using a separate variable subArt to avoid shadowing art,
and propagate art.Type to subArt so nested accessories inherit the
root subject artifact's type.
Signed-off-by: stonezdj <stonezdj@gmail.com>
* test(replication): cover nested accessory type inheritance
Signed-off-by: stonezdj <stonezdj@gmail.com>
---------
Signed-off-by: stonezdj <stonezdj@gmail.com>
Move the RBAC evaluator from github.com/casbin/casbin v1.9.1 to
github.com/casbin/casbin/v3 v3.11.0. v1 is unmaintained and pulls in the
archived Knetic/govaluate.
v3 returns errors from NewEnforcer, Enforce and policy loading; a
failure now denies the request and is logged. v3 pre-registers
keyMatch2 and never replaces it, so Harbor's matcher is registered as
harborKeyMatch2. v3 parses policy lines as CSV, so the adapter loads
rules as arrays to keep names with quotes or commas working.
Signed-off-by: Prasanth Baskar <prasanth@8gears.com>
The resource filter stopped filtering artifacts by type when the chart
resource type was removed in #18191, so policies with the image filter
replicated charts and other artifacts as well.
Fixes#24047
Signed-off-by: chlins <chlins.zhang@gmail.com>
Use the established “Tag Immutability” label for the robot-account permission selector in all supported locales.
Fixes#24170
Prepared and tested with Codex assistance; no human review is claimed.
Signed-off-by: NUAAwqy <146524024+NUAAwqy@users.noreply.github.com>
fix(webhook): default webhook egress policy level to allow_all
When the webhook egress policy is empty or unset, default to egressAllowAll
to preserve Harbor's default egress behavior and avoid regressions for internal
and test webhook targets.
Signed-off-by: stonezdj <stonezdj@gmail.com>
- Record and bubble up failure error messages (including tail of job logs) in Jobservice
- Store status_message on failed task and aggregate task errors into execution status_message
- Expose status_message in swagger ReplicationTask definition and handler
- Display status_message tooltip on replication tasks and execution status in UI portal
Signed-off-by: stonezdj <stonezdj@gmail.com>
feat(replication): separate event-triggered replication into dedicated queue
Separate replication tasks generated by event-based executions into
a dedicated EVENT_REPLICATION queue in JobService with higher priority
to prevent head-of-line blocking by bulk manual or scheduled replication.
Signed-off-by: stonezdj <stonezdj@gmail.com>
Co-authored-by: Wang Yan <wangyan_0219@hotmail.com>
* fix(jobservice): prevent CPU spin from unreachable cron expressions
Cron expressions like '0 0 3 30 2 *' (Feb 30) parse successfully because
robfig/cron validates each field individually without cross-checking day-of-month
against month. schedule.Next() then returns time.Time{} since no matching date
exists within the five-year search window. This causes three failures:
1. ValidateCronString accepts the expression. Fix: call schedule.Next after Parse
and reject zero results.
2. The scheduleNextJobs enqueuer loop spins indefinitely. Fix: add !t.IsZero()
guard to the loop condition.
3. clearDirtyJobs misses negative-epoch entries. Fix: use math.MinInt64 as the
lower bound instead of 0.
Fixes#23924
Signed-off-by: Nikita Kodkany <nikitakodkany@gmail.com>
* fix(jobservice): address review feedback on unreachable cron fix
Signed-off-by: Nikita Kodkany <nikitakodkany@gmail.com>
* fix(jobservice): address review feedback on unreachable cron fix
- Log and set lastEnqueueErr when policy is skipped as unreachable
- Add IsCronReachable check in jobservice Policy.Validate and controller
submit validation so pre-upgrade schedules are rejected at restore
- Fix probe comment: century year gap, not weekday intersection
- Collapse cronIsReachable/IsCronReachable into one exported function
- Use consistent error message across all validation sites
- Fix clearDirtyJobs comment to describe legacy cleanup
- Improve test robustness: FailNow on timeout, policy-specific assertion
- Add Feb 29 leap day test case to pin reachable behavior
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: Nikita Kodkany <nikitakodkany@gmail.com>
* fix(jobservice): stop the enqueuer loop when Next returns zero mid-loop
The zero-time check only covered the first schedule.Next call. A reachable
spec can still return time.Time{} later: for "0 0 0 29 2 *", Next after
2096-02-29 finds nothing because 2104-02-29 is past robfig/cron's five-year
search window, and time.Time{}.Before(horizon) keeps the loop spinning.
The loop now lives in fireTimes with the IsZero guard in its condition. The
warning for a zero first Next now says there is no occurrence within the
search window, because in 2097-2099 that also applies to valid Feb 29 specs.
Signed-off-by: Vadim Bauer <vb@container-registry.com>
* fix(jobservice): use dedicated enqueuer in test to fix data race
The test goroutine called scheduleNextJobs on the suite-level enqueuer
while its background loop was running, racing on lastEnqueueErr.
Signed-off-by: Nikita Kodkany <nikitakodkany@gmail.com>
* style(jobservice): gofmt enqueuer_test.go
Signed-off-by: Vadim Bauer <vb@container-registry.com>
---------
Signed-off-by: Nikita Kodkany <nikitakodkany@gmail.com>
Signed-off-by: Vadim Bauer <vb@container-registry.com>
Co-authored-by: Nikita Kodkany <nkodkany@crusoe.ai>
Co-authored-by: Nikita Kodkany <nikitakodkany@gmail.com>
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-authored-by: Vadim Bauer <vb@container-registry.com>
Clean up empty secrets when creating a new secret handler in registryctl
and skip empty secrets during validation to prevent authentication bypass
when JOBSERVICE_SECRET is empty/unset.
Signed-off-by: stonezdj <stonezdj@gmail.com>
Co-authored-by: Pushkar Joglekar <pushkar.joglekar@broadcom.com>
Redact the URL field in ListScannerCandidatesOfProject response to prevent
non-admin project members from listing system-global scanner URLs, while
retaining the scanner URL when the caller has system admin permissions.
Signed-off-by: stonezdj <stonezdj@gmail.com>
In basicController.Stop, the execution query previously used artifact.Digest
without scoping the lookup to the caller's artifact. Because artifact digests
are content-addressable and can be identical across different projects/tenants,
this allowed users to stop scans belonging to other projects, introducing a
cross-tenant scan-stop (availability) vulnerability.
This commit scopes the execution query specifically to the artifact's unique
database ID (`artifact.ID`), ensuring that only the scan of the caller's specific
artifact is stopped.
Signed-off-by: stonezdj <stonezdj@gmail.com>
Filters with several comma-separated values are stored wrapped in braces
(e.g. {a,b}). When opening such a policy for edit, the brace-stripping
branch returned from the forEach callback instead of assigning the value,
so the repository, tag and label fields stayed empty.
The fix was originally proposed in #23323, which was closed
as stale without review; this re-submits it with a regression test.
Fixes#23293
Signed-off-by: Viktor Erpylev <velmoga@gmail.com>
Co-authored-by: Psykii22 <189542486+Psykii22@users.noreply.github.com>
feat(portal): custom project roles UI
Frontend for the custom project roles feature (backend: #23804). Splits the
UI out of the combined feature branch and adapts it to the final backend API.
- Roles management: list/create/edit custom roles with a permissions panel
(left-side-nav/roles/*, shared/components/role-permissions-panel/*), wired
into routing (base.module, harbor-shell nav, shared.const) behind the
system-admin guard.
- Member add/edit (member, add-member, add-group): dynamic role selector that
lists built-in and custom roles from the role API, with a built-in badge.
- Robot account editor: fetch creator's effective project permissions from
/users/current/permissions (scope=/project/{id}, relative) instead of the
removed /projects/{name}/permissions/effective endpoint, to filter grantable
robot permissions; fail closed on empty permissions.
- Roles grid: compact fixed widths for ID and built-in columns, multi-row selection,
ascending ID sorting by default, and proper navigation label.
- Role pickers: shared getAllRoles() helper across member and group dialogs with
loading state and validation against unlisted roles.
- Role wizard: exact role name uniqueness check, input validation, and full-width modal.
- i18n: ROLE.* keys and MEMBER.BUILT_IN synced and translated across all supported locales.
- Tests: unit test coverage for role permissions panel, roles component, add-role wizard,
and role pagination helper.
Signed-off-by: Max Grau Stenzel <max.grau.stenzel@gmail.com>
Signed-off-by: Prasanth Baskar <prasanth@8gears.com>
Signed-off-by: stonezdj <stonezdj@gmail.com>
Co-authored-by: Prasanth Baskar <prasanth@8gears.com>
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Translate the strings of the French portal file that were still in
English, mostly from recent features (audit log event types, Dragonfly
preheat scope, OIDC logout, preferences).
Signed-off-by: Lewis Bertin <lewis@tactiqual.tech>
* fix(registry): validate upload Location against the request origin, not the configured endpoint
The GHSA-wjpm-hv5w-gxr6 fix compared registry-provided upload Location
headers against the configured registry endpoint (c.url). Adapters whose
authorizer rewrites the request host break under that rule: AWS ECR is
configured with https://api.ecr.<region>.amazonaws.com but every request
is rewritten to <account>.dkr.ecr.<region>.amazonaws.com, which is also
the host in the Location ECR returns. Every push replication to ECR
failed with 'resolves to a different origin'.
The correct invariant is 'the Location must be same-origin with the
server that issued it'. Validate the Location when it is received, using
the URL of the request that produced the response (after the authorizer
ran) as the origin. Root-relative Locations are still appended to the
endpoint so a path prefix is preserved. Cross-origin, network-path and
userinfo-bearing values are still rejected and the attacker host is
never contacted.
Fixes the ECR push replication regression introduced by #<GHSA-wjpm PR>.
Signed-off-by: wang yan <yan-yw.wang@broadcom.com>
* fix(registry): resolve upload Location against the final request URL after redirects
c.client.Do follows redirects, but req.URL still refers to the original
request. A same-origin 307 to another path followed by a path-relative
Location would resolve against the original path and send the next
upload to the wrong URL. Use resp.Request.URL as the origin when the
HTTP client followed redirects, but reject a redirect that changed the
origin so an arbitrary redirect target can never become a trusted,
credential-bearing upload destination.
Signed-off-by: wang yan <yan-yw.wang@broadcom.com>
* fix(registry): simplify upload Location comments
Signed-off-by: wang yan <yan-yw.wang@broadcom.com>
---------
Signed-off-by: wang yan <yan-yw.wang@broadcom.com>
Three dependabot bumps landed on release-2.15.0 but never reached main:
- github.com/jackc/pgx/v5 v5.10.0 -> v5.11.0 (#23902)
- github.com/aws/aws-sdk-go-v2/credentials v1.19.19 -> v1.20.3 (#23908)
- github.com/go-openapi/runtime v0.32.2 -> v0.33.2 (#23909)
The transitive sets follow from the module graph. main is behind
release-2.15.0 on the aws-sdk core, so the aws side pulls more here than
it did on the release branch. go-openapi/runtime 0.33 moves spec,
validate, analysis and jsonpointer to 1.0.0 and brings along strfmt,
loads, the swag modules, otel and testify.
Signed-off-by: Prasanth Baskar <prasanth@8gears.com>
fix(contenttrust): allow legacy signer pull by default to avoid regression
Default CONTENT_TRUST_LEGACY_SIGNER_PULL_ENABLED to true to prevent breaking
existing cosign and notation signing workflows on upgrade.
Signed-off-by: wang yan <yan-yw.wang@broadcom.com>
The webhook SSRF guard was a single boolean. With it off (the default
since #24078) webhooks can reach loopback and cloud metadata; with it on,
every internal receiver breaks.
HARBOR_WEBHOOK_EGRESS_POLICY (harbor.yml webhook_egress.policy) selects a
level: allow_all, block_restricted (default) or public_only. Restricted
covers loopback, link-local, cloud metadata, unspecified, multicast and
NAT64/6to4/Teredo. HARBOR_WEBHOOK_EGRESS_ALLOWLIST takes IPs, CIDRs,
hostnames and *.domain suffixes that are allowed at every level.
When the policy is unset, the deprecated HARBOR_ALLOW_PRIVATE_NETWORK_ACCESS
keeps its 2.15 meaning: true is allow_all, false is public_only. Invalid
values fail closed, prepare rejects an unknown policy, and a blocked
target is logged with the setting that reopens it.
Signed-off-by: Vadim Bauer <vb@container-registry.com>
* fix(webhook): allow private network access by default to avoid regression
Default HARBOR_ALLOW_PRIVATE_NETWORK_ACCESS to true to prevent breaking
existing on-premise and Kubernetes deployments where webhooks target
internal services.
Remove the allow_private_network_access field from harbor.yml to keep
the YAML schema clean, managing this toggle solely via the environment
variable.
Signed-off-by: wang yan <yan-yw.wang@broadcom.com>
* resolve copilot review
Signed-off-by: wang yan <yan-yw.wang@broadcom.com>
* resolve the comments
Signed-off-by: wang yan <yan-yw.wang@broadcom.com>
---------
Signed-off-by: wang yan <yan-yw.wang@broadcom.com>
* fix(webhook): block SSRF to private-network targets and stop reflecting response bodies
Webhook and Slack notification jobs delivered to any operator-supplied URL with
no address classification, and folded a non-2xx response body into the
project-readable task log. A project admin could reach loopback, RFC1918,
link-local and cloud-metadata (169.254.169.254) targets and read internal
service responses back through the webhook task log (CWE-918).
Classify destinations against the IANA special-purpose registries and reject
non-public targets at (1) webhook policy validation and (2) a dial-time guard on
the notification HTTP clients; drop the response body from the task-log error;
refuse redirects. The HARBOR_ALLOW_PRIVATE_NETWORK_ACCESS opt-out (default off)
restores delivery to internal endpoints for operators who rely on it.
Signed-off-by: Vadim Bauer <vb@container-registry.com>
* test(webhook): cover SSRF target rejection, body-drop and redirect refusal
Signed-off-by: Vadim Bauer <vb@container-registry.com>
* test(webhook): resolve upstream's mixed-case target host in the stub resolver
Upstream #23889 added a create-policy test that posts a mixed-case target
address and expects 201. With the SSRF target check in place that hostname
now goes through the stub resolver, which only knew hooks.example.com, so
the test failed with 400. Teach the stub the second hostname; the upstream
test keeps asserting lowercasing, the SSRF tests keep asserting rejection.
Signed-off-by: Vadim Bauer <vb@container-registry.com>
* fix(webhook): honour the jobservice proxy settings in the SSRF-safe transport
The public-network guard cleared Transport.Proxy, so webhook and Slack
delivery ignored the HTTP_PROXY/HTTPS_PROXY/NO_PROXY settings that
prepare writes for jobservice. Deployments without a direct Internet
route lost every public webhook.
Keep the transport's proxy function. When it selects a proxy, validate
the target host (literal or all resolved addresses) as public before the
request is handed to the proxy, and let only dials to that proxy address
skip the dial-time private-address check. When no proxy applies
(NO_PROXY, loopback), the resolved-address dial guard is unchanged. A
direct request aimed at the proxy's own address is refused so it cannot
inherit the proxy exemption.
Signed-off-by: Vadim Bauer <vb@container-registry.com>
* chore: describe the fix without advisory identifiers
Signed-off-by: Vadim Bauer <vb@container-registry.com>
(cherry picked from commit 54cc3fca7a741ecb31293a767b998ceffec9d5b5)
* fix(webhook): drain notification response bodies before closing
Closing an unread response body forces the transport to drop the
keep-alive connection. Discard up to 64KiB of the body (never surfacing
it) so the pooled connection is reused after non-2xx responses.
Signed-off-by: Vadim Bauer <vb@container-registry.com>
* test(webhook): assert the dial guard on production clients and pin the env
Exercise the production notification clients against a loopback target
so dropping the public-network guard fails a test, cover connection
reuse after non-2xx responses, and pin HARBOR_ALLOW_PRIVATE_NETWORK_ACCESS
to false in tests that assert the default blocking behaviour.
Signed-off-by: Vadim Bauer <vb@container-registry.com>
* fix(prepare): keep network.allow_private_network_access across migrate
The migrate command re-renders harbor.yml from the 2.15.0 template, so a
key missing there is dropped. Render the network block, preserving an
operator-set value and defaulting to false.
Signed-off-by: Vadim Bauer <vb@container-registry.com>
---------
Signed-off-by: Vadim Bauer <vb@container-registry.com>
* fix(policy): stop User-Agent from bypassing content-trust and vulnerability policies
SkipPolicyChecking exempted a caller from the content-trust and
vulnerability-prevention pull policies when it held push and the request
User-Agent contained "cosign" or "notation". User-Agent is fully
attacker-controlled (CWE-807), so any push-capable principal (e.g. the
Developer role) bypassed both policies on a plain manifest pull by setting
one header.
Remove the User-Agent discriminator. Scanner pull keeps its bypass via the
non-forgeable scanner-pull RBAC action, and a genuinely signed artifact is
still recognised by its signature accessory. The pull-before-first-signature
bootstrap for content-trust is preserved as an opt-in, content-trust-only
exemption (CONTENT_TRUST_LEGACY_SIGNER_PULL_ENABLED=true); it never affects
the vulnerability-prevention policy.
Reported by sabuhimammadov0 (GHSA-wrw5-gmvj-gf23); independently reported on
the cncf-harbor-security list by kt (thread #61), Oleh Konko (thread #25) and
Eren Akdag (thread #68).
Signed-off-by: Vadim Bauer <vb@container-registry.com>
* test(policy): spoofed User-Agent no longer bypasses pull policies
Cover GHSA-wrw5-gmvj-gf23: a push-capable v2token caller with
User-Agent: cosign is now blocked by both the content-trust and the
vulnerability-prevention middleware, while scanner-pull and genuine signature
accessories still bypass. Add a test for the opt-in content-trust bootstrap.
Signed-off-by: Vadim Bauer <vb@container-registry.com>
* test(contenttrust): pin the legacy signer-pull hatch off in the spoofed User-Agent test
The test asserted the hatch-off behaviour but inherited the process
environment, so it failed whenever the flag was exported.
Signed-off-by: Vadim Bauer <vb@container-registry.com>
* test(vulnerable): enable the legacy signer-pull hatch in the spoofed User-Agent test
With the flag off the test could not detect the User-Agent bypass
being moved back into the shared policy check behind the flag.
Signed-off-by: Vadim Bauer <vb@container-registry.com>
* chore: describe the fix without advisory identifiers
Signed-off-by: Vadim Bauer <vb@container-registry.com>
(cherry picked from commit 49f78850621446dd113a3d2517fed7e23f3e565b)
* test(contenttrust): cover the legacy signer-pull hatch guards
Add negative controls for the opt-in signer-pull exemption: it stays off
when the variable is unset, and with it enabled a pull is still rejected
without push permission or without a signing-client User-Agent. Also
cover the notation User-Agent path.
Signed-off-by: Vadim Bauer <vb@container-registry.com>
---------
Signed-off-by: Vadim Bauer <vb@container-registry.com>
* test(pkg/registry): regression for chunk-upload Location SSRF (GHSA-wjpm-hv5w-gxr6)
Signed-off-by: Vadim Bauer <vb@container-registry.com>
* fix(registry): validate chunk-upload Location against the registry origin (GHSA-wjpm-hv5w-gxr6)
Signed-off-by: Vadim Bauer <vb@container-registry.com>
* fix(registry): keep the endpoint path prefix for root-relative upload Locations
Distribution with relativeurls returns /v2/... without the path prefix the endpoint is served under. Resolving it against the endpoint dropped that prefix, so pushes through a path-prefixed registry endpoint hit the wrong route. Append root-relative paths to the endpoint as before and keep the origin and userinfo checks.
Signed-off-by: Vadim Bauer <vb@container-registry.com>
* chore: describe the fix without advisory identifiers
Signed-off-by: Vadim Bauer <vb@container-registry.com>
(cherry picked from commit 6b4be4734a401c5b427667544534aea8bed53c1d)
---------
Signed-off-by: Vadim Bauer <vb@container-registry.com>
* test(common/http): regression for pagination Link SSRF (GHSA-xgp2-5vxg-rgh2)
Signed-off-by: Vadim Bauer <vb@container-registry.com>
* fix(common/http): pin pagination Link next URL to the request origin (GHSA-xgp2-5vxg-rgh2)
GetAndIteratePagination built the next-page URL by string-concatenating the
attacker-controlled Link rel="next" value onto scheme+host and re-parsing it, so
a value like "@evil.example/x" re-pointed the request at evil.example (host) with
the original host demoted to userinfo. Because Client.Do re-runs its auth modifiers
on every hop, the configured registry credentials were then sent to that host
(SSRF + credential disclosure).
Resolve the Link as an RFC 3986 reference against the current page URL, reject
userinfo, and require the resolved scheme/host/port to equal the origin. Relative
same-origin pagination keeps working; any cross-origin next target fails closed.
Signed-off-by: Vadim Bauer <vb@container-registry.com>
* test(common/http): require cross-origin pagination links to fail with an error
The regression test discarded the pagination error, so an implementation that silently stopped at a cross-origin Link and returned a truncated list would still pass. Assert that every vector returns an error.
Signed-off-by: Vadim Bauer <vb@container-registry.com>
(cherry picked from commit bfa9a1e750cceb9363a510d548f14694fdf6ae31)
* chore: describe the fix without advisory identifiers
Signed-off-by: Vadim Bauer <vb@container-registry.com>
(cherry picked from commit 00e1558ff2fd848ebd071d21a9a9b3e1895df761)
* test(common/http): cover userinfo rejection and absolute same-origin pagination links
The existing userinfo vector is rejected by url.Parse before the
userinfo check runs, so add resolveNextLink cases that only that check
can reject. Also exercise absolute same-origin next links, default port
equivalence and scheme/port mismatches.
Signed-off-by: Vadim Bauer <vb@container-registry.com>
* fix(registry): keep catalog and tag list pagination on the registry origin
Catalog and ListTags followed the Link rel="next" value from the remote
registry verbatim when absolute, and appended it to the endpoint otherwise,
so an absolute URL to another host or a value starting with "@" sent the
next request elsewhere. Authorizers that attach credentials regardless of
host (basic auth used by the JFrog adapter, the ACR token authorizer) then
disclosed them to that host.
Build the next URL exactly as before and reject it unless it keeps the
endpoint's scheme, host, effective port and userinfo. SameOrigin is
exported from common/http to share the origin check.
Signed-off-by: Vadim Bauer <vb@container-registry.com>
---------
Signed-off-by: Vadim Bauer <vb@container-registry.com>
* fix(security): fail closed when auth mode lookup fails (GHSA-3x7c-wf86-mjx6)
The security context middleware called config.AuthMode and, on error, logged a
warning and continued processing the request with no auth mode in the request
context. Determining the request's security context is an authorization decision
that depends on the configured auth mode, so a lookup failure must reject the
request rather than fall through to the handler (CWE-636).
Reject with 500 via lib_http.SendError before the security-context generators
run. Normal requests, where the lookup succeeds, are unaffected.
Signed-off-by: Vadim Bauer <vb@container-registry.com>
* test(security): assert middleware fails closed on auth mode lookup error (GHSA-3x7c-wf86-mjx6)
Signed-off-by: Vadim Bauer <vb@container-registry.com>
* chore: describe the fix without advisory identifiers
Signed-off-by: Vadim Bauer <vb@container-registry.com>
(cherry picked from commit 33304265ce4ad826c63e471fd65bcbf2aa589c86)
* test(security): restore default config manager after TestSecurity
Signed-off-by: Vadim Bauer <vb@container-registry.com>
---------
Signed-off-by: Vadim Bauer <vb@container-registry.com>
* fix(authproxy): stop a local sysadmin flag leaking into an authproxy context (GHSA-xvh7-74g6-29xw)
In auth_mode=http_auth the /v2 path loads the Harbor-local user record by name
and overlays only GroupIDs and AdminRoleInAuth from the TokenReview result
(server/middleware/security/auth_proxy.go). The local SysAdminFlag survives, and
local.SecurityContext.IsSysAdmin returns SysAdminFlag || AdminRoleInAuth, so an
external identity that the authproxy admin policy did not grant admin obtains
sysadmin /v2 authorization purely by colliding with a local sysadmin name -- the
seeded admin account always exists.
Clear SysAdminFlag on the overlaid record so AdminRoleInAuth, derived from the
TokenReview username and groups against the configured admin set, is the only
admin signal in authproxy mode. An explicitly allowlisted authproxy admin is
unaffected: its access comes from AdminRoleInAuth, not a local flag.
Reported by cyberlanc3r (GHSA-xvh7-74g6-29xw); independently reported by
Mitchell Benjamin (thread #49, topic 119514428).
Signed-off-by: Vadim Bauer <vb@container-registry.com>
* test(authproxy): regression for sysadmin-flag collision (GHSA-xvh7-74g6-29xw)
Drive the real authProxy.Generate + local.SecurityContext.IsSysAdmin with a
mocked user manager returning a colliding sysadmin-flagged local record. Assert
the external identity is not sysadmin with an empty admin allowlist, and that an
explicitly allowlisted authproxy admin still is.
Signed-off-by: Vadim Bauer <vb@container-registry.com>
* chore: describe the fix without advisory identifiers
Signed-off-by: Vadim Bauer <vb@container-registry.com>
(cherry picked from commit df6273ae8d0dd15a0a1f2f84d5a9b3f5b018bdee)
* test(authproxy): return a fresh user per lookup in the collision test
The mocked GetByName returned one shared *User, and Generate mutates it,
so the allowlisted arm saw a record whose sysadmin flag the first arm had
already cleared and never exercised the collision. Return a new record
per call, as the DB-backed manager does, and assert the allowlisted arm
derives admin from AdminRoleInAuth with the local flag cleared.
Signed-off-by: Vadim Bauer <vb@container-registry.com>
---------
Signed-off-by: Vadim Bauer <vb@container-registry.com>
* fix(immutable): evaluate all repository and tag selectors (GHSA-27f2-qg57-2gwg)
The immutable tag rule matcher only read ScopeSelectors["repository"][0] and
TagSelectors[0], so a rule carrying more than one repository or tag selector
protected only the first entry. Pushes and deletes to the repositories or tags
named by the remaining selectors bypassed the immutability control.
Evaluate every selector in each dimension and match when any of them matches,
mirroring the OR/alternation the portal expresses as a single {a,b} doublestar
pattern. Single-selector rules, which is all the portal ever creates, keep
identical behaviour.
Signed-off-by: Vadim Bauer <vb@container-registry.com>
* test(immutable): cover multi-selector immutable rule matching (GHSA-27f2-qg57-2gwg)
Signed-off-by: Vadim Bauer <vb@container-registry.com>
* fix(immutable): subtract exclusion selectors instead of OR-ing them
Several selectors in one rule dimension must behave like the portal's
single {a,b} pattern. OR-ing every selector made two repoExcludes or
excludes selectors select everything, so excluded repositories and
tags became immutable. Inclusion selectors stay alternatives, every
exclusion selector now has to select the candidate, and multi-tag
candidates are evaluated per tag so tags excluded by different
selectors do not cancel out.
Signed-off-by: Vadim Bauer <vb@container-registry.com>
* chore: describe the fix without advisory identifiers
Signed-off-by: Vadim Bauer <vb@container-registry.com>
(cherry picked from commit 27f4614d1ec7484eda1336bb2e01cf1b891326df)
* fix(immutable): ignore null selectors in rule matching
The rule API stores selector lists without validation, so a null entry
after the first selector was harmless while only the first one was read.
Evaluating every selector turned such entries into a nil dereference.
Drop null entries before matching.
Tests now purge leftover rules of their project before running, since
Match evaluates every rule of the project.
Signed-off-by: Vadim Bauer <vb@container-registry.com>
---------
Signed-off-by: Vadim Bauer <vb@container-registry.com>
* fix(scan): verify TLS and bound body on scanner bearer token request (GHSA-884q-mjcg-86x4)
Signed-off-by: Vadim Bauer <vb@container-registry.com>
* test(scan): assert bearer token request verifies TLS and bounds body (GHSA-884q-mjcg-86x4)
Signed-off-by: Vadim Bauer <vb@container-registry.com>
* chore: describe the fix without advisory identifiers
Signed-off-by: Vadim Bauer <vb@container-registry.com>
(cherry picked from commit c9d2c8dc3997ef80fb1ea8a09936d01a13bab8e9)
* test(scan): assert certificate verification error type in bearer token TLS test
Match *tls.CertificateVerificationError via errors.As instead of a
substring of the error message, so the test does not depend on x509
error wording and cannot pass on an unrelated failure.
Signed-off-by: Vadim Bauer <vb@container-registry.com>
---------
Signed-off-by: Vadim Bauer <vb@container-registry.com>
* fix(scanner): stop access_cred ORM filter oracle (GHSA-vwvv-675v-p6rr)
The scanner_registration.access_cred column stores the literal Authorization
value Harbor presents to a scanner adapter. It lacked the filter:"false" ORM
tag, so the generic q= builder let a caller put it in a WHERE clause. Since the
API never returns the column, the row/X-Total-Count difference is a blind
boolean oracle that recovers the secret character by character (sibling of
CVE-2025-30086, matching the guarded reg.access_secret). Add filter:"false".
Signed-off-by: Vadim Bauer <vb@container-registry.com>
* test(scanner): regression for access_cred filter oracle (GHSA-vwvv-675v-p6rr)
Drives the real ListRegistrations/GetTotalOfRegistrations DAO path (the call
ListScannerCandidatesOfProject makes) with a non-matching access_cred filter and
asserts list/count are unchanged, i.e. the filter is dropped. Red without the
filter:"false" tag (oracle excludes the row), green with it. Positive control
keeps name filtering working.
Signed-off-by: Vadim Bauer <vb@container-registry.com>
* chore: describe the fix without advisory identifiers
Signed-off-by: Vadim Bauer <vb@container-registry.com>
(cherry picked from commit 9c160302ee3c56394957b263061adf374db4502f)
* docs(scanner): describe access_cred storage accurately
New writes are encrypted by the scanner manager; rows saved before that
remain plaintext because there is no data migration.
Signed-off-by: Vadim Bauer <vb@container-registry.com>
---------
Signed-off-by: Vadim Bauer <vb@container-registry.com>
* fix(rbac): collapse cover-all robot project scope to a single-segment matcher (GHSA-xq2m-cj8w-56v5)
Signed-off-by: Vadim Bauer <vb@container-registry.com>
* test(rbac): regression for cover-all robot sub-resource escalation (GHSA-xq2m-cj8w-56v5)
Signed-off-by: Vadim Bauer <vb@container-registry.com>
* chore(rbac): make the cover-all robot scope comment generic
Review follow-up: the comment and the test docs now describe the wildcard
expansion and the single-segment matcher in general terms instead of naming
the advisory. Wording only, no behaviour change.
Signed-off-by: Vadim Bauer <vb@container-registry.com>
---------
Signed-off-by: Vadim Bauer <vb@container-registry.com>
## Summary
This PR contributes the focused fix for Mitchell Benjamin's private thread #45 report. Granular
user-update authority no longer permits a caller to mint a Harbor sysadmin.
## Patch shape
An explicit `IsSysAdmin` guard follows the existing system permission check and precedes the sole
production `SetSysAdmin` call. Other user automation permissions are unchanged.
## Verification
The routed robot-denial and true-sysadmin success arms were checked and formatted against the pin.
Generated APIs were unavailable, so compile, focused handler test, and a served `403`/`200` check
remain reviewer tasks.
PTP-Ref: PTP-HARBOR-B-45
Class: B
Upstream-Report: thread #45, topic 119493101, cncf-harbor-security list
Original-Reporter: Mitchell Benjamin
Issue: TBD
Patch: patches/BACKLOG-45.patch
Base: 934ba4c60d
Audit-Pin: 1cd644ca39
Evidence: poc-out/45/ (receipt), patches/anish/BACKLOG-45.md (rationale)
Ported-From: https://github.com/container-registry/harbor-vuln-fixes/pull/8
Signed-off-by: Vadim Bauer <vb@container-registry.com>
Co-authored-by: Clinton Thomas <1033162+KernelClint@users.noreply.github.com>
* fix(registry): reject dot segments in /v2/ distribution paths (GHSA-qj39-pjf2-wmrc)
The /v2/ authorization middleware matches the raw request path to decide
which repository is accessed, while the beego router and the backend
registry resolve the handler after path.Clean. A path such as
/v2/public/x/manifests/x/../../../../private/secret/manifests/1.0.0 is
authorized as public/x but dispatched to private/secret, so anonymous or
low-privileged callers can read, overwrite or delete artifacts in projects
they cannot access.
Reject any /v2/ request whose URL path contains a '.' or '..' segment
before the auth middleware runs. No legitimate OCI path contains a dot
segment, so this fails closed with no impact on valid clients.
Signed-off-by: Vadim Bauer <vb@container-registry.com>
* test(registry): regression for /v2/ dot-segment path traversal (GHSA-qj39-pjf2-wmrc)
Signed-off-by: Vadim Bauer <vb@container-registry.com>
---------
Signed-off-by: Vadim Bauer <vb@container-registry.com>
* chore(deps-dev): bump stylelint from 14.16.1 to 17.15.0 in /src/portal
Bumps [stylelint](https://github.com/stylelint/stylelint) from 14.16.1 to 17.15.0.
- [Release notes](https://github.com/stylelint/stylelint/releases)
- [Changelog](https://github.com/stylelint/stylelint/blob/main/CHANGELOG.md)
- [Commits](https://github.com/stylelint/stylelint/compare/14.16.1...17.15.0)
---
updated-dependencies:
- dependency-name: stylelint
dependency-version: 17.15.0
dependency-type: direct:development
update-type: version-update:semver-major
...
Signed-off-by: dependabot[bot] <support@github.com>
* chore(portal): drop the deprecated stylelint prettier configs
stylelint-config-prettier caps its stylelint peer at < 15, so npm ci
fails with ERESOLVE once stylelint is on 17. Both prettier configs only
existed to switch off stylelint's formatting rules, which stylelint
removed in 16, and both packages are deprecated. Remove them from
package.json and .stylelintrc.json.
Bump stylelint-config-standard to 40.0.0 and
stylelint-config-standard-scss to 17.0.0, the first versions that accept
stylelint 17.
The newer configs flag 30 lines. 26 are fixed with stylelint --fix and
render the same (longhands to inset/place-* shorthands, word-wrap to
overflow-wrap, quoted attribute selectors, spacing). The remaining four
use deprecated CSS that browsers still support (clip: rect() in the
gauge, word-break: break-word in the alert banner). Replacing them could
change how they render, so they get inline disable comments instead.
Signed-off-by: Prasanth Baskar <prasanth@8gears.com>
---------
Signed-off-by: dependabot[bot] <support@github.com>
Signed-off-by: Prasanth Baskar <prasanth@8gears.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Prasanth Baskar <prasanth@8gears.com>
Scheduled runs now take the URL from core's current _REDIS_URL_REG and
only fall back to the stored value when the variable is empty. Existing
schedules recover without being saved again.
Fixes#22905
Signed-off-by: Vadim Bauer <vb@container-registry.com>
* feat: custom project roles (hybrid — built-ins in code, custom in DB)
Adds admin-defined custom project roles as an alternative to #22815, per
maintainer feedback: built-in project roles keep resolving their permissions
from the compile-time rolePoliciesMap (no DB, no cache — identical to today),
and ONLY custom roles are stored in and loaded from the database. There is no
permission cache.
Backend + API only (UI ships separately). Highlights:
- RBAC (common/rbac/project): projectRBACRole gains an optional custom *role.Role.
The built-in path is byte-identical to upstream; the role controller is
consulted only for non-built-in role IDs, so built-in authorization incurs no
DB lookup. Custom roles expand their DB-loaded permissions.
- Role subsystem: pkg/role (model/dao/manager) + controller/role for CRUD and
persistence; existing role_permission/permission_policy reused for custom perms.
- REST API: /roles endpoints (sysadmin-gated), an effective-permissions endpoint,
anti-escalation checks on member/robot assignment, and audit events.
- Migration 0191 adds is_builtin/description/audit columns to `role`, widens the
name, enforces name uniqueness, and marks seeded roles built-in. Built-in
permissions are NOT seeded into the DB.
Rebased onto current main; API bindings regenerated via gen_apis.
Tracking: #18124
Alternative to: #22815
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Max Grau Stenzel <max.grau.stenzel@gmail.com>
* review: make role a system RBAC resource; drop checkSysAdmin & ResourceSelf
Addresses stonezdj's review on PR #23804:
- Define rbac.ResourceRole (system-level) and add it to the system permission
catalog; gate CreateRole/UpdateRole/DeleteRole with RequireSystemAccess(action,
ResourceRole) instead of the hand-rolled checkSysAdmin — so the permission is
managable while still effectively system-only.
- Remove the checkSysAdmin helper and its unit tests; update handler tests to
mock Can().
- Drop ResourceSelf from the ScopeRole permission catalog.
- Remove tests/integration/.gitignore (per-dir ignore file).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Max Grau Stenzel <max.grau.stenzel@gmail.com>
* review: close built-in-role escalation gap; reject nil role payloads
Addresses Copilot findings on PR #23804:
- checkNoEscalation treated built-in roles as permissionless: it read permissions
from role_permission, which has no rows for built-in roles, so a caller with
member:create could assign projectAdmin (or any built-in) without holding its
permissions. Built-in permissions are now synthesized from the compile-time
policy map via the new rbac/project.BuiltinRolePolicies; custom roles still read
the DB. Adds TestCheckNoEscalation_BuiltinRoleSynthesized.
- role validate() now rejects nil permission/access elements (permissions:[null],
access:[null]) with 400 instead of panicking. Adds nil-payload tests.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Max Grau Stenzel <max.grau.stenzel@gmail.com>
* review: validate that a member's custom role exists in the database
Addresses stonezdj's review on PR #23804: isValidRole returned true for any
integer above the built-in range, so a member could be assigned a non-existent
custom role ID. The member controller now holds a role manager and validateRole
verifies custom role IDs (>5) exist via roleMgr.Get, rejecting unknown IDs with
ErrInvalidRole; built-in roles (1-5) still pass without a DB lookup.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Max Grau Stenzel <max.grau.stenzel@gmail.com>
* review: make role Create/Update/Delete atomic via orm.WithTransaction
Addresses Copilot findings on PR #23804:
- Create: the role row and its permission records are now created in one
transaction, so a failed permission insert no longer leaves a named role with
missing permissions (whose unique name would block a clean retry).
- Update: the field update and the delete+recreate of permissions run in one
transaction, so the role's authorization is never observable in a partial state.
- Delete: the member-assignment guard and the role/permission deletes run in one
transaction, serializing the guard with the delete and ensuring permission rows
never outlive the role. (Full prevention of a concurrent assignment needs a
DB-level FK on project_member.role — tracked as a follow-up.)
Controller unit tests inject a fake ormer (testing/lib/orm) so the transaction
wrappers resolve an ormer from context.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Max Grau Stenzel <max.grau.stenzel@gmail.com>
* review: group /roles/{role_id} right after /roles in swagger
Addresses stonezdj's review on PR #23804: the two role endpoints serve the same
feature and should be adjacent in the spec.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Max Grau Stenzel <max.grau.stenzel@gmail.com>
* review: split out role audit-log events; drop the standalone integration test
Addresses stonezdj's review on PR #23804 (shrink the PR):
- Remove the role audit-log event wiring from this PR (controller/event
metadata/role.go, topic.go role topics/events, init.go subscriptions, and the
notification.AddEvent calls in the role controller). It will be submitted as a
separate follow-up PR; the code is preserved on branch 18124-role-audit-events.
- Remove tests/integration/test_custom_roles.py: it is not wired into Harbor CI
(which uses tests/apitests) and contained the issues Copilot flagged. Feature
behavior is covered by the Go unit tests.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Max Grau Stenzel <max.grau.stenzel@gmail.com>
* review: validate role on member UpdateRole; fix stale invalid-role message
Addresses stonezdj's follow-up review on PR #23804:
- UpdateRole (changing a member's role) now runs the same validateRole check as
AddProjectMember, so a member can't be moved to a non-existent role.
- ErrInvalidRole no longer claims "role is not in 1,2,3"; it now states the role
must be a built-in role (1-5) or an existing custom role.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Max Grau Stenzel <max.grau.stenzel@gmail.com>
* review: remove duplicate effective-permissions endpoint
Addresses stonezdj's review on PR #23804: GET /projects/{id}/permissions/effective
duplicated the existing GET /users/current/permissions, which already supports a
project scope and a `relative` flag (scope=/project/{id}&relative=true returns the
same set of project permissions the caller holds). Removed the handler and the
swagger path (bindings regenerated). The shared mapRobotToHumanResource helper is
kept — it is still used by the robot anti-escalation check.
The robot-account UI (separate PR) will use /users/current/permissions instead.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Max Grau Stenzel <max.grau.stenzel@gmail.com>
* fix(rbac): keep role management sysadmin-only, not delegable
Role write access is checked via RequireSystemAccess(ctx, action,
rbac.ResourceRole), but ResourceRole was also added to the grantable
ScopeSystem catalog (NolimitProvider). That published role:* as an
assignable system permission: it could be granted to a system robot
(robots evaluate system-scope policies) and listed in /permissions,
even though a human non-admin never evaluates system-scope perms.
A system robot granted role:update could then rewrite any custom
role's permission set (UpdateRole/updateV2Role has no subset guard)
= privilege escalation.
Remove the ResourceRole entries from the ScopeSystem catalog. The
admin evaluator bypasses the catalog, so sysadmins still pass the
RequireSystemAccess check and manage roles exactly as before, but
role:* is no longer assignable/enumerable to non-admins. This keeps
role management strictly sysadmin-only while still expressing it as
a first-class action+resource check.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Max Grau Stenzel <max.grau.stenzel@gmail.com>
* fix(rbac): grant custom-role members baseline self:read (project visibility)
self:read (ResourceSelf) is the permission that gates viewing a project:
RequireProjectAccess(ctx, id, ActionRead) with no subresource checks
<project-ns>:"" and GetProject/GetSummary use exactly that. Built-in
roles carry self:read in rolePoliciesMap, and public projects grant it
via publicProjectPolicies, but a custom role in a private project had no
way to obtain it -- self:* is intentionally excluded from the ScopeRole
catalog, and GetPolicies returned only the role's explicit permissions.
Result: a member whose only access to a private project was a custom
role got a 403 opening that project.
Append {ResourceSelf, ActionRead} to the custom-role branch of
projectRBACRole.GetPolicies (de-duplicated), mirroring the built-in
roles and the public-project baseline. Only self:read is granted, never
self:update/self:delete (project edit/delete stay exclusive to the
projectAdmin built-in), and self:* remains out of the user-facing
catalog so it is guaranteed by the system rather than a tickable option.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Max Grau Stenzel <max.grau.stenzel@gmail.com>
* refactor(rbac): grant self:read at membership level, not per custom role
Move the baseline project-visibility grant out of the custom-role branch
of projectRBACRole.GetPolicies and into rbacUser.GetPolicies: any user
who holds at least one role in a project (len(projectRoles) > 0) is
granted self:read on that project.
This makes baseline visibility a property of project membership rather
than a per-custom-role special case, sitting beside the existing
public-project baseline (getPoliciesForPublicProject) and using the same
eval-time idiom -- no permission data seeded into the database. Built-in
and custom roles now obtain self:read the same way, and the grant stays
scoped to the membership project (ListRoles is per-project), so it never
exposes projects the user is not a member of.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Max Grau Stenzel <max.grau.stenzel@gmail.com>
* chore(deps-dev): bump webpack-cli from 4.10.0 to 7.2.3 in /src/portal/app-swagger-ui (#23850)
chore(deps-dev): bump webpack-cli in /src/portal/app-swagger-ui
Bumps [webpack-cli](https://github.com/webpack/webpack-cli) from 4.10.0 to 7.2.3.
- [Release notes](https://github.com/webpack/webpack-cli/releases)
- [Changelog](https://github.com/webpack/webpack-cli/blob/main/CHANGELOG.md)
- [Commits](https://github.com/webpack/webpack-cli/compare/webpack-cli@4.10.0...webpack-cli@7.2.3)
---
updated-dependencies:
- dependency-name: webpack-cli
dependency-version: 7.2.3
dependency-type: direct:development
update-type: version-update:semver-major
...
Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Prasanth Baskar <prasanth@8gears.com>
Signed-off-by: Max Grau Stenzel <max.grau.stenzel@gmail.com>
* review: consolidate 2.16.0 migration into 0190 and simplify robot resource map
Address two review comments from stonezdj on PR #23804:
- 2.16.0 is not released yet, so keep all its schema changes in a single
migration file. Move the custom-role `role` table additions (columns,
name widened to varchar(255), uq_role_name unique index, is_builtin
update) from 0191 into 0190 and delete 0191.
- Simplify mapRobotToHumanResource: replace the single-entry
robotToHumanResourceMap + lookup with a direct
`if r == rbac.ResourceProject { return rbac.ResourceSelf }`.
Also commit the cross-project isolation test for the membership-level
self:read baseline (a member of one private project cannot read another).
No behavior change; robot resource-mapping tests pass unchanged.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Max Grau Stenzel <max.grau.stenzel@gmail.com>
* review: fix bugs from bupd's review (keep roles global + sysadmin-only)
Address the correctness findings from bupd's review of PR #23804, keeping
the global, sysadmin-managed role model (project-scoped roles remain a
separate future feature):
- evaluator: a failed custom-role load no longer drops the whole rbacUser.
Returning nil stripped every role the member held in the project (built-ins
included), turning a transient DB error or a dangling project_member.role
into a total 403 / permanent lockout. Skip only the failing role instead.
- role validate(): reject any permission Effect other than "" or "allow".
It was stored verbatim and handed to casbin, so a mis-cased "Allow" or a
bogus value was accepted and silently granted nothing.
- read path: GET /roles(/{id}) now returns a built-in role's real permissions
(resolved from rolePoliciesMap via BuiltinRolePolicies) instead of an empty
set; built-ins have no role_permission rows by design. Removed the dead
"Visible" list filter (model.Role has no such field).
- controller Delete: a custom role with no permission rows is deletable again
(DeletePermissionsByRole's NotFound no longer rolls back the txn); dropped
the unused variadic option parameter.
- controller Update: persist a role name change instead of silently dropping
it (validated in the handler; DB unique index rejects collisions).
- permissions.go: restore the sysadmin-or-project-admin gate on GET
/permissions (it had been relaxed to any authenticated user), and expose the
role-level catalog only to sysadmins.
- rolePoliciesMap["projectAdmin"]: add metadata:list, restoring 2.15 behavior
where a project admin can create a robot with metadata:list (the new robot
anti-escalation check validated against projectAdmin, which lacked it).
- swagger: document that role reads are open to any authenticated user while
writes are sysadmin-only.
- add a drift-guard test asserting the ScopeRole catalog equals projectAdmin
minus self:{read,update,delete}, plus tests for the fixes above.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Max Grau Stenzel <max.grau.stenzel@gmail.com>
* review: make role name uniqueness case-insensitive
Per bupd's review: enforce uniqueness on lower(name) so "Maintainer" and
"maintainer" cannot coexist and be confused for one another. 2.16.0 is
unreleased, so the index is dropped and recreated in place.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Max Grau Stenzel <max.grau.stenzel@gmail.com>
* fix(role): correct stale TestList assertion (roles have no "visible" concept)
TestDaoTestSuite/TestList asserted that a freshly created role is NOT
returned by List (len == 0), a leftover from a removed "visible" column.
model.Role has no such field and the List DAO applies no visibility
filter, so the role is returned. Assert it is listed (len == 1) to match
the actual, intended behavior. Surfaced now that UTTEST runs on the PR.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Max Grau Stenzel <max.grau.stenzel@gmail.com>
* review: robot-refresh escalation, deny-effect guard, delete-race FK, swagger 409/412
Address chlins' and Copilot's review of PR #23804:
- RefreshSec escalation (chlins #1): refreshing a robot's secret is
equivalent to being granted the robot's permissions, so it now runs the
same no-escalation check as create/update (load the robot with
permissions; human callers checked via validateNoEscalation, robot
callers via isValidPermissionScope). A member with robot:update but not
the robot's own permissions can no longer take over a more-privileged
robot.
- Deny-effect guard (chlins #2): member checkNoEscalation now skips
Effect==deny accesses; they restrict rather than grant, so requiring the
caller to hold them wrongly blocked assigning a role that merely denies
something.
- Delete-vs-assign race (chlins #3): add FK project_member.role ->
role(role_id) ON DELETE RESTRICT so the DB atomically rejects deleting an
assigned role, closing the count-then-delete race; the resulting FK error
is mapped to 412 in the role Delete controller.
- Swagger: document 409 (CreateRole duplicate-name conflict) and 412
(DeleteRole still-assigned) responses.
- Tests: FK-race delete -> 412, and a Create failure-injection so dropping
the transaction would fail.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Max Grau Stenzel <max.grau.stenzel@gmail.com>
---------
Signed-off-by: Max Grau Stenzel <max.grau.stenzel@gmail.com>
Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Prasanth Baskar <prasanth@8gears.com>
