feat(intelligent-assistant): add new endpoint to list available skills

[yangcao77]

Hey, I just made a Pull Request!

fixes https://redhat.atlassian.net/browse/RHIDP-14226
fixes https://redhat.atlassian.net/browse/RHIDP-14225

Summary

• Add new RBAC permission intelligent-assistant.skills.access to gate visibility of the skills list
  endpoint
• Add GET /api/intelligent-assistant/v1/skills endpoint that proxies LCORE's GET /v1/skills, returning
  loaded skill metadata (name, description) as-is
• Add the new permission to rbac-policy.csv for the default user role

Details

Permission (intelligent-assistant-common):

• New iaSkillsAccessPermission with name: 'intelligent-assistant.skills.access' and no attributes, matching
  the iaChatAccessPermission pattern
• Registered in the iaPermissions array for automatic integration with the Backstage permissions framework

Endpoint (intelligent-assistant-backend):

• GET /v1/skills uses a standalone fetch() call to LCORE (matching the existing pattern for non-streaming
  endpoints like /v1/feedback), gated by iaSkillsAccessPermission with general rate limiting
• LCORE response is passed through as-is — no interpretation or enrichment by RHDH

Tests:

• 4 new integration tests: skills loaded, empty list, unauthorized (403), LCORE unreachable (500)

✔️ 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-gh-app]

Important

This PR includes changes that affect public-facing API. Please ensure you are adding/updating documentation for new features or behavior.

Changed Packages

Package Name	Package Path	Changeset Bump	Current Version
@red-hat-developer-hub/backstage-plugin-intelligent-assistant-backend	workspaces/intelligent-assistant/plugins/intelligent-assistant-backend	minor	v3.2.0
@red-hat-developer-hub/backstage-plugin-intelligent-assistant-common	workspaces/intelligent-assistant/plugins/intelligent-assistant-common	patch	v3.2.0

[rhdh-qodo-merge]

PR Summary by Qodo

Add RBAC-gated /v1/skills endpoint for Intelligent Assistant

✨ Enhancement 🧪 Tests ⚙️ Configuration changes 🕐 20-40 Minutes

AI Description

• Add RBAC-gated GET /api/intelligent-assistant/v1/skills proxying LCORE /v1/skills.
• Introduce intelligent-assistant.skills.access permission and grant to default IA user role.
• Add integration tests and MSW fixtures for skills success, empty, 403, and 500 cases.

Diagram

graph TD
  U["Backstage client"] --> R["IA backend router"] --> PF["Permission check"] --> L["LCORE skills API"] --> U
  C["IA common permissions"] --> R --> L
  RBAC[("rbac-policy.csv")] --> PF --> R

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Use apiProxy middleware for /v1/skills

• ➕ Less bespoke code; consistent with other proxied endpoints
• ➕ Preserves upstream status/headers without manual JSON parsing
• ➖ Harder to customize error payloads/logging consistently
• ➖ May require additional skip/rewrite handling to avoid unintended user_id behavior

2. Reuse an existing permission (e.g., chat.access) instead of adding a new one

• ➕ Fewer permissions to manage in policy files
• ➕ Simplifies RBAC configuration for admins
• ➖ Over-broad access (violates least privilege)
• ➖ Harder to selectively expose skills listing without exposing chat features

Recommendation: The PR’s approach (new least-privilege permission + explicit fetch-based proxy) is appropriate for an admin-controlled “visibility” endpoint and matches the existing non-streaming /v1/feedback style. If more simple pass-through endpoints are added later, consider factoring a small helper or switching to apiProxy for consistency, but the current implementation is clear and test-covered.

Files changed (5) +117 / -0

Enhancement (2) +38 / -0

router.tsImplement RBAC-gated GET /v1/skills route +29/-0

Implement RBAC-gated GET /v1/skills route

• Adds a new GET /v1/skills endpoint with general rate limiting and iaSkillsAccessPermission enforcement. Proxies LCORE’s /v1/skills via fetch(), passing through JSON on success and returning 500 on network errors or mapped upstream errors via handleLCSFetchError.

workspaces/intelligent-assistant/plugins/intelligent-assistant-backend/src/service/router.ts

permissions.tsDefine and register iaSkillsAccessPermission +9/-0

Define and register iaSkillsAccessPermission

• Adds a new permission named intelligent-assistant.skills.access (no attributes) and registers it in iaPermissions for Backstage permission framework integration.

workspaces/intelligent-assistant/plugins/intelligent-assistant-common/src/permissions.ts

Tests (2) +78 / -0

lcsHandlers.tsMock LCORE /v1/skills response in MSW handlers +15/-0

Mock LCORE /v1/skills response in MSW handlers

• Adds an MSW GET handler for the LCORE /v1/skills endpoint returning a sample skills list. This supports integration tests for the new backend skills route.

workspaces/intelligent-assistant/plugins/intelligent-assistant-backend/fixtures/lcsHandlers.ts

router.test.tsAdd integration tests for GET /v1/skills +63/-0

Add integration tests for GET /v1/skills

• Introduces tests validating successful proxying of the skills list, empty results, permission denial (403), and upstream failure handling (500). Uses MSW overrides to simulate LCORE responses.

workspaces/intelligent-assistant/plugins/intelligent-assistant-backend/src/service/router.test.ts

Other (1) +1 / -0

rbac-policy.csvGrant skills access to default intelligent-assistant-user role +1/-0

Grant skills access to default intelligent-assistant-user role

• Adds a policy entry granting intelligent-assistant.skills.access (use) to the default Intelligent Assistant user role for local dev/testing.

workspaces/intelligent-assistant/rbac-policy.csv

[rhdh-qodo-merge]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. No timeout on skills fetch ⊘ Outdated 🐞 Bug ☼ Reliability

Description

GET /v1/skills performs an upstream fetch to LCS without any AbortSignal timeout, so a stalled
upstream connection can keep the request hanging and consume backend resources. The same file uses
an explicit timeout for another LCS fetch, indicating time-bounded LCS calls are expected.

Code

workspaces/intelligent-assistant/plugins/intelligent-assistant-backend/src/service/router.ts[R669-672]

+        const fetchResponse = await fetch(`${lcsBaseUrl}/v1/skills`);
+
+        if (!fetchResponse.ok) {
+          await handleLCSFetchError(

Relevance

●●● Strong

Repo has accepted adding AbortSignal timeouts to prevent hung upstream fetches.

PR-#2581

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The newly added /v1/skills handler uses a bare fetch call with no signal/timeout. In the same
router, another LCS call explicitly sets AbortSignal.timeout(5000), demonstrating an established
expectation to bound upstream waits.

workspaces/intelligent-assistant/plugins/intelligent-assistant-backend/src/service/router.ts[663-683]
workspaces/intelligent-assistant/plugins/intelligent-assistant-backend/src/service/router.ts[243-248]

Agent prompt

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

### Issue description
`GET /v1/skills` calls LCS via `fetch()` without a timeout. If LCS stalls (partial outage, network partition), this handler can remain pending for an unbounded period.

### Issue Context
Elsewhere in the same router (`refreshLcsUrlCache`) the LCS fetch uses `AbortSignal.timeout(5000)`. Use a similar timeout for `/v1/skills` and ensure the timeout error is handled via the sanitized/generic upstream error path (likely a 502/504 depending on conventions).

### Fix Focus Areas
- workspaces/intelligent-assistant/plugins/intelligent-assistant-backend/src/service/router.ts[663-687]
- workspaces/intelligent-assistant/plugins/intelligent-assistant-backend/src/service/router.ts[243-248]

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

---

2. Leaky fetch error response ⊘ Outdated 🐞 Bug ⛨ Security

Description

In GET /v1/skills, the catch block returns Error while fetching skills: ${error} to the client,
which can disclose internal upstream/network/runtime details and is inconsistent with the existing
LCS error sanitization approach. This also uses a 500 even for upstream connectivity failures, where
other code paths use a generic upstream error response.

Code

workspaces/intelligent-assistant/plugins/intelligent-assistant-backend/src/service/router.ts[R684-686]

+        const errormsg = `Error while fetching skills: ${error}`;
+        logger.error(errormsg);
+        response.status(500).json({ error: errormsg });

Relevance

● Weak

Very similar suggestion to avoid echoing raw ${error} in router catch blocks was previously
rejected.

PR-#4076

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The new route’s exception handler echoes the raw error to the client, while the repo already
contains a dedicated sanitization helper for LCS errors and uses a safer pattern elsewhere (log the
error object, return a generic upstream message).

workspaces/intelligent-assistant/plugins/intelligent-assistant-backend/src/service/router.ts[663-687]
workspaces/intelligent-assistant/plugins/intelligent-assistant-backend/src/service/utils.ts[33-81]
workspaces/intelligent-assistant/plugins/intelligent-assistant-backend/src/service/router.ts[1017-1024]

Agent prompt

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

### Issue description
`GET /v1/skills` currently returns a client-facing error containing the caught exception string. This can leak internal details (upstream host/connection info) and is inconsistent with the existing sanitization helper used for non-2xx LCS responses.

### Issue Context
There is already a sanitization and error-handling helper for LCS error bodies (`sanitizeLCSError` / `handleLCSFetchError`). For thrown fetch exceptions (network errors/timeouts), return a generic upstream-unreachable message (e.g., 502) while logging the full error server-side.

### Fix Focus Areas
- workspaces/intelligent-assistant/plugins/intelligent-assistant-backend/src/service/router.ts[663-687]
- workspaces/intelligent-assistant/plugins/intelligent-assistant-backend/src/service/utils.ts[33-81]
- workspaces/intelligent-assistant/plugins/intelligent-assistant-backend/src/service/router.ts[1017-1024]

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

---

Context used

⚠️ Tickets: not configured — ticket URL found in PR but could not be fetched — check ticket provider credentials

✅ Compliance rules (platform): 11 rules

✅ Cross-repo context

  Explored: repo: redhat-developer/rhdh (sha: 4c5a4e85)

  Explored: repo: redhat-developer/rhdh-local (sha: a1776caa)

  Explored: repo: redhat-developer/rhdh-operator (sha: ad48a1de)

  Not relevant to this PR: redhat-developer/rhdh-chart

_{To customize comments, go to the Qodo configuration screen, or learn more in the docs.}

[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

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 58.33%. Comparing base (d10b409) to head (53ab6a9).
✅ All tests successful. No failed tests found.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #4204   +/-   ##
=======================================
  Coverage   58.33%   58.33%           
=======================================
  Files        2432     2432           
  Lines       96775    96776    +1     
  Branches    26899    26913   +14     
=======================================
+ Hits        56456    56457    +1     
  Misses      40116    40116           
  Partials      203      203           

Flag	Coverage Δ		*Carryforward flag
adoption-insights	84.55% <ø> (ø)		Carriedforward from d10b409
ai-integrations	69.76% <ø> (ø)		Carriedforward from d10b409
app-defaults	69.79% <ø> (ø)		Carriedforward from d10b409
augment	46.67% <ø> (ø)		Carriedforward from d10b409
boost	76.77% <ø> (ø)		Carriedforward from d10b409
bulk-import	72.79% <ø> (ø)		Carriedforward from d10b409
cost-management	13.55% <ø> (ø)		Carriedforward from d10b409
dcm	67.21% <ø> (ø)		Carriedforward from d10b409
extensions	56.59% <ø> (ø)		Carriedforward from d10b409
global-floating-action-button	71.18% <ø> (ø)		Carriedforward from d10b409
global-header	66.50% <ø> (ø)		Carriedforward from d10b409
homepage	47.59% <ø> (ø)		Carriedforward from d10b409
install-dynamic-plugins	59.95% <ø> (ø)		Carriedforward from d10b409
intelligent-assistant	75.24% <100.00%> (+<0.01%)	⬆️
konflux	91.98% <ø> (ø)		Carriedforward from d10b409
lightspeed	69.02% <ø> (ø)		Carriedforward from d10b409
mcp-integrations	83.40% <ø> (ø)		Carriedforward from d10b409
orchestrator	66.91% <ø> (ø)		Carriedforward from d10b409
quickstart	63.74% <ø> (ø)		Carriedforward from d10b409
sandbox	79.56% <ø> (ø)		Carriedforward from d10b409
scorecard	86.17% <ø> (ø)		Carriedforward from d10b409
theme	88.77% <ø> (ø)		Carriedforward from d10b409
translations	5.12% <ø> (ø)		Carriedforward from d10b409
x2a	79.20% <ø> (ø)		Carriedforward from d10b409

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

---

Continue to review full report in Codecov by Harness.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d10b409...53ab6a9. 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.