Skip to content

feat(#3300): agent lifecycle routes with permission integration#3539

Merged
gabemontero merged 2 commits into
mainfrom
agent/3300-agent-lifecycle-routes
Jun 23, 2026
Merged

feat(#3300): agent lifecycle routes with permission integration#3539
gabemontero merged 2 commits into
mainfrom
agent/3300-agent-lifecycle-routes

Conversation

@fullsend-ai-coder

Copy link
Copy Markdown
Contributor

Implement agent CRUD routes with 4-stage lifecycle (Draft → Pending → Published → Archived) and fine-grained permission integration via authorizeLifecycleAction.

Changes:

  • boost-common: Add LifecycleStage type and AgentRecord
    interface for agent governance state
  • boost-backend: Add AgentLifecycleStore (DB-backed) for
    persisting agent governance records
  • boost-backend: Add lifecycle transition validation
    (isValidTransition, isDeletableStage)
  • boost-backend: Add agent routes with permission gating:
    • GET /agents (boost.agent.list)
    • PUT /agents/:id/register (boost.agent.register)
    • PUT /agents/:id/promote (boost.agent.promote)
    • PUT /agents/:id/approve (boost.agent.approve)
    • PUT /agents/:id/request-unpublish (boost.agent.unpublish)
    • PUT /agents/:id/withdraw (boost.agent.withdraw)
    • DELETE /agents/:id (boost.agent.delete)
  • Each route uses authorizeLifecycleAction middleware with
    admin fallback pattern
  • Cascading delete documented: store removes governance
    record; source-specific cleanup is caller responsibility
  • 34 new tests covering lifecycle transitions, route
    handlers, and permission integration

Closes #3300

Post-script verification

  • Branch is not main/master (agent/3300-agent-lifecycle-routes)
  • Secret scan passed (gitleaks — 112eb2235f21cd45946c662381ea473c0fbb59c1..HEAD)
  • Pre-commit hooks passed (authoritative run on runner)
  • Tests ran inside sandbox

Implement agent CRUD routes with 4-stage lifecycle
(Draft → Pending → Published → Archived) and fine-grained
permission integration via authorizeLifecycleAction.

Changes:
- boost-common: Add LifecycleStage type and AgentRecord
  interface for agent governance state
- boost-backend: Add AgentLifecycleStore (DB-backed) for
  persisting agent governance records
- boost-backend: Add lifecycle transition validation
  (isValidTransition, isDeletableStage)
- boost-backend: Add agent routes with permission gating:
  - GET /agents (boost.agent.list)
  - PUT /agents/:id/register (boost.agent.register)
  - PUT /agents/:id/promote (boost.agent.promote)
  - PUT /agents/:id/approve (boost.agent.approve)
  - PUT /agents/:id/request-unpublish (boost.agent.unpublish)
  - PUT /agents/:id/withdraw (boost.agent.withdraw)
  - DELETE /agents/:id (boost.agent.delete)
- Each route uses authorizeLifecycleAction middleware with
  admin fallback pattern
- Cascading delete documented: store removes governance
  record; source-specific cleanup is caller responsibility
- 34 new tests covering lifecycle transitions, route
  handlers, and permission integration

Closes #3300
@fullsend-ai-coder fullsend-ai-coder Bot requested review from a team, durandom and gabemontero as code owners June 23, 2026 01:55
@rhdh-gh-app

rhdh-gh-app Bot commented Jun 23, 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/backstage-plugin-boost-backend
  • @red-hat-developer-hub/backstage-plugin-boost-common

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/backstage-plugin-boost-backend workspaces/boost/plugins/boost-backend none v0.1.1
@red-hat-developer-hub/backstage-plugin-boost-common workspaces/boost/plugins/boost-common none v0.1.1

@codecov

codecov Bot commented Jun 23, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 58.85167% with 86 lines in your changes missing coverage. Please review.
✅ Project coverage is 53.72%. Comparing base (112eb22) to head (0071393).
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3539      +/-   ##
==========================================
+ Coverage   53.71%   53.72%   +0.01%     
==========================================
  Files        2264     2267       +3     
  Lines       86227    86435     +208     
  Branches    24233    24271      +38     
==========================================
+ Hits        46319    46441     +122     
- Misses      39609    39699      +90     
+ Partials      299      295       -4     
Flag Coverage Δ *Carryforward flag
adoption-insights 83.70% <ø> (ø) Carriedforward from 112eb22
ai-integrations 67.95% <ø> (ø) Carriedforward from 112eb22
app-defaults 69.79% <ø> (ø) Carriedforward from 112eb22
augment 46.39% <ø> (ø) Carriedforward from 112eb22
boost 71.71% <58.85%> (-6.92%) ⬇️
bulk-import 72.46% <ø> (ø) Carriedforward from 112eb22
cost-management 14.10% <ø> (ø) Carriedforward from 112eb22
dcm 61.79% <ø> (ø) Carriedforward from 112eb22
extensions 61.53% <ø> (ø) Carriedforward from 112eb22
global-floating-action-button 71.18% <ø> (ø) Carriedforward from 112eb22
global-header 59.71% <ø> (ø) Carriedforward from 112eb22
homepage 49.84% <ø> (ø) Carriedforward from 112eb22
install-dynamic-plugins 56.23% <ø> (ø) Carriedforward from 112eb22
konflux 91.49% <ø> (ø) Carriedforward from 112eb22
lightspeed 68.57% <ø> (ø) Carriedforward from 112eb22
mcp-integrations 85.46% <ø> (ø) Carriedforward from 112eb22
orchestrator 37.79% <ø> (ø) Carriedforward from 112eb22
quickstart 63.76% <ø> (ø) Carriedforward from 112eb22
sandbox 79.56% <ø> (ø) Carriedforward from 112eb22
scorecard 83.96% <ø> (ø) Carriedforward from 112eb22
theme 61.26% <ø> (ø) Carriedforward from 112eb22
translations 7.25% <ø> (ø) Carriedforward from 112eb22
x2a 78.68% <ø> (ø) Carriedforward from 112eb22

*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 112eb22...0071393. 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.

@fullsend-ai-review

fullsend-ai-review Bot commented Jun 23, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 1:57 AM UTC · Completed 2:09 AM UTC
Commit: 112eb22 · View workflow run →

@fullsend-ai-review

fullsend-ai-review Bot commented Jun 23, 2026

Copy link
Copy Markdown

Review

Findings

Medium

  • [api-contract] workspaces/boost/plugins/boost-backend/src/middleware/security.ts:164 — When permissions.authorize() is called with a resource-scoped permission, the Backstage permission framework can return AuthorizeResult.CONDITIONAL instead of ALLOW or DENY. The current code only checks for ALLOW and treats everything else (including CONDITIONAL) as a denial that falls through to the admin check. This means any permission policy returning conditional rules for these resource-scoped permissions will be silently ignored. The PR documents this as deferred ("conditional authorization is deferred to a later issue"), but the gap should be explicitly handled.
    Remediation: Either handle AuthorizeResult.CONDITIONAL by evaluating conditions against the loaded resource, or add an explicit check for CONDITIONAL and log a warning to make the behavior visible.

  • [race-condition] workspaces/boost/plugins/boost-backend/src/agents/routes.ts:220 — All lifecycle-transition routes perform a read-then-check-then-write pattern without transactional or optimistic-concurrency guards. The updateStage method has no WHERE clause on the current stage, so concurrent requests could cause invalid transitions (e.g., two simultaneous promotes on the same draft agent, or a promote racing with a delete).
    Remediation: Add a WHERE lifecycle_stage = :expectedStage condition to the updateStage SQL query. If zero rows are updated, return a 409 Conflict.

  • [stale-reference] workspaces/boost/plugins/boost-backend/src/middleware/security.ts:206 — The existing createAgentResourceLoader() function is now dead code — a new createStoreAgentResourceLoader is defined in routes.ts that actually loads from the store. The old function is still exported publicly from src/index.ts and could mislead future developers.
    Remediation: Remove the dead createAgentResourceLoader function or update it to delegate to the store-backed implementation.

Low

  • [privilege-escalation] workspaces/boost/plugins/boost-backend/src/agents/routes.ts:289 — The approve route has no ownership check to enforce separation of duties (IS_NOT_CREATOR). Any user with boost.agent.approve or boost.admin can approve their own agents. This aligns with the deferred conditional authorization work.

  • [auth-policy] workspaces/boost/plugins/boost-backend/src/plugin.ts:249 — The auth policy { path: '/agents', allow: 'user-cookie' } follows the existing plugin pattern (same as /config/status). Verify this is the intended auth level for all state-mutating agent operations.

  • [auth-policy-misconfiguration] workspaces/boost/plugins/boost-backend/src/plugin.ts:252 — Verify that Backstage's httpRouter.addAuthPolicy treats /agents as a prefix match covering sub-paths like /agents/:id/promote.

  • [error-handling] workspaces/boost/plugins/boost-backend/src/agents/AgentLifecycleStore.ts:166 — The register method uses record! (non-null assertion) after re-reading a just-inserted row. Consider returning the record directly from the insert or handling the edge case gracefully.

  • [input-validation] workspaces/boost/plugins/boost-backend/src/agents/routes.ts:298 — The register route has no length validation on name or description fields. Add explicit limits (e.g., name max 255, description max 4096).

  • [test-adequacy] workspaces/boost/plugins/boost-backend/src/agents/routes.test.ts:564 — Missing tests for path traversal in agent IDs, excessively long IDs, concurrent requests, and store.delete returning false after passing the get check.

  • [edge-case] workspaces/boost/plugins/boost-backend/src/agents/routes.ts:258 — The validateAgentId regex allows single-character IDs (^[a-zA-Z0-9][a-zA-Z0-9._-]{0,254}$). Consider whether this is intentional.

Info

  • [sub-agent-failure] N/A — The style-conventions, intent-coherence, and cross-repo-contracts sub-agents did not return findings: model claude-sonnet-4-5@20250929 is not available on the deployment. These dimensions were not evaluated.
Previous run

Review

Findings

Medium

  • [api-contract] workspaces/boost/plugins/boost-backend/src/agents/routes.ts:69 — Resource-scoped permissions (boostAgentPromotePermission, boostAgentApprovePermission, boostAgentUnpublishPermission, boostAgentWithdrawPermission, boostAgentDeletePermission) are passed to authorizeLifecycleAction which declares BasicPermission. At runtime, calling permissions.authorize() with a resource-scoped permission without a resourceRef means the Backstage permission framework cannot apply resource-conditional policies. Fine-grained checks will always fail; only the admin fallback grants access. The JSDoc notes this is intentional for now, but the route comments claim IS_OWNER/IS_NOT_CREATOR enforcement that is not implemented. See also: [auth-bypass] finding on the same middleware.
    Remediation: Update the parameter type from BasicPermission to Permission (the union type) and add a code comment documenting that resource-conditional checks are deferred. Remove or qualify the route comments claiming IS_OWNER/IS_NOT_CREATOR enforcement.

  • [auth-bypass] workspaces/boost/plugins/boost-backend/src/middleware/security.ts:153 — The authorizeLifecycleAction middleware does not call _resourceLoader and does not pass resourceRef to permissions.authorize(). Resource-conditional policies (IS_OWNER, IS_NOT_CREATOR, HAS_LIFECYCLE_STAGE) are not enforced. Any authenticated user with admin access can perform lifecycle actions on any agent regardless of ownership. The JSDoc documents this as deferred, but the PR description claims "fine-grained permission integration" which overstates what is delivered. See also: [api-contract] finding on the routes.
    Remediation: Document this as a known limitation in the PR description. When resource-conditional authorization is implemented, call _resourceLoader(req) to obtain resource metadata and pass it to permissions.authorizeConditional().

  • [fail-open] workspaces/boost/plugins/boost-backend/src/agents/routes.ts:109 — When a valid userEntityRef cannot be extracted from credentials (e.g., service-to-service credentials or unexpected credential shapes), the code silently falls back to 'user:default/unknown' as the createdBy value. Agents registered this way are effectively ownerless — no user can match IS_OWNER checks once those are enforced, and multiple agents could share the same fake identity, undermining audit trails.
    Remediation: Throw an AuthenticationError if a valid userEntityRef cannot be extracted from the credentials.

  • [error-handling-gap] workspaces/boost/plugins/boost-backend/src/agents/routes.ts:156 — The return value of store.updateStage(id, stage) is not checked for undefined. The updateStage method returns undefined if the agent is not found (e.g., deleted between the get check and the updateStage call). Routes at lines 156, 185, 219, and 249 pass the result directly to res.json(updated) without checking, which would produce an empty HTTP 200 response.
    Remediation: After each store.updateStage() call, check if the result is undefined and throw a NotFoundError.

Low

  • [race-condition] workspaces/boost/plugins/boost-backend/src/agents/routes.ts:144 — TOCTOU gap between the store.get(id) check and the store.updateStage() call in lifecycle transition routes. Currently low-impact since the _resourceLoader is unused, but will become relevant when resource-conditional authorization is implemented.

  • [error-handling-gap] workspaces/boost/plugins/boost-backend/src/agents/AgentLifecycleStore.ts:191 — Non-null assertion record! after re-reading a just-inserted agent. While practically safe in a single-node deployment, an explicit null check would be more defensive.

  • [test-inadequate] workspaces/boost/plugins/boost-backend/src/agents/routes.test.ts:462 — The permission integration test validates the current limited behavior (fine-grained always fails due to type mismatch) rather than testing proper fine-grained authorization. Also missing: test for updateStage returning undefined.

  • [input-validation] workspaces/boost/plugins/boost-backend/src/agents/routes.ts:94 — The id parameter from req.params is used directly without validation (length limits, character whitelist). While Knex prevents SQL injection, crafted IDs could cause storage or logging issues.

  • [edge-case] workspaces/boost/plugins/boost-backend/src/agents/AgentLifecycleStore.ts:178 — Concurrent duplicate registrations would produce a 500 (primary key violation) instead of a user-friendly 409 Conflict.

Info

  • [sub-agent-failure] The intent-coherence, style-conventions, and cross-repo-contracts sub-agents could not be dispatched due to model unavailability. These dimensions were not reviewed. The cross-repo changes are purely additive (new types and exports, no breaking changes to existing interfaces).

@fullsend-ai-review fullsend-ai-review Bot 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.

See the review comment for full details.

Comment thread workspaces/boost/plugins/boost-backend/src/agents/routes.ts
Comment thread workspaces/boost/plugins/boost-backend/src/agents/routes.ts
Comment thread workspaces/boost/plugins/boost-backend/src/agents/routes.ts
Comment thread workspaces/boost/plugins/boost-backend/src/agents/routes.ts
Comment thread workspaces/boost/plugins/boost-backend/src/agents/routes.test.ts
Comment thread workspaces/boost/plugins/boost-backend/src/agents/routes.ts
Comment thread workspaces/boost/plugins/boost-backend/src/agents/AgentLifecycleStore.ts Outdated
- Widen authorizeLifecycleAction parameter from BasicPermission to
  Permission to support resource-scoped permissions (promote, approve,
  unpublish, withdraw, delete)
- Add resource ref extraction from req.params.id for resource-scoped
  permission checks in the authorization middleware
- Add agent ID validation (AGENT_ID_PATTERN) on all :id routes to
  prevent path traversal and invalid identifiers
- Change duplicate agent registration from InputError to ConflictError
  (409) with DB-level PK violation handling in AgentLifecycleStore
- Reject registration when user identity cannot be resolved instead of
  falling back to user:default/unknown
- Check updateStage() return value in all transition routes to detect
  concurrent deletion during transitions
- Add TSDoc to public interface members to fix API report warnings
- Fix unresolved @link reference in AgentLifecycleStore
- Add tests for invalid agent ID, missing user identity, and concurrent
  deletion during transition
- Regenerate API reports

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: gabemontero <gmontero@redhat.com>
@fullsend-ai-review

fullsend-ai-review Bot commented Jun 23, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 2:58 AM UTC · Completed 3:09 AM UTC
Commit: 112eb22 · View workflow run →

@fullsend-ai-review fullsend-ai-review Bot 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.

See the review comment for full details.

Comment thread workspaces/boost/plugins/boost-backend/src/middleware/security.ts
Comment thread workspaces/boost/plugins/boost-backend/src/agents/routes.ts
Comment thread workspaces/boost/plugins/boost-backend/src/agents/routes.ts
Comment thread workspaces/boost/plugins/boost-backend/src/agents/routes.ts
Comment thread workspaces/boost/plugins/boost-backend/src/agents/routes.test.ts
@fullsend-ai-review fullsend-ai-review Bot added the requires-manual-review Review requires human judgment label Jun 23, 2026
@gabemontero gabemontero merged commit d197bd6 into main Jun 23, 2026
65 of 68 checks passed
@gabemontero gabemontero deleted the agent/3300-agent-lifecycle-routes branch June 23, 2026 03:22
@sonarqubecloud

Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed

Failed conditions
6.2% Duplication on New Code (required ≤ 3%)

See analysis details on SonarQube Cloud

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

Labels

requires-manual-review Review requires human judgment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

boost-backend — Agent lifecycle routes with permission integration (issue 4 of 15)

1 participant