[WIP] Add support for Backstage/RHDH MCP DCR #219
Open
maysunfaisal wants to merge 1 commit into
Open
Conversation
maysunfaisal
requested review from
JslYoon,
gabemontero,
johnmcollier,
michael-valdron and
thepetk
as code owners
Code Review by Qodo
1. rhdh-plugin-lightspeed tag format invalid
|
PR Summary by QodoEnable DCR auth for Lightspeed MCP servers (RHDH/Backstage Helm) WalkthroughsDescription• Switch Lightspeed frontend/backend to custom OCI builds with DCR support • Enable Backstage experimental Dynamic Client Registration and OAuth2 consent route • Turn on RBAC permissions and mount policy CSV via ConfigMap Diagramgraph TD
H["Helm values.yaml"] --> B["Backstage (RHDH) pod"] --> A["Auth plugin (/oauth2)"] --> I{{"OIDC provider"}}
B --> L["Custom Lightspeed plugins"] --> M{{"MCP server"}}
C[("ConfigMap: rbac-policy.csv")] --> B
subgraph Legend
direction LR
_cfg["Config"] ~~~ _svc["Service/Pod"] ~~~ _cm[("ConfigMap")] ~~~ _ext{{"External"}}
end
High-Level AssessmentThe following are alternative approaches to this PR: 1. Wait for upstream Lightspeed DCR images/overlays
2. Constrain DCR redirect URI patterns (instead of '*')
3. Externalize RBAC policy management (OPA/GitOps-managed policies)
Recommendation: The PR’s approach is appropriate for enabling DCR quickly (custom plugin builds + Helm wiring), but plan to switch back to upstream plugin overlays once the referenced upstream work lands. Also strongly consider tightening File ChangesOther (3)
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Comment on lines
339
to
341
| plugins: | ||
| - package: oci://ghcr.io/redhat-developer/rhdh-plugin-export-overlays/red-hat-developer-hub-backstage-plugin-lightspeed:bs_1.49.4__2.8.5 | ||
| - package: oci://quay.io/maysunfaisal/rhdh-plugin-lightspeed:dcr-0.4.0!red-hat-developer-hub-backstage-plugin-lightspeed | ||
| disabled: false |
There was a problem hiding this comment.
1. rhdh-plugin-lightspeed tag format invalid 📘 Rule violation ≡ Correctness
Several OCI plugin package references in charts/rhdh/values.yaml use non-compliant tag texts (dcr-0.4.0!… for the Lightspeed and Lightspeed backend plugins, and pr_2498__0.1.6 for a newly added package) that do not match the required bs_<backstage-version>__<plugin-version> format. This violates the enforced plugin image tag convention mandated for plugin images in this values file.
Agent Prompt
## Issue description
`charts/rhdh/values.yaml` contains multiple OCI plugin package tags that do not follow the required `bs_<backstage-version>__<plugin-version>` convention (including `dcr-0.4.0!…` tags for the Lightspeed and Lightspeed backend plugins and the newly added `pr_2498__0.1.6` tag).
## Issue Context
PR Compliance ID 902156 requires all OCI plugin image tags in `charts/rhdh/values.yaml` to match `bs_<backstage-version>__<plugin-version>` (i.e., start with `bs_` and use the `__` separator between the two non-empty segments). Update the non-compliant tags to conform to this enforced format.
## Fix Focus Areas
- charts/rhdh/values.yaml[13-15]
- charts/rhdh/values.yaml[339-341]
- charts/rhdh/values.yaml[366-368]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+7
to
+24
| ##### Disable upstream lightspeed (replaced by custom DCR builds) ##### | ||
| - package: oci://ghcr.io/redhat-developer/rhdh-plugin-export-overlays/red-hat-developer-hub-backstage-plugin-lightspeed:bs_1.49.4__2.8.5 | ||
| disabled: true | ||
| - package: oci://ghcr.io/redhat-developer/rhdh-plugin-export-overlays/red-hat-developer-hub-backstage-plugin-lightspeed-backend:bs_1.49.4__2.8.5 | ||
| disabled: true | ||
|
|
||
| ##### OAuth2 consent page plugin (required for DCR) ##### | ||
| - package: oci://ghcr.io/redhat-developer/rhdh-plugin-export-overlays/backstage-plugin-auth:pr_2498__0.1.6 | ||
| disabled: false | ||
| pluginConfig: | ||
| dynamicPlugins: | ||
| frontend: | ||
| backstage.plugin-auth: | ||
| dynamicRoutes: | ||
| - path: /oauth2 | ||
| importName: Router | ||
| module: PluginRoot | ||
|
|
There was a problem hiding this comment.
2. Ci helm index drift 🐞 Bug ☼ Reliability
charts/rhdh/values.yaml prepends new entries to global.dynamic.plugins, shifting the positional indices that scripts/ci-setup.sh overrides with --set global.dynamic.plugins[8]/[9]. This can disable unintended plugins in CI (or fail to disable the intended ones), causing CI installs to behave incorrectly or fail.
Agent Prompt
## Issue description
`charts/rhdh/values.yaml` prepends new items to `global.dynamic.plugins`, but CI uses positional Helm overrides (`global.dynamic.plugins[8]` and `[9]`) in `scripts/ci-setup.sh`. After the prepend, the same indices point at different plugin entries.
## Issue Context
Helm array overrides are order-dependent; inserting entries at the head of the list changes the meaning of any `--set ...[N]...` overrides.
## Fix Focus Areas
- scripts/ci-setup.sh[101-110]
- charts/rhdh/values.yaml[1-80]
## Suggested fix approaches
- Preferred: stop using hard-coded indices in CI. Instead, generate a small temporary values file in CI that disables plugins by matching `package:` (e.g., via `yq`), then pass that file to `helm install`.
- Acceptable fallback: if you must keep index-based overrides, update `scripts/ci-setup.sh` indices to match the new ordering **and** add a guard/comment that these must be updated whenever `global.dynamic.plugins` ordering changes.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+424
to
+427
| experimentalDynamicClientRegistration: | ||
| enabled: true | ||
| allowedRedirectUriPatterns: | ||
| - '*' |
There was a problem hiding this comment.
3. Wildcard redirect uris 🐞 Bug ⛨ Security
values.yaml enables experimentalDynamicClientRegistration while allowing allowedRedirectUriPatterns: ['*'], which permits redirect URIs to match anything. This undermines OAuth redirect URI validation and can allow authorization code/token exfiltration to attacker-controlled redirect endpoints.
Agent Prompt
## Issue description
`auth.experimentalDynamicClientRegistration.allowedRedirectUriPatterns` is set to `'*'`, effectively allowing any redirect URI for dynamically registered clients.
## Issue Context
OAuth redirect URI validation is a key security control; wildcard patterns make it possible for a malicious client registration to supply a redirect URI controlled by an attacker.
## Fix Focus Areas
- charts/rhdh/values.yaml[422-427]
## Suggested fix
Replace the wildcard with a narrowly-scoped allowlist (e.g., specific origins/paths you control such as `${RHDH_BASE_URL}/oauth2/*` or exact callback URLs). If multiple environments are needed, make this configurable via chart values with safe defaults (no wildcard).
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+575
to
+581
| permission: | ||
| enabled: true | ||
| rbac: | ||
| policies-csv-file: /opt/app-root/src/rbac-policy.csv | ||
| admin: | ||
| users: | ||
| - name: user:default/mfaisal |
There was a problem hiding this comment.
4. Hardcoded rbac admin user 🐞 Bug ⛨ Security
The chart hardcodes user:default/mfaisal as a permission RBAC admin and also binds that same user to role:default/mcp-admin in the shipped rbac-policy.csv. Any environment where that identity exists will grant elevated permissions unintentionally via default chart install.
Agent Prompt
## Issue description
A specific human identity (`user:default/mfaisal`) is embedded in the chart defaults as an RBAC admin and is also granted the `mcp-admin` role in the policy CSV ConfigMap.
## Issue Context
Charts are often reused across environments; hardcoding a privileged user can unintentionally grant admin access in clusters where that identity exists.
## Fix Focus Areas
- charts/rhdh/values.yaml[575-581]
- charts/rhdh/templates/rbac-policy-configmap.yaml[1-23]
## Suggested fix
- Remove the hardcoded user from defaults.
- Parameterize admin users and role bindings via values (e.g., `.Values.backstage.upstream.backstage.appConfig.permission.rbac.admin.users` and/or a chart value like `.Values.permission.rbacPolicyCsv`), with an empty default.
- If a demo user is needed, gate it behind an explicit `demo: true`/`unsafeDefaults: true` value so production installs don’t inherit it.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
gabemontero
reviewed
Jun 11, 2026
| data: | ||
| rbac-policy.csv: | | ||
| p, role:default/mcp-admin, catalog.entity.read, read, allow | ||
| p, role:default/mcp-admin, catalog.entity.create, create, allow |
Contributor
There was a problem hiding this comment.
it is going to be very cool to get this level of authorization granularity once the DCR stuff lands @maysunfaisal @johnmcollier :-)
maysunfaisal
force-pushed
the
feat/dcr-mcp-auth-v2
branch
3 times, most recently
from
4b5a589 to
90948b7
Compare
- Add custom Lightspeed OCI images with DCR support (dcr-0.4.0) - Configure MCP server with auth: dcr in values.yaml - Enable experimentalDynamicClientRegistration - Add @backstage/plugin-auth for OAuth2 consent page Co-authored-by: Cursor <cursoragent@cursor.com>
maysunfaisal
force-pushed
the
feat/dcr-mcp-auth-v2
branch
from
90948b7 to
5e75e62
Compare
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.
What does this PR do?
I'll clean up this PR/branch as things progress on redhat-developer/rhdh-plugins#3347
Which issue(s) does this PR fix
https://redhat.atlassian.net/browse/RHIDP-11657
How to test changes / Special notes to the reviewer