Skip to content

docs(cost-management): update documentation to reflect security PRs - #2679

Merged
PreetiW merged 2 commits into
redhat-developer:mainfrom
PreetiW:docs/update-cost-management-docs-post-security-prs
Apr 6, 2026
Merged

docs(cost-management): update documentation to reflect security PRs#2679
PreetiW merged 2 commits into
redhat-developer:mainfrom
PreetiW:docs/update-cost-management-docs-post-security-prs

Conversation

@PreetiW

@PreetiW PreetiW commented Apr 2, 2026

Copy link
Copy Markdown
Contributor

Description

docs(cost-management): update documentation to reflect security PRs

✔️ 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

rhdh-gh-app Bot commented Apr 2, 2026

Copy link
Copy Markdown

Missing Changesets

The following package(s) are changed by this PR but do not have a changeset:

  • @red-hat-developer-hub/plugin-cost-management-backend
  • @red-hat-developer-hub/plugin-cost-management

See CONTRIBUTING.md for more information about how to add changesets.

Changed Packages

Package Name Package Path Changeset Bump Current Version
@red-hat-developer-hub/plugin-cost-management-backend workspaces/cost-management/plugins/cost-management-backend none v2.0.2
@red-hat-developer-hub/plugin-cost-management workspaces/cost-management/plugins/cost-management none v2.0.3-rc.4

@rhdh-qodo-merge

Copy link
Copy Markdown

Review Summary by Qodo

Update cost-management documentation for security improvements

📝 Documentation

Grey Divider

Walkthroughs

Description
• Remove outdated proxy configuration from workspace README
• Update ADR 0002 status to Accepted with implementation notes
• Clarify permission format (dot vs slash) in RBAC documentation
• Add missing backend endpoints and audit logging documentation
• Fix broken link typo in frontend plugin README
Diagram
flowchart LR
  A["Documentation Updates"] --> B["RBAC Clarification"]
  A --> C["ADR 0002 Acceptance"]
  A --> D["Backend Endpoints"]
  A --> E["Audit Logging"]
  A --> F["Link Fixes"]
  B --> G["Dot vs Slash Permissions"]
  C --> H["Implementation Notes"]
  D --> I["Access & Apply Endpoints"]
  E --> J["Structured Audit Logs"]
Loading

Grey Divider

File Changes

1. workspaces/cost-management/README.md 📝 Documentation +0/-8

Remove outdated proxy configuration

• Removed outdated proxy configuration block from app-config.yaml example
• Reflects PR #2616 which moved data fetching server-side

workspaces/cost-management/README.md


2. workspaces/cost-management/docs/adrs/0002-applying-optimizations-using-the-orchestrator-plugin.md 📝 Documentation +21/-1

Update ADR status and add implementation notes

• Changed status from Proposed to Accepted
• Added comprehensive implementation notes documenting backend gateway pattern
• Documented security hardening including input validation and audit logging
• Referenced related security PRs #2616, #2618, #2619

workspaces/cost-management/docs/adrs/0002-applying-optimizations-using-the-orchestrator-plugin.md


3. workspaces/cost-management/docs/rbac.md 📝 Documentation +4/-4

Clarify permission format in RBAC documentation

• Clarified permission naming format: dot notation for plugin-level, slash for cluster/project
• Updated intro section to explain ros.plugin, ros.apply, ros/CLUSTER patterns
• Updated example policy file description with specific permission formats
• Enhanced permission policy evaluation documentation

workspaces/cost-management/docs/rbac.md


View more (2)
4. workspaces/cost-management/plugins/cost-management-backend/README.md 📝 Documentation +31/-5

Add endpoints documentation and audit logging

• Updated permission sets description to include slash notation (ros/…, cost/…)
• Added three missing endpoints: access check, cost access check, and apply-recommendation
• Added new Audit Logging section with structured log entry format and example
• Clarified that no proxy block is needed in app-config

workspaces/cost-management/plugins/cost-management-backend/README.md


5. workspaces/cost-management/plugins/cost-management/README.md 📝 Documentation +1/-1

Fix broken link typo in README

• Fixed broken link typo: cost-mangament-backend to cost-management-backend

workspaces/cost-management/plugins/cost-management/README.md


Grey Divider

Qodo Logo

@rhdh-qodo-merge

rhdh-qodo-merge Bot commented Apr 2, 2026

Copy link
Copy Markdown

Code Review by Qodo

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

Grey Divider


Action required

1. Sanitization claim untrue🐞 Bug ⛨ Security
Description
ADR-0002 claims that “all input fields are sanitized” for `POST
/api/cost-management/apply-recommendation`, but the backend only validates presence/types and
resourceType allowlisting. This overstates the implemented hardening and can mislead
security/threat-model expectations.
Code

workspaces/cost-management/docs/adrs/0002-applying-optimizations-using-the-orchestrator-plugin.md[R18-21]

+- **Input validation**: `resourceType` is validated against a server-side
+  allowlist; all input fields are sanitized.
+- **Audit logging**: Every apply action is logged with user identity, cluster,
+  namespace, workload, and outcome.
Evidence
The ADR explicitly states sanitization for all input fields, but the route’s validation only checks
that inputs exist and are strings, with an allowlist for resourceType; there is no sanitization
step in this handler.

workspaces/cost-management/docs/adrs/0002-applying-optimizations-using-the-orchestrator-plugin.md[13-21]
workspaces/cost-management/plugins/cost-management-backend/src/routes/applyRecommendation.ts[48-92]

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

### Issue description
ADR-0002 claims apply-recommendation inputs are sanitized, but the implementation performs validation (presence/type checks + `resourceType` allowlist) rather than sanitization. This is a security documentation mismatch.

### Issue Context
The ADR is used to capture security hardening decisions; over-claiming controls (sanitization) can mislead reviewers and operators.

### Fix Focus Areas
- workspaces/cost-management/docs/adrs/0002-applying-optimizations-using-the-orchestrator-plugin.md[18-21]

### Suggested fix
Change wording from “all input fields are sanitized” to something implementation-accurate, e.g. “all input fields are validated for presence/type; `resourceType` is allowlisted”. If sanitization is actually required by the threat model, implement explicit sanitization/normalization in the backend and keep the ADR as written.

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



Remediation recommended

2. Missing audit action documented🐞 Bug ✧ Quality
Description
The backend README documents an audit action value cost_access_check, but the backend
implementation emits access_check for both access endpoints. This mismatch can break log
parsing/alerting that keys off the documented action name.
Code

workspaces/cost-management/plugins/cost-management-backend/README.md[R50-56]

+All backend endpoints emit structured audit log entries with user identity via Backstage's logger. Each entry includes:
+
+- **`actor`** — the authenticated user's entity ref (e.g., `user:default/admin`)
+- **`action`** — the operation performed (`data_access`, `apply_recommendation`, `access_check`, `cost_access_check`)
+- **`decision`** — the RBAC outcome (`ALLOW` or `DENY`)
+- **`resource`** — the upstream API path accessed
+- **`filters`** — the server-injected cluster/project filters (for proxy requests)
Evidence
The README lists cost_access_check as a possible audit action, but the /access/cost-management
route emits action: 'access_check' (in both the allow and filtered paths), meaning
cost_access_check will never appear in logs as documented.

workspaces/cost-management/plugins/cost-management-backend/README.md[50-56]
workspaces/cost-management/plugins/cost-management-backend/src/routes/costManagementAccess.ts[50-59]
workspaces/cost-management/plugins/cost-management-backend/src/routes/costManagementAccess.ts[176-186]

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

### Issue description
The backend README documents an audit log action `cost_access_check`, but the backend code only emits `access_check` for `/access/cost-management`. This creates a doc/implementation mismatch and can cause downstream audit log parsing rules to miss events.

### Issue Context
- Documentation currently lists four actions, including `cost_access_check`.
- Implementation logs `access_check` for both access endpoints.

### Fix Focus Areas
- workspaces/cost-management/plugins/cost-management-backend/README.md[50-56]

### Suggested fix
Update the README to list only actions that are actually emitted (e.g., `data_access`, `apply_recommendation`, `access_check`), or (if intended) update the backend to emit a distinct `cost_access_check` action for `/access/cost-management` and keep the README as-is.

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



Advisory comments

3. Audit logs are JSON-stringed 🐞 Bug ✧ Quality
Description
The backend README says audit logs are “structured”, but the backend logs them as a JSON string
message (logger.info(JSON.stringify(...))), which many logging setups won’t index as fields. This
can make querying by actor/action/decision unreliable compared to true structured logging.
Code

workspaces/cost-management/plugins/cost-management-backend/README.md[R50-56]

+All backend endpoints emit structured audit log entries with user identity via Backstage's logger. Each entry includes:
+
+- **`actor`** — the authenticated user's entity ref (e.g., `user:default/admin`)
+- **`action`** — the operation performed (`data_access`, `apply_recommendation`, `access_check`, `cost_access_check`)
+- **`decision`** — the RBAC outcome (`ALLOW` or `DENY`)
+- **`resource`** — the upstream API path accessed
+- **`filters`** — the server-injected cluster/project filters (for proxy requests)
Evidence
The docs claim structured entries, but emitAuditLog serializes the object into a string before
logging, so fields may not be captured as structured log attributes depending on logger
transport/configuration.

workspaces/cost-management/plugins/cost-management-backend/README.md[50-56]
workspaces/cost-management/plugins/cost-management-backend/src/util/auditLog.ts[50-52]

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

### Issue description
Docs describe audit logs as structured, but the implementation logs a JSON string. This may reduce field-based searchability in log backends.

### Issue Context
If the goal is true structured logging, prefer passing an object to the logger (so fields become top-level structured attributes) rather than embedding JSON into the message.

### Fix Focus Areas
- workspaces/cost-management/plugins/cost-management-backend/README.md[50-56]
- workspaces/cost-management/plugins/cost-management-backend/src/util/auditLog.ts[50-52]

### Suggested fix
Either:
1) Update README wording to “JSON-formatted audit log message”, or
2) Update `emitAuditLog` to log an object, e.g. `options.logger.info({ audit: true, ...entry }, 'cost-management audit')`, and keep README’s “structured” claim.

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


Grey Divider

ⓘ The new review experience is currently in Beta. Learn more

Grey Divider

Qodo Logo

Comment on lines +18 to +21
- **Input validation**: `resourceType` is validated against a server-side
allowlist; all input fields are sanitized.
- **Audit logging**: Every apply action is logged with user identity, cluster,
namespace, workload, and outcome.

This comment was marked as resolved.

PreetiW added 2 commits April 2, 2026 15:52
…edhat-developer#2616-redhat-developer#2620

- Remove outdated proxy config from workspace README (PR redhat-developer#2616 moved
  data fetching server-side)
- Fix broken link typo in frontend plugin README
- Update backend README: clarify mixed dot/slash permission format,
  add missing endpoints (access, apply-recommendation), add audit
  logging section (PR redhat-developer#2619)
- Clarify permission name format in docs/rbac.md intro (dot for
  plugin-level, slash for cluster/project per PR redhat-developer#2620)
- Update ADR 0002 status to Accepted with implementation notes
  documenting the backend gateway pattern (PRs redhat-developer#2616, redhat-developer#2618, redhat-developer#2619)

Made-with: Cursor
- Fix inaccurate sanitization claim in ADR 0002: changed to
  "validated for presence and type" to match actual implementation
- Remove non-existent `cost_access_check` audit action from backend
  README — the code emits `access_check` for both access endpoints

Made-with: Cursor
@PreetiW
PreetiW force-pushed the docs/update-cost-management-docs-post-security-prs branch from 6fa6431 to 73b90d1 Compare April 2, 2026 10:22
@sonarqubecloud

sonarqubecloud Bot commented Apr 2, 2026

Copy link
Copy Markdown

@pkliczewski pkliczewski left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@PreetiW
PreetiW merged commit 31916b7 into redhat-developer:main Apr 6, 2026
10 checks passed
@hardengl

hardengl commented Apr 6, 2026

Copy link
Copy Markdown
Contributor

/lgtm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants