Skip to content

feat: add support_copilot app type with OAuth 2.1 JWT auth#3128

Merged
omar-inkeep merged 2 commits intomainfrom
feat/support-copilot-app
Apr 14, 2026
Merged

feat: add support_copilot app type with OAuth 2.1 JWT auth#3128
omar-inkeep merged 2 commits intomainfrom
feat/support-copilot-app

Conversation

@omar-inkeep
Copy link
Copy Markdown
Contributor

Summary

Adds a new support_copilot app type for deploying agents across external tools and support platforms (browser extensions, helpdesks, etc.). End-users authenticate via the existing OAuth 2.1 provider (#3103) using asymmetric JWTs verified via JWKS.

What's included

Backend (agents-api, agents-core):

  • New SupportCopilotConfig schema with credentialReferenceIds
  • tryOAuthSupportCopilotAuth() — JWKS-verified JWT flow with azp, tenant-match, and SpiceDB canUseProjectStrict checks
  • OAuth JWT path added to manageAuth at priority done #3 (JWT-only — opaque tokens are not issued by this flow)
  • GET /manage/tenants/:tenantId/apps — cross-project app discovery (used by the copilot BFF to resolve its app_id per tenant)
  • COPILOT_OAUTH_CLIENT_ID env var for azp validation

UI (agents-manage-ui):

  • "Support Copilot" option in the New App dropdown (double-gated: PUBLIC_IS_INKEEP_CLOUD_DEPLOYMENT=true AND org has feature:support_copilot entitlement with maxValue > 0)
  • CredentialMultiSelect component for picking credential references
  • Default agent + credential pickers in create/update forms
  • Orange badge for support_copilot in the apps table

Tooling:

  • scripts/setup-oauth-client.ts — creates the OAuth client for the copilot BFF
  • --print-only flag for prod bootstrap (no .env write, values go to stdout for copy into secret manager)

Gating (per-org opt-in on cloud only)

The Support Copilot option requires both:

  1. PUBLIC_IS_INKEEP_CLOUD_DEPLOYMENT=true (deployment-level flag)
  2. A row in org_entitlement with resource_type = 'feature:support_copilot' and max_value > 0 for that org

To enable for a customer:

INSERT INTO org_entitlement (id, organization_id, resource_type, max_value)
VALUES ('<uuid>', '<tenant-id>', 'feature:support_copilot', 1);

Production setup

  1. pnpm setup-oauth-client --print-only (pointed at prod DB) → copy COPILOT_OAUTH_CLIENT_ID + COPILOT_OAUTH_CLIENT_SECRET into secret manager
  2. Set COPILOT_OAUTH_CLIENT_ID on the agents-api deployment (validates azp on incoming JWTs)
  3. Set both COPILOT_OAUTH_CLIENT_ID and COPILOT_OAUTH_CLIENT_SECRET on the copilot BFF deployment (needs the secret to exchange auth codes)
  4. Insert the feature:support_copilot entitlement row for any cloud tenant that should see the dropdown option

Test plan

  • Full agents-api test suite passes (2362 passed, 15 new)
  • Typecheck, lint, format clean across monorepo
  • Create support_copilot app via UI in a cloud-flagged, entitled tenant
  • Edit existing support_copilot app (credentials + default agent persist)
  • Verify option is hidden when deployment flag or entitlement is missing
  • Verify GET /manage/tenants/:tenantId/apps?type=support_copilot returns cross-project results
  • Manual prod bootstrap: pnpm setup-oauth-client --print-only against prod DB
  • Chrome extension → copilot BFF → agents-api end-to-end with real OAuth flow

Known gaps / follow-ups

  • No API-side entitlement gate. Intentional — the OAuth auth path silently rejects any JWT when COPILOT_OAUTH_CLIENT_ID is unset, so an OSS or un-entitled tenant can't actually use a support_copilot app even if they create one via curl. Consider adding an API gate if we want stricter defense-in-depth.
  • No self-service OAuth client management UI. Clients are created via the bootstrap script today. Future: a "Developer apps" page in the manage UI.
  • No customer-facing docs. Skipped per product call — this is cloud-only and ops-provisioned for now.

🤖 Generated with Claude Code

Adds a new support_copilot app type for deploying agents across external
tools and support platforms (e.g. browser extensions, helpdesks). The
app authenticates end-users via the existing OAuth 2.1 provider using
asymmetric JWTs validated via JWKS.

Backend (agents-api, agents-core):
- New SupportCopilotConfig schema with credentialReferenceIds
- tryOAuthSupportCopilotAuth: JWKS-verified JWT flow with azp, tenant,
  and SpiceDB canUseProjectStrict checks
- OAuth JWT path added to manageAuth (priority #3, JWT-only — opaque
  tokens are not issued by this flow)
- GET /manage/tenants/:tenantId/apps for cross-project app discovery
  (used by the copilot BFF to resolve its app_id per tenant)
- COPILOT_OAUTH_CLIENT_ID env var for azp validation

UI (agents-manage-ui):
- Support Copilot option in New App dropdown (double-gated:
  PUBLIC_IS_INKEEP_CLOUD_DEPLOYMENT=true AND org has
  feature:support_copilot entitlement with maxValue > 0)
- CredentialMultiSelect component for picking credential references
- Default agent and credential pickers in create/update forms
- Orange badge for support_copilot in the apps table

Tooling:
- scripts/setup-oauth-client.ts — creates the OAuth client for the
  copilot BFF. Supports --print-only for safe prod bootstrap
  (no .env write).

Tests: 15 new unit tests (tryOAuthSupportCopilotAuth, manage OAuth JWT
path, tenant apps route). Full agents-api suite: 2362 passed.

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
@vercel
Copy link
Copy Markdown

vercel Bot commented Apr 14, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
agents-api Ready Ready Preview, Comment Apr 14, 2026 5:19pm
agents-manage-ui Ready Ready Preview, Comment Apr 14, 2026 5:19pm
1 Skipped Deployment
Project Deployment Actions Updated (UTC)
agents-docs Skipped Skipped Apr 14, 2026 5:19pm

Request Review

@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented Apr 14, 2026

🦋 Changeset detected

Latest commit: 4f3a044

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 10 packages
Name Type
@inkeep/agents-core Patch
@inkeep/agents-api Patch
@inkeep/agents-manage-ui Patch
@inkeep/agents-cli Patch
@inkeep/agents-sdk Patch
@inkeep/agents-work-apps Patch
@inkeep/ai-sdk-provider Patch
@inkeep/create-agents Patch
@inkeep/agents-email Patch
@inkeep/agents-mcp Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 14, 2026

This public PR was merged directly in the public repo. The matching monorepo PR was left open for manual follow-up because agents-private remains the source of truth.

Matching internal PR: #89

@socket-security
Copy link
Copy Markdown

socket-security Bot commented Apr 14, 2026

@pullfrog
Copy link
Copy Markdown
Contributor

pullfrog Bot commented Apr 14, 2026

TL;DR

This PR introduces a new support_copilot app type that authenticates via OAuth 2.1 JWT tokens issued by the platform's own better-auth OIDC provider. It adds JWT verification to both manage and run auth middleware, a new tenant-level app discovery endpoint (GET /manage/tenants/:tenantId/apps), full CRUD UI support for creating/updating support copilot apps with credential associations, and a setup script for provisioning the OAuth client. The feature is gated behind a feature:support_copilot entitlement and cloud-deployment flag.

Key changes

  • New app type support_copilot added to the AppType discriminated union, validation schemas, and OpenAPI spec alongside web_client and api.
  • OAuth 2.1 JWT auth in manage middleware — a new step (priority 3) verifies JWTs against the platform's own JWKS endpoint using jose, extracting sub, tenantId, and email from claims before falling through to Slack/internal token auth.
  • OAuth JWT auth in run middleware — new tryOAuthSupportCopilotAuth() function validates JWTs for support_copilot apps, enforces azp client-ID matching, tenant-ID consistency, and SpiceDB project-level authorization.
  • Tenant-level apps listing endpoint (GET /manage/tenants/:tenantId/apps) for cross-project app discovery, with optional type filter.
  • Manage UI now supports creating/editing support_copilot apps with a new CredentialMultiSelect component for associating credentials, gated behind entitlements + cloud deployment check.
  • Setup script (pnpm setup-oauth-client) provisions an OAuth 2.1 client via better-auth admin API and persists credentials to .env.
  • New env variable COPILOT_OAUTH_CLIENT_ID for validating the azp claim on incoming OAuth JWTs.
  • Comprehensive tests for OAuth JWT flows in both manage and run auth middleware, plus integration tests for the tenant apps endpoint.

OAuth JWT authentication added to manage and run middleware

Before: The manage auth middleware supported bypass secret, better-auth sessions, Slack user JWTs, internal service tokens, and a legacy API key exception. The run auth middleware had no support_copilot app type handling.

After: Both middleware stacks now verify OAuth 2.1 JWT tokens against the platform's own JWKS endpoint (/api/auth/jwks) using jose.createRemoteJWKSet with getInProcessFetch() to keep the request in-process. In manageAuth.ts, JWT verification is inserted at priority 3 (after session auth, before Slack tokens). It checks the azp claim against COPILOT_OAUTH_CLIENT_ID, extracts sub/tenantId/email from namespaced claims, and sets them on the Hono context. In runAuth.ts, a new tryOAuthSupportCopilotAuth() function handles the support_copilot app type branch — it performs JWT verification, azp validation, tenant-ID cross-check against the app record, and SpiceDB canUseProjectStrict permission check before granting access with the app_credential_support_copilot auth method.

New support_copilot app type in schema and validation

Before: AppType was 'web_client' | 'api' and AppConfigSchema was a discriminated union of those two.

After: AppType includes 'support_copilot'. A new SupportCopilotConfigSchema defines the config shape with type: 'support_copilot' and a supportCopilot.credentialReferenceIds array. This is added to both AppConfigSchema and AppConfigResponseSchema discriminated unions, and the AppInsertSchema type enum. The BaseExecutionContext.authMethod union gains 'app_credential_support_copilot'. The OpenAPI snapshot reflects all these additions.

Tenant-level app discovery endpoint

Before: Apps could only be listed within a specific project scope (/manage/tenants/:tenantId/projects/:projectId/apps).

After: A new GET /manage/tenants/:tenantId/apps endpoint lists apps across all projects for a tenant, with optional type filter and pagination. This is designed for cross-project discovery scenarios like finding all support_copilot apps. It uses inheritedManageTenantAuth() for authorization. Integration tests verify cross-project listing, type filtering, and empty tenant handling.

Manage UI: support copilot app creation and credential association

Before: The app creation dialog offered web_client and api types. No credential selection existed.

After: A third Support Copilot option appears in the new-app dropdown, conditionally shown only when the tenant has a feature:support_copilot entitlement AND the deployment is Inkeep Cloud. A new CredentialMultiSelect component (combobox with badges, search, and removal) lets users associate stored credentials with support copilot apps. Both create and update forms pass credentialReferenceIds through the supportCopilot config object. The apps table shows an orange "Support Copilot" badge for the new type. The apps page now fetches credentials and entitlements in parallel alongside apps and agents.

OAuth client setup script

A new scripts/setup-oauth-client.ts (run via pnpm setup-oauth-client) provisions an OAuth 2.1 client through the better-auth admin API. It creates a client with redirect URIs for both local dev (localhost:3100) and production (copilot.inkeep.com), with skip_consent: true and enable_end_session: true. The script is idempotent — it checks .env for existing credentials before creating a new client. A --print-only flag outputs credentials to stdout without writing to .env.

Test coverage

New test files cover: (1) OAuth JWT validation in manage auth — valid tokens, azp mismatch rejection, non-JWT fallthrough, signature failure fallthrough, and missing sub fallthrough. (2) support_copilot app credential auth in run middleware — successful auth with SpiceDB permission checks, JWT verification failures, azp mismatch, tenant mismatch (403), insufficient project permissions (403), SpiceDB unavailability (503), and missing sub. (3) Tenant apps endpoint integration tests — cross-project listing, type filtering, and empty tenant. Existing test mocks were updated to include createRemoteJWKSet, customFetch, and getInProcessFetch stubs required by the new jose JWKS initialization at module scope.

Pullfrog  | View workflow run | Triggered by Pullfrog | Using Claude Opus𝕏

Copy link
Copy Markdown
Contributor

@pullfrog pullfrog Bot left a comment

Choose a reason for hiding this comment

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

Good feature PR with solid test coverage and well-structured auth. Two items need attention: a security nuance in the manage auth JWT path, and a stale type cast. The rest is clean.

Security note (pre-existing code touched by this PR): The manageBearerAuth OAuth JWT path (step 3, manageAuth.ts:132-155) verifies JWTs via JWKS but only enforces azp when COPILOT_OAUTH_CLIENT_ID is set. When the env var is unset (and the JWKS endpoint is active), any validly-signed JWT with a sub claim passes manage auth. The PR description says "silently rejects any JWT when COPILOT_OAUTH_CLIENT_ID is unset", but the code actually accepts such JWTs (it only rejects when azp is present and mismatches). Consider whether that's desired — a JWT from a future OAuth client could inadvertently gain manage-domain access.

Minor note (outside diff): apps.ts:70 has c.req.query('type') as 'web_client' | 'api' | undefined — the cast misses 'support_copilot' even though the query schema now includes it. Not a runtime bug (string passes through), but the type is stale.

Pullfrog  | Fix all ➔Fix 👍s ➔View workflow run | Using Claude Opus𝕏

Comment thread agents-api/src/middleware/manageAuth.ts Outdated
try {
const { payload } = await jwtVerify(token, JWKS);
const azp = payload.azp as string | undefined;
if (env.COPILOT_OAUTH_CLIENT_ID && azp !== env.COPILOT_OAUTH_CLIENT_ID) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

When COPILOT_OAUTH_CLIENT_ID is unset, this condition is falsy and the JWT passes through unconditionally (as long as it has a sub). This means any JWKS-verified JWT — including ones minted for future OAuth clients — gets manage access.

For defense-in-depth, consider skipping the OAuth JWT path entirely when the env var is unset:

if (!env.COPILOT_OAUTH_CLIENT_ID) {
  // No copilot client configured — skip OAuth JWT path
} else {
  // existing JWKS verify + azp check
}

Comment thread agents-api/src/middleware/manageAuth.ts Outdated
c.set('userId', sub);
if (email) c.set('userEmail', email);
if (tenantId) c.set('tenantId', tenantId);
if (azp) c.set('oauthClientId' as any, azp);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

c.set('oauthClientId' as any, azp) — the as any cast bypasses the Variables type. If oauthClientId is needed by downstream middleware, add it to the Variables type. If it's only for debugging, consider logging it instead.

Comment thread agents-api/src/middleware/runAuth.ts Outdated
throw new HTTPException(401, { message: 'Invalid OAuth client' });
}

if (app.tenantId && jwtTenantId && app.tenantId !== jwtTenantId) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The tenant-mismatch check is skipped when app.tenantId is falsy (null/empty for a global app). Should a support_copilot app ever be global? If not, consider adding an explicit guard: if (!app.tenantId) throw ... to enforce that support_copilot apps are always tenant-scoped.

apiKey: bearerToken,
tenantId: app.tenantId || '',
projectId: app.projectId,
agentId: requestedAgentId || app.defaultAgentId || '',
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Falling back to '' for agentId when both requestedAgentId and app.defaultAgentId are absent will produce an execution context with an empty string agent ID. Downstream code likely won't handle this well. Consider rejecting early with a descriptive error.

fetchApps(tenantId, projectId),
fetchAgents(tenantId, projectId),
fetchCredentials(tenantId, projectId),
fetchEntitlements(tenantId),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If fetchEntitlements(tenantId) throws on self-hosted deployments where the entitlements endpoint doesn't exist, the entire apps page shows an error. Consider wrapping with .catch(() => []) since entitlements are only meaningful on cloud.

const { tenantId } = c.req.valid('param');
const page = Number(c.req.query('page')) || 1;
const limit = Math.min(Number(c.req.query('limit')) || 10, 100);
const type = c.req.query('type') as 'web_client' | 'api' | 'support_copilot' | undefined;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Using c.req.query('type') as ... bypasses the Zod validation already declared in the route schema. Consider using c.req.valid('query') instead so the value is type-safe from the schema.

Copy link
Copy Markdown
Contributor

@claude claude Bot left a comment

Choose a reason for hiding this comment

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

PR Review Summary

(6) Total Issues | Risk: Medium

🟠⚠️ Major (2) 🟠⚠️

🟠 1) manageAuth.ts:136 + runAuth.ts:448 OAuth JWT azp validation bypassed when COPILOT_OAUTH_CLIENT_ID is unset

Issue: When env.COPILOT_OAUTH_CLIENT_ID is undefined (not configured), the azp validation check is skipped entirely in both manageAuth.ts and runAuth.ts. The condition env.COPILOT_OAUTH_CLIENT_ID && azp !== env.COPILOT_OAUTH_CLIENT_ID evaluates to false when the env var is unset, meaning any valid JWT signed by the JWKS endpoint (regardless of its azp claim) will be accepted.

Why: The PR description states "the OAuth auth path silently rejects any JWT when COPILOT_OAUTH_CLIENT_ID is unset" as a security feature, but the actual implementation does the opposite — it silently ACCEPTS. This could allow tokens issued to other OAuth clients to authenticate, creating a confused deputy vulnerability in non-production environments or during misconfiguration.

Fix: If the intent is to disable OAuth JWT auth when unconfigured, add an early return before JWT verification:

// In manageAuth.ts, before line 132:
if (!env.COPILOT_OAUTH_CLIENT_ID) {
  // OAuth JWT auth disabled - fall through to other auth methods
}

// Or make azp validation mandatory when configured:
if (env.COPILOT_OAUTH_CLIENT_ID && azp !== env.COPILOT_OAUTH_CLIENT_ID) {
  throw new HTTPException(401, { message: 'Invalid OAuth client' });
}
if (!env.COPILOT_OAUTH_CLIENT_ID && azp) {
  // JWT has azp but we have no expected client - reject
  throw new HTTPException(401, { message: 'OAuth not configured' });
}

Apply the same pattern in tryOAuthSupportCopilotAuth in runAuth.ts.

Refs:


🟠 2) app-credential-auth.test.ts Missing test for COPILOT_OAUTH_CLIENT_ID unset scenario

Issue: There is no test verifying the behavior when COPILOT_OAUTH_CLIENT_ID env var is not configured. This is the critical security boundary behavior documented in the PR description.

Why: Without this test, a refactor could accidentally change the conditional logic. Given the discrepancy between documented intent ("silently rejects") and actual behavior ("silently accepts"), explicit test coverage is essential to lock in the intended behavior.

Fix: Add test case:

it('should reject OAuth JWT when COPILOT_OAUTH_CLIENT_ID is not configured', async () => {
  // Mock env.COPILOT_OAUTH_CLIENT_ID = undefined
  // Provide valid JWT with any azp
  // Verify 401 response (if intent is to reject)
  // OR document and test the accept behavior if intentional
});

Refs:

🟡 Minor (3) 🟡

🟡 1) tenantApps.ts:34 Missing commonGetErrorResponses in route definition

Issue: All peer list routes in the manage domain include ...commonGetErrorResponses in their responses object (e.g., apps.ts, credentials.ts, apiKeys.ts, entitlements.ts). This route omits it.

Why: Ensures consistent error response shapes in the OpenAPI spec across all list endpoints.

Fix: Add ...commonGetErrorResponses to the responses object and import from @inkeep/agents-core.

Refs:


🟡 2) manageAuth.ts:20 + runAuth.ts:38 JWKS singleton duplication

Issue: The JWKS object is created at module load time in both manageAuth.ts and runAuth.ts with identical configuration.

Why: This violates DRY and creates two separate caches that refresh independently. Additionally, module initialization now depends on env.INKEEP_AGENTS_API_URL being set at import time.

Fix: Extract JWKS initialization to a shared utility in agents-core (e.g., src/auth/jwks.ts) that exports a lazy-initialized singleton.

Refs:


🟡 3) runAuth.ts:453 Tenant mismatch check allows missing tenantId to bypass validation

Issue: The condition if (app.tenantId && jwtTenantId && app.tenantId !== jwtTenantId) only triggers when BOTH values are truthy. If the JWT is missing the tenantId claim, the check is skipped.

Why: While canUseProjectStrict later validates project access using the app's tenantId, allowing JWTs without explicit tenant claims may be inconsistent with the security model.

Fix: Consider making the tenantId claim required:

if (!jwtTenantId) {
  return { authResult: null, failureMessage: 'OAuth JWT missing tenantId claim' };
}

Refs:

Inline Comments:

  • 🟡 Minor: apps.ts:70 Type cast missing support_copilot
  • 🟡 Minor: credential-multi-select.tsx:86 Decorative icon missing aria-hidden
  • 🟡 Minor: credential-multi-select.tsx:103 Selection icon missing aria-hidden
  • 🟡 Minor: credential-multi-select.tsx:129 Remove button icon missing aria-hidden

💭 Consider (1) 💭

💭 1) manageAuth.ts:134 + runAuth.ts:430 OAuth JWT verification lacks issuer (iss) claim validation

Issue: The jwtVerify calls do not pass an issuer option, meaning JWTs from the JWKS will be accepted regardless of what iss claim they contain.

Why: While the JWKS lookup constrains which keys are trusted, explicit issuer validation is a defense-in-depth measure. The anonymous JWT flow at runAuth.ts:821 does validate issuer: { issuer: 'inkeep' }.

Fix: Add issuer validation for consistency:

await jwtVerify(token, JWKS, { issuer: env.INKEEP_AGENTS_API_URL || 'http://localhost:3002' })

Refs:


🚫 REQUEST CHANGES

Summary: The PR implements a well-structured new app type with appropriate SpiceDB authorization and UI gating. However, there's a critical discrepancy between the documented security behavior ("OAuth auth path silently rejects any JWT when COPILOT_OAUTH_CLIENT_ID is unset") and the actual implementation (which accepts any valid JWKS-signed JWT when unconfigured). This needs to be aligned — either fix the code to match the documentation, or update the documentation if the current behavior is intentional. Once this is clarified and the azp validation behavior is locked in with tests, this should be good to merge.

Discarded (3)
Location Issue Reason Discarded
runAuth.ts:849-852 api app type unhandled in tryAppCredentialAuth Pre-existing behavior, not introduced by this PR. API apps use different auth path.
schemas.ts:1901 credentialReferenceIds has no referential integrity validation Consistent with existing patterns (e.g., defaultAgentId). Schema validates structure, not references.
setup-oauth-client.ts:121 Client secret printed to stdout in --print-only mode Documented behavior for ops bootstrapping. Script includes appropriate warning.
Reviewers (7)
Reviewer Returned Main Findings Consider While You're Here Inline Comments Pending Recs Discarded
pr-review-security-iam 5 2 1 0 0 0 0
pr-review-architecture 6 1 0 0 0 0 2
pr-review-tests 6 1 0 0 0 0 0
pr-review-frontend 4 0 0 0 4 0 0
pr-review-consistency 2 1 0 0 1 0 0
pr-review-standards 2 0 0 0 0 0 0
pr-review-appsec 3 0 0 0 0 0 1
Total 28 5 1 0 5 0 3

Note: Security-iam and standards both flagged the azp validation issue — consolidated as single Major finding. Architecture flagged JWKS duplication and tenant-level auth boundary. Tests flagged missing coverage for unconfigured env var scenario.

) : (
<span className="text-muted-foreground">{placeholder}</span>
)}
<ChevronsUpDown className="opacity-50 text-muted-foreground" />
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Minor: Decorative icon missing aria-hidden

Issue: The ChevronsUpDown icon is decorative but missing aria-hidden="true".

Why: Screen readers may announce the icon unnecessarily when reading the button.

Fix:

Suggested change
<ChevronsUpDown className="opacity-50 text-muted-foreground" />
<ChevronsUpDown className="opacity-50 text-muted-foreground" aria-hidden="true" />

onSelect={() => toggleValue(option.value)}
>
{option.label}
<Check
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Minor: Selection indicator icon missing aria-hidden

Issue: The Check icon indicating selection state should be hidden from assistive technology.

Why: The CommandItem component already handles selection state semantics internally; the visual checkmark is decorative.

Fix:

Suggested change
<Check
<Check
aria-hidden="true"
className={cn(

onClick={() => removeValue(id)}
aria-label={`Remove ${opt?.label ?? id}`}
>
<X className="size-3" />
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Minor: Remove button icon missing aria-hidden

Issue: The X icon inside the remove button is decorative and should have aria-hidden="true".

Why: The button already has an accessible aria-label so the icon is purely visual.

Fix:

Suggested change
<X className="size-3" />
<X className="size-3" aria-hidden="true" />

@github-actions github-actions Bot deleted a comment from claude Bot Apr 14, 2026
- Disable OAuth JWT auth entirely when COPILOT_OAUTH_CLIENT_ID is unset
  (previously the azp check was skipped silently, which could allow JWTs
  issued to other OAuth clients on the same provider to authenticate)
- Require tenantId claim on support_copilot JWTs (was optional before)
- Validate issuer (iss) claim in jwtVerify as defense-in-depth
- Extract JWKS singleton to shared lazy-initialized util
  (src/utils/oauthJwks.ts) to remove duplication across middleware
- Add commonGetErrorResponses to tenantApps route for consistency
- Fix support_copilot missing from type cast in apps list handler
- Add aria-hidden to decorative icons in CredentialMultiSelect

Tests: new coverage for the unconfigured env var and missing-tenantId
paths. Full agents-api suite: 2365 passed.

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
Copy link
Copy Markdown
Contributor

@claude claude Bot left a comment

Choose a reason for hiding this comment

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

PR Review Summary

(0) Total Issues | Risk: Low

This is a re-review scoped to the delta since the prior automated review. The fix commit (4f3a0445 fix(oauth): tighten OAuth JWT auth based on review feedback) addresses all previously raised issues.

Prior Findings — Now Resolved ✅

Prior Finding Resolution in Delta
🟠 OAuth JWT azp validation bypassed when COPILOT_OAUTH_CLIENT_ID unset Fixed: Both manageAuth.ts:130 and runAuth.ts:419-424 now guard the entire JWT path with if (env.COPILOT_OAUTH_CLIENT_ID && ...) or early-return when unset
🟠 Missing test coverage for unconfigured env var scenario Fixed: New tests in both manage-auth-oauth-jwt.test.ts:164-182 and app-credential-auth.test.ts:758-776 verify jwtVerifyMock is NOT called when env var is undefined
🟡 Missing tenantId claim validation in runAuth Fixed: runAuth.ts:449-452 now explicitly rejects JWTs missing the https://inkeep.com/tenantId claim
🟡 Missing issuer (iss) validation Fixed: Both paths now call jwtVerify(token, getOAuthJwks(), { issuer: getOAuthIssuer() })
🟡 JWKS singleton duplication between files Fixed: Extracted to shared oauthJwks.ts with lazy-initialized singleton
🟡 Type cast missing support_copilot in apps.ts Fixed: Line 70 now casts to 'web_client' | 'api' | 'support_copilot' | undefined
🟡 Missing commonGetErrorResponses in tenantApps.ts Fixed: Added at line 44
🟡 Accessibility: icons missing aria-hidden Fixed: All icons in credential-multi-select.tsx now have aria-hidden attribute

Delta Quality Assessment

The fix commit is clean and targeted:

  • Security: The OAuth JWT auth path is now correctly gated — when COPILOT_OAUTH_CLIENT_ID is unset, no JWT verification is attempted, preventing confused-deputy attacks from other OAuth clients
  • Defense-in-depth: Added issuer validation ensures JWTs from external issuers are rejected
  • Test coverage: New tests explicitly verify the security boundary (env var unset → no JWT verification)
  • Architecture: JWKS singleton extraction eliminates duplication and module-load-time env dependency

✅ APPROVE

Summary: The fix commit comprehensively addresses all prior review findings with appropriate tests. The OAuth JWT security model is now correctly implemented: unconfigured deployments reject all JWTs for support_copilot apps, and configured deployments validate azp, issuer, tenantId, and SpiceDB permissions. Ship it! 🚀

Reviewers (1)
Reviewer Returned Main Findings Consider While You're Here Inline Comments Pending Recs Discarded
orchestrator (delta) 0 0 0 0 0 0 0
Total 0 0 0 0 0 0 0

Note: Delta re-review — no new reviewer dispatch needed as prior findings were addressed.

@github-actions
Copy link
Copy Markdown
Contributor

Preview URLs

Use these stable preview aliases for testing this PR:

These point to the same Vercel preview deployment as the bot comment, but they stay stable and easier to find.

Raw Vercel deployment URLs

@socket-security
Copy link
Copy Markdown

Warning

Review the following alerts detected in dependencies.

According to your organization's Security Policy, it is recommended to resolve "Warn" alerts. Learn more about Socket for GitHub.

Action Severity Alert  (click "▶" to expand/collapse)
Warn High
Obfuscated code: npm @inkeep/agents-manage-ui is 98.0% likely obfuscated

Confidence: 0.98

Location: Package overview

From: create-agents-template/package.jsonnpm/@inkeep/[email protected]

ℹ Read more on: This package | This alert | What is obfuscated code?

Next steps: Take a moment to review the security alert above. Review the linked package source code to understand the potential risk. Ensure the package is not malicious before proceeding. If you're unsure how to proceed, reach out to your security team or ask the Socket team for help at [email protected].

Suggestion: Packages should not obfuscate their code. Consider not using packages with obfuscated code.

Mark the package as acceptable risk. To ignore this alert only in this pull request, reply with the comment @SocketSecurity ignore npm/@inkeep/[email protected]. You can also ignore all packages with @SocketSecurity ignore-all. To ignore an alert for all future pull requests, use Socket's Dashboard to change the triage state of this alert.

View full report

@omar-inkeep omar-inkeep added this pull request to the merge queue Apr 14, 2026
Merged via the queue into main with commit 557f700 Apr 14, 2026
27 checks passed
@omar-inkeep omar-inkeep deleted the feat/support-copilot-app branch April 14, 2026 18:12
@itoqa
Copy link
Copy Markdown

itoqa Bot commented Apr 14, 2026

Ito Test Report ❌

15 test cases ran. 1 failed, 14 passed.

Across 15 executed test cases, 14 passed and 1 failed: Support Copilot create/edit flows (including zero-credential, deep-link/refresh/back-forward, double-submit, and desktop/mobile viewport behavior), tenant-level discovery/filtering, entitlement/cloud gating, manage OAuth JWT success, and multiple negative run-auth checks (wrong azp, tenant mismatch, denied user) all behaved as expected. The key finding is one high-severity likely bug in /run/api/chat/completions where malformed OAuth bearer tokens missing the tenant claim are not deterministically rejected at auth time and can fall through to a dev/test default context, resulting in a 500 instead of the expected 401/403.

❌ Failed (1)
Category Summary Screenshot
Adversarial ⚠️ Run auth does not consistently reject malformed OAuth JWTs; missing tenant claim can produce 500 instead of 401. ADV-4
⚠️ Manage and run flows reject malformed OAuth bearer tokens
  • What failed: The request is not rejected as unauthorized (401) at auth time; it can fall into default dev execution context and later error with internal failure behavior (500).
  • Impact: Security-boundary behavior is inconsistent for malformed OAuth tokens, and clients receive an internal server failure instead of a deterministic auth denial. This can break integrations and obscures the true auth failure mode.
  • Steps to reproduce:
    1. Create or select a support_copilot app and call /run/api/chat/completions with x-inkeep-app-id set to that app.
    2. Send a malformed OAuth bearer token that is missing the https://inkeep.com/tenantId claim.
    3. Observe that auth does not return a deterministic 401 and the request can proceed into fallback context and fail later as a 500.
  • Stub / mock context: Malformed OAuth scenarios were simulated with deterministic bearer-token fixtures in auth middleware, and a local bypass credential was used to seed a minimal support_copilot app before the run endpoint check. This setup was used to exercise malformed-token paths without external identity-provider dependencies.
  • Code analysis: I reviewed run auth middleware and downstream run context validation. The OAuth support-copilot path returns a non-terminal auth miss when tenant claim is absent, then the development/test fallback builds a default context instead of rejecting the request, and downstream header validation dereferences an agent from that fallback context.
  • Why this is likely a bug: The auth middleware explicitly recognizes a malformed OAuth token but still allows a fallback execution context in dev/test, which violates the expected malformed-token rejection path and plausibly causes the observed 500.

Relevant code:

agents-api/src/middleware/runAuth.ts (lines 445-452)

if (!sub) {
  return { authResult: null, failureMessage: 'OAuth JWT missing sub claim' };
}

if (!jwtTenantId) {
  logger.warn({ appId: app.id, userId: sub }, 'OAuth JWT missing tenantId claim');
  return { authResult: null, failureMessage: 'OAuth JWT missing tenantId claim' };
}

agents-api/src/middleware/runAuth.ts (lines 998-1014)

if (isDev) {
  logger.info('development environment');

  const attempt = await authenticateRequest(reqData);

  if (attempt.authResult) {
    c.set('executionContext', buildExecutionContext(attempt.authResult, reqData));
  } else {
    logger.info(
      {},
      reqData.apiKey
        ? 'Development/test environment - fallback to default context due to invalid API key'
        : 'Development/test environment - no API key provided, using default context'
    );
    c.set('executionContext', buildExecutionContext(createDevContext(reqData), reqData));
  }
}

agents-api/src/domains/run/context/validation.ts (lines 282-287)

const { agentId, project } = executionContext;
logger.info('Validating headers');
const agent = project.agents[agentId];

const contextConfig = agent.contextConfig;
logger.debug({ contextConfig }, 'Context config found');
✅ Passed (14)
Category Summary Screenshot
Adversarial Both adversarial wrong-azp requests to run chat completions were rejected with HTTP 401 as expected after enabling local OAuth config and deterministic non-production token-map bypass. ADV-1
Adversarial Tenant-mismatch OAuth token was rejected with HTTP 403 on run endpoint, matching expected forbidden behavior. ADV-2
Adversarial Structurally valid OAuth token mapped to denied user path returned HTTP 403, confirming project-permission enforcement behavior. ADV-3
Adversarial Rapid double-submit produced exactly one created app row after reload, confirming duplicate prevention. ADV-5
Edge Support Copilot stayed hidden when cloud deployment was off, while Web Client and API remained visible. EDGE-1
Edge Support Copilot stayed hidden with entitlement max value 0 and reappeared after entitlement was restored to 1. EDGE-2
Edge Zero-credential project flow created a Support Copilot app successfully without requiring credential selection. EDGE-3
Edge Deep-link refresh reset in-progress dialog without phantom writes, and back/forward did not duplicate creation. EDGE-5
Mobile Desktop + 390x844 mobile create/edit flows succeeded; credential multi-select stayed usable/visible and edits persisted after reopen. MOBILE-1
Happy-path Support Copilot app creation succeeded and rendered with the correct type badge after local gate setup was corrected. ROUTE-1
Happy-path Existing Support Copilot app update persisted credentials change (A+B to B+C) and enabled state after reopen. ROUTE-2
Happy-path Tenant-level apps endpoint returned cross-project data with expected pagination total after seeded baseline data was present. ROUTE-3
Happy-path Tenant support_copilot filter returned HTTP 200 with only support_copilot app types. ROUTE-4
Happy-path Manage endpoint accepted valid OAuth JWT claims and returned tenant-scoped app data over bearer auth. ROUTE-6

Commit: 4f3a044

View Full Run


Tell us how we did: Give Ito Feedback

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant