[RHIDP-13060] harden lightspeed proxy against arbitrary routes

[Jdubrick]

Hey, I just made a Pull Request!

• Adds an explicit allowlist for routes to restrict authenticated users from hitting arbitrary routes on the LCORE side
• Moved the pre-existing passthrough routes array to constants

Issue: https://redhat.atlassian.net/browse/RHIDP-13060

✔️ Checklist

• A changeset describing the change and affected packages. (more info)
• Added or Updated documentation
• Tests for new functionality and regression tests for bug fixes
• Screenshots attached (for UI changes)

[rhdh-qodo-merge]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (1)

Context used

✅ Tickets: 🎫 Open proxy surface to LCS

1. isAllowedProxyPath lacks normalization 📎 Requirement gap ⛨ Security

Description

The new allowlist check is a simple prefix match and does not normalize or reject dot-segment paths
(e.g., /v1/models/../admin), which can allow requests that are not explicitly allowlisted to be
forwarded if the upstream normalizes the path. This weakens the “reject all other paths” guarantee
intended to harden the proxy surface.

Code

workspaces/lightspeed/plugins/lightspeed-backend/src/service/validation.ts[R22-26]

+export function isAllowedProxyPath(path: string): boolean {
+  return ALLOWED_PROXY_PREFIXES.some(
+    prefix => path === prefix || path.startsWith(`${prefix}/`),
+  );
+}

Relevance

⭐⭐⭐ High

Team accepts security hardening (path traversal/"../" blocked in PR #2913; SSRF allowlisting
tightened in PR #2581).

PR-#2913
PR-#2581

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

PR Compliance ID 1 requires that any non-allowlisted path be rejected instead of being forwarded to
LCS. The implementation allows any path that string-matches an allowlisted prefix without path
normalization, meaning paths that start with an allowlisted prefix but resolve to a different
endpoint after normalization are not explicitly rejected.

Allowlist-only proxy access to LCS endpoints (reject all other paths)
workspaces/lightspeed/plugins/lightspeed-backend/src/service/validation.ts[22-26]
workspaces/lightspeed/plugins/lightspeed-backend/src/service/router.ts[428-430]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`isAllowedProxyPath()` performs a raw string prefix check without normalizing the path or rejecting dot-segments. This can allow paths that begin with an allowlisted prefix but effectively target a different endpoint after normalization (e.g., containing `..`).

## Issue Context
The proxy hardening requirement is to forward requests only to explicitly allowlisted LCS paths and reject everything else with 404.

## Fix Focus Areas
- workspaces/lightspeed/plugins/lightspeed-backend/src/service/validation.ts[22-26]
- workspaces/lightspeed/plugins/lightspeed-backend/src/service/router.ts[428-430]
- workspaces/lightspeed/plugins/lightspeed-backend/src/service/validation.test.ts[19-53]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

[rhdh-gh-app]

Changed Packages

Package Name	Package Path	Changeset Bump	Current Version
@red-hat-developer-hub/backstage-plugin-lightspeed-backend	workspaces/lightspeed/plugins/lightspeed-backend	patch	v2.5.1

[rhdh-qodo-merge]

Review Summary by Qodo

Add route allowlist to harden lightspeed proxy security

✨ Enhancement 🧪 Tests

Walkthroughs

Description

• Adds explicit allowlist for LCORE proxy routes to prevent arbitrary endpoint access
• Moves passthrough paths to constants for better maintainability
• Implements path validation middleware to reject non-allowlisted requests
• Adds comprehensive test coverage for allowlist validation logic

Diagram

flowchart LR
  A["Request to /api/lightspeed/*"] --> B["Check PROXY_PASSTHROUGH_PATHS"]
  B -->|Match| C["Skip validation"]
  B -->|No match| D["Check ALLOWED_PROXY_PREFIXES"]
  D -->|Allowed| E["Proxy to LCORE"]
  D -->|Blocked| F["Return 404 error"]

Loading

File Changes

1. workspaces/lightspeed/plugins/lightspeed-backend/src/service/constant.ts ⚙️ Configuration changes +20/-0

Define proxy route allowlist and passthrough constants

• Added ALLOWED_PROXY_PREFIXES constant with four allowed LCORE path prefixes
• Added PROXY_PASSTHROUGH_PATHS constant for routes handled by dedicated handlers
• Includes documentation explaining security rationale for the allowlist

workspaces/lightspeed/plugins/lightspeed-backend/src/service/constant.ts

---

2. workspaces/lightspeed/plugins/lightspeed-backend/src/service/validation.ts ✨ Enhancement +7/-0

Implement path validation against allowlist

• Added isAllowedProxyPath() function to validate paths against allowlist
• Function checks for exact matches or sub-paths under allowed prefixes
• Imported ALLOWED_PROXY_PREFIXES from constants

workspaces/lightspeed/plugins/lightspeed-backend/src/service/validation.ts

---

3. workspaces/lightspeed/plugins/lightspeed-backend/src/service/validation.test.ts 🧪 Tests +53/-0

Add validation tests for proxy path allowlist

• New test file with comprehensive coverage for isAllowedProxyPath() function
• Tests exact path matches for all four allowed prefixes
• Tests sub-path allowance under allowed prefixes
• Tests rejection of arbitrary and malformed paths

workspaces/lightspeed/plugins/lightspeed-backend/src/service/validation.test.ts

---

View more (3)

4. workspaces/lightspeed/plugins/lightspeed-backend/src/service/router.ts ✨ Enhancement +11/-8

Integrate path validation into proxy middleware

• Imported PROXY_PASSTHROUGH_PATHS constant and isAllowedProxyPath function
• Replaced inline passthrough paths array with constant reference
• Added middleware check to validate proxy paths and return 404 for non-allowlisted routes
• Maintains existing behavior for notebooks routes and PUT requests

workspaces/lightspeed/plugins/lightspeed-backend/src/service/router.ts

---

5. workspaces/lightspeed/plugins/lightspeed-backend/src/service/router.test.ts 🧪 Tests +35/-0

Add router tests for proxy allowlist enforcement

• Added new test suite proxy path allowlist with three test cases
• Tests rejection of non-allowlisted paths like /v1/admin and /internal/config
• Tests rejection of arbitrary POST requests to unauthorized paths
• All tests verify 404 response with appropriate error message

workspaces/lightspeed/plugins/lightspeed-backend/src/service/router.test.ts

---

6. workspaces/lightspeed/.changeset/brown-rivers-build.md 📝 Documentation +5/-0

Add changeset for proxy hardening feature

• New changeset file documenting the security hardening change
• Marks change as patch version for lightspeed-backend plugin
• Describes the addition of route allowlist for proxy passthrough

workspaces/lightspeed/.changeset/brown-rivers-build.md

---

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 60.56%. Comparing base (c7ff423) to head (32526a7).
⚠️ Report is 1 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files

@@           Coverage Diff            @@
##             main    #2970    +/-   ##
========================================
  Coverage   60.56%   60.56%            
========================================
  Files        2005     2005            
  Lines       62781    62788     +7     
  Branches    16366    16370     +4     
========================================
+ Hits        38023    38030     +7     
+ Misses      24244    24141   -103     
- Partials      514      617   +103     

Flag	Coverage Δ		*Carryforward flag
adoption-insights	83.58% <ø> (ø)		Carriedforward from 6ece995
app-defaults	69.60% <ø> (ø)		Carriedforward from 6ece995
augment	69.36% <ø> (ø)		Carriedforward from 6ece995
bulk-import	72.57% <ø> (ø)		Carriedforward from 6ece995
cost-management	16.49% <ø> (ø)		Carriedforward from 6ece995
dcm	33.63% <ø> (ø)		Carriedforward from 6ece995
extensions	61.42% <ø> (ø)		Carriedforward from 6ece995
global-floating-action-button	73.75% <ø> (ø)		Carriedforward from 6ece995
global-header	61.56% <ø> (ø)		Carriedforward from 6ece995
homepage	50.92% <ø> (ø)		Carriedforward from 6ece995
konflux	91.01% <ø> (ø)		Carriedforward from 6ece995
lightspeed	69.57% <100.00%> (+0.04%)	⬆️
mcp-integrations	81.59% <ø> (ø)		Carriedforward from 6ece995
orchestrator	33.08% <ø> (ø)		Carriedforward from 6ece995
quickstart	62.64% <ø> (ø)		Carriedforward from 6ece995
sandbox	79.49% <ø> (ø)		Carriedforward from 6ece995
scorecard	83.23% <ø> (ø)		Carriedforward from 6ece995
theme	64.54% <ø> (ø)		Carriedforward from 6ece995
translations	8.49% <ø> (ø)		Carriedforward from 6ece995
x2a	82.09% <ø> (ø)		Carriedforward from 6ece995

*This pull request uses carry forward flags. Click here to find out more.

---

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c7ff423...32526a7. Read the comment docs.

🚀 New features to boost your workflow:

• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[Jdubrick]

/review

[rhdh-qodo-merge]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶 RHIDP-13060 - Partially compliant Compliant requirements: Restrict the proxy so users cannot proxy arbitrary LCORE/LCS endpoints Implement an explicit allowlist of path prefixes that may be proxied Reject all other paths with HTTP 404 Non-compliant requirements: Requires further human verification: Verify in a deployed environment that requests with encoded/ambiguous paths (e.g., %2f, %2e, double slashes) cannot bypass the allowlist and still reach non-allowlisted LCORE/LCS endpoints

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪

🧪 PR contains tests

🔒 Security concerns Possible allowlist bypass: The new allowlist enforcement is a significant hardening, but it depends on req.path matching behavior. If upstream routing/proxying can interpret encoded path separators or dot segments differently than Express’s req.path normalization, an attacker might craft a request that passes isAllowedProxyPath yet reaches a different upstream endpoint. This should be validated with URL-encoded traversal/separator cases (e.g., %2f, %2e, double slashes) against the running service.

⚡ Recommended focus areas for review Allowlist bypass The allowlist check relies on req.path string matching; it’s worth validating that Express/path normalization + URL-encoded inputs (e.g., %2f, %2e, repeated slashes) cannot produce a req.path that passes isAllowedProxyPath while proxying to a different upstream route than intended. router.use('/', async (req, res, next) => { // Skip middleware for notebooks routes and specific paths if ( req.path.startsWith('/notebooks') || PROXY_PASSTHROUGH_PATHS.includes(req.path) || req.method === 'PUT' ) { return next(); } if (!isAllowedProxyPath(req.path)) { return res.status(404).json({ error: 'Requested path is not available' }); } Inconsistent constants ALLOWED_PROXY_PREFIXES includes /v1/feedback while PROXY_PASSTHROUGH_PATHS also includes /v1/feedback (bypassing the proxy middleware entirely). This duplication may be confusing and could lead to incorrect future assumptions about whether /v1/feedback is proxied vs handled locally. export const ALLOWED_PROXY_PREFIXES = [ '/v1/models', '/v1/shields', '/v2/conversations', '/v1/feedback', ]; /** * Paths that bypass the proxy middleware and are handled by dedicated route handlers */ export const PROXY_PASSTHROUGH_PATHS = [ '/v1/query', '/v1/query/interrupt', '/v1/feedback', ]; Edge-case matching isAllowedProxyPath uses a strict prefix or prefix + '/' match which is good, but consider whether trailing slashes (/v1/models/) and empty/undefined path inputs are consistently handled across all request types and middleware ordering to avoid accidental denial/allow behavior changes. export function isAllowedProxyPath(path: string): boolean { return ALLOWED_PROXY_PREFIXES.some( prefix => path === prefix || path.startsWith(`${prefix}/`), ); }

📄 References No matching references available

[Jdubrick]

For reviewers in response to Qodo's review:

1. I confirmed LCORE does not normalize, so the encoded cases will return a 404 if they make it to LCORE
2. /v1/feedback is in both constant arrays as there is the exact match /v1/feedback and the partial match /v1/feedback/status where one has its own handler and one does not

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[karthikjeeyar]

Changes looks good!

/approve
/lgtm