Repository navigation
chore: sync main into develop (23 Sep) - #7052
Merged
Merged
Conversation
GetAllApiTokensForWebhook checked whether each STORED api token had trigger permission on the requested project/env, but never checked whether the CALLING user was permitted to query that project/env at all. As a result any authenticated user could request an arbitrary projectName/environmentName/appName and receive the plaintext token values of every stored api token (including super-admin tokens, whose Casbin policy is "*" and therefore always passes the per-token check). Add the same enforcement pattern used by the sibling handlers in this file, but scoped to the resource being queried: verify the requesting user's own token has trigger permission on the requested project and every requested environment before the service call is made, so no token data is looked up or returned for a project/env the caller cannot access. Fixes #7013
Inline the unnecessary "ok" variable in the caller-RBAC check added in the previous commit, per SonarCloud's "remove this unnecessary variable declaration and use the expression directly" finding on PR #7016.
…allers Checking that a caller has trigger permission on the requested project/env was not sufficient: a super-admin api-token's Casbin policy is "*", so it always satisfies that same per-object check regardless of how narrow the actual project/env being queried is. That meant any caller with legitimate, narrow trigger access to some app/env could still have super-admin (or other more-privileged) tokens returned alongside their own in the same response. Add callerDominatesToken, which excludes a stored token from the response whenever it is a super-admin token and the caller is not, reusing the same super-admin check already used by the other handlers in this file. The per-object check now runs against each stored token before this dominance check, matching the existing per-token-per-object evaluation the service already performs. The Enforcer interface here only exposes single-tuple Enforce/ EnforceInBatch calls, not policy or role enumeration, so a fully generic "is token's permission a strict subset of the caller's" comparison across every possible object isn't available without extending that interface. Super-admin dominance is the well-defined, checkable case that matches this issue (leaking admin token values), so it's what this fix targets; a more general scope comparison would need broader RBAC changes outside this endpoint's blast radius.
Group consecutive same-type parameters (storedToken/projObj/envObj and callerToken/storedToken) per SonarCloud's finding on the code added in the previous two commits.
Per thread decision: GetAllApiTokensForWebhook's only real caller is the dashboard's webhook config modal, which is already super-admin-only in the UI. Rather than layering a per-project/env caller check plus a token-dominance check on top of the existing per-stored-token filter, just gate the whole endpoint on the caller being a super-admin - the same Enforce(token, ResourceGlobal, ActionUpdate, "*") check already used by every other handler in this file (GetAllApiTokens, CreateApiToken, UpdateApiToken, DeleteApiToken). This removes the per-object caller check and the callerDominatesToken logic added in the previous two commits; the existing per-stored-token CheckAuthorizationForWebhook callback into the service is left as-is since it still does useful filtering (only tokens with trigger permission on the requested project/env are returned), it's just no longer load-bearing for access control now that the endpoint itself requires a super-admin caller. Net effect vs main: an 8-line super-admin check added right after GetLoggedInUser, matching the pattern already used by every sibling handler in this file.
Inline the unnecessary "ok" variable in the super-admin check added to GetAllApiTokensForWebhook, per SonarCloud's "remove this unnecessary variable declaration" finding on PR #7016.
…-7013 fix(security): enforce caller RBAC in API token webhook endpoint
… performance - Introduce `createApp`/`deleteApp` actions to segregate application lifecycle control from pipeline/workflow/config operations. - Update RBAC enforcement to reflect the new `createApp`/`deleteApp` actions for better granularity. - Add `idx_ci_artifact`, `idx_ci_workflow`, and other indexes to improve CI query execution and resolve PG_READ_TIMEOUT issues. - Introduce GIN index for `git_triggers` to optimize material/commit lookup in `validateBuildSequence`.
DELETE /orchestrator/user/bulk with a listingRequest filter (instead of
explicit ids) resolved the delete target list via the same query
builder the listing endpoint uses, but skipped the normalization the
listing endpoint applies first (SetDefaultValuesIfNotPresent,
setStatusFilterType). With no default page size and no status
filtering applied, a filter such as {"searchKey":"","status":["inactive"]}
resolved to every active user instead of the intended filtered subset,
causing a bulk delete of far more users than intended.
getUserIdsHonoringFilters now applies the same normalization the
listing path (GetAllWithFilters) applies before building the query,
and rejects a filter-based bulk delete that carries no concrete filter
criteria instead of letting it silently resolve to "every user". The
explicit ids-based delete path is unchanged.
Fixes #7020
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01J5DesTG5XvPdmPCDuL6izq
feat: decouple app lifecycle actions; add indexes to improve CI query…
fix: scope filter-based bulk user delete to match listing filters
When an external helm app is linked to the chart store, app_name holds the unique identifier (<releaseName>-<namespace>-<clusterId>) while display_name holds the release name. RBAC policies are always created against the release name, since that is what the listing APIs expose to the user. EnforcerUtil did not account for this: - GetHelmObjectByAppNameAndEnvId looked the app up by app_name only, so a release-name lookup returned ErrNoRows and the object collapsed to "//", making the app invisible to any non super-admin user. - Both it and GetHelmObject built the object from application.AppName, i.e. the unique identifier, which never matched the granted policy. - Neither substituted the "unassigned" project when team_id is 0, so an app linked without a project produced an empty project segment. EnforcerUtilHelm.GetAppRBACNameByInstalledAppId and GetAppRBACNameByInstalledAppIdAndTeamId had the same raw AppName problem. Resolve the app the same way getAppObject already does: reconstruct the unique identifier from the environment and try that first, falling back to the plain name for regular chart store apps, legacy rows and callers that already pass the identifier. Emit the display name via the new App.GetAppNameForRbac(), and fall back to the unassigned project when no project is assigned. Fixing the primitives covers all downstream call sites without touching the handlers. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…re intentionally not sent for the plugin's internal steps.
feat: resolve RBAC inconsistencies for helm apps with unique identifiers
…-pipeline-level-oss misc: Conditions saved with the plugin definition (plugin_step_condition) a…
* chore: create a makefile to setup and clean resources for development * feat: add flag for frontend to show argo/flux tabs - add migration files - append flags in the /orchestrator/user/check/roles API to show/hide argo/flux tabs * feat: changes for dropdown API in user permissions page * fix: missing dependencies in constructor method * fix: improper filtering logic - earlier filter was only applied with the cluster ID - add changes to filter based on cluster ID's and namespaces * fix: prevent connecting to kubelink for failed clusters * wip: policy enforcements * wip: policy enforcements * chore: remove test folder * fix: code review fixes * fix: code review fixes * fix: code review fixes * chore: rename migration scripts to maintain sync with enterprise * chore: change migration script number * fix: roles missing while fetching user roles * fix: deleted roles being listed * fix: deleted roles being listed * fix: review fixes
Points common-lib and authenticator at devtron-services main (f0b3fba15081b8d40a795928aaad5df55473c681, the v0.47.0 RC merge). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
chore: update devtron-services deps to main for v0.47.0
…bac idempotent
Two different migrations shared version 36504600 on the release branch:
add_ci_workflow_active_status_index and dev_admin_app_lifecycle. The
latter is already released on main at that number, so the CI index
migration (RC-only) is renumbered to 36904600, the next free slot.
Also adds ON CONFLICT ("entity","access_type","role") DO NOTHING to both
INSERTs in 36804600_argo_flux_rbac.up.sql. Both rbac_policy_data and
rbac_role_data declare UNIQUE (entity, access_type, role), so a re-run
previously failed on the unique constraint instead of being a no-op.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
….47.0 chore: sync main into release-candidate-v0.47.0
misc: Release candidate v0.47.0
…nation The Devtron Apps list dropped the entire AppStatuses filter whenever HIBERNATING was among the selected statuses, ran LIMIT/OFFSET and count(distinct a.id) over the *unfiltered* app set, and only then re-applied the status filter in memory -- to the single page that came back -- while overwriting the total count with the size of that surviving slice. So the filter returned just the hibernating apps that happened to land on the requested page of the unfiltered listing, and reported that handful as the grand total, leaving the UI with no further pages. Verified against a customer database: 592 hibernating apps exist, 2 were shown. HIBERNATING is an ordinary value persisted in app_status.status (appStatus.UpdateStatusWithAppIdEnvId normalises Suspended/Hibernated to it), and the in-memory pass compared against that very same column -- so the predicate was always expressible in SQL. buildAppListingWhereCondition already emits `aps.status IN (?)` for any status list, including the NOT DEPLOYED combination, so the workaround is simply removed rather than replaced. This also fixes combination filters that were collateral damage: selecting Healthy + Hibernating blanked both predicates, and Hibernating + Not Deployed additionally killed the NOT DEPLOYED SQL branch. The workaround was originally added for Helm-type pipelines, whose status is not persisted in app_status. That cohort is unaffected either way -- the in-memory pass read the same empty column -- and is now documented as a TODO in place of the removed code. Pager: devtron-labs/sprint-tasks#2974 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ination-2974 fix: Hibernating filter in Devtron Apps list shows incomplete results (pager #2974)
Shivam-nagar23
requested review from
prakarsh-dt and
vivek-devtron
as code owners
September 23, 2026 07:27
|
Bito Automatic Review Skipped - Branch Excluded |
vivek-devtron
approved these changes
Sep 23, 2026
|
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
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.
Description
Routine sync of
mainintodevelop.developwas 27 commits behindmainand 0 commits ahead, so this is a clean fast-forward — no conflicts, no manual resolution.Notable changes being brought in
6ab8595f05/c24e9d825dfix: apply Hibernating app-status filter in SQL instead of after pagination (fix: Hibernating filter in Devtron Apps list shows incomplete results (pager #2974) #7049)50fa81bfa2release-candidate-v0.47.0 (misc: Release candidate v0.47.0 #7045)c83900e0c7chore(sql): resolve migration 36504600 collision and make argo/flux rbac idempotent5e957a5e34chore: update devtron-services deps to main for v0.47.0bbb3ec7aa7feat: RBAC support for external argo/flux apps (feat: RBAC support for external argo/flux apps #7014)72dcd49fe9feat: trigger conditions at pipeline level (misc: Conditions saved with the plugin definition (plugin_step_condition) a… #7035)1679300d2efeat: resolve RBAC inconsistencies for helm apps with unique identifiers (feat: resolve RBAC inconsistencies for helm apps with unique identifiers #7034)4b2bf385d8fix(user): scope filter-based bulk delete to match listing filters (fix: scope filter-based bulk user delete to match listing filters #7021)5e5ebdc9f6feat: decouple app lifecycle actions; add indexes to improve CI query performance (feat: decouple app lifecycle actions; add indexes to improve CI query… #7019)33f57640b9…ba2f3b9f18fix(security): enforce caller RBAC in API token webhook endpoint, super-admin gate (fix(security): enforce caller RBAC in API token webhook endpoint #7016)Testing
No code changes authored here — content is identical to
main, which is already released/verified.Paired with the enterprise sync PR devtron-labs/devtron-enterprise#3423.
🤖 Generated with Claude Code