diff --git a/server/src/__tests__/company-skills-service.test.ts b/server/src/__tests__/company-skills-service.test.ts index 52d098e275..26a479567f 100644 --- a/server/src/__tests__/company-skills-service.test.ts +++ b/server/src/__tests__/company-skills-service.test.ts @@ -577,6 +577,116 @@ describeEmbeddedPostgres("companySkillService.list", () => { await expect(svc.getById(companyId, skillId)).resolves.toBeNull(); }); + it("refreshes stale local-path file inventory from disk", async () => { + const companyId = randomUUID(); + const skillId = randomUUID(); + const skillDir = await fs.mkdtemp(path.join(os.tmpdir(), "paperclip-stale-inventory-skill-")); + cleanupDirs.add(skillDir); + await fs.mkdir(path.join(skillDir, "references"), { recursive: true }); + await fs.writeFile(path.join(skillDir, "SKILL.md"), "# Stale Inventory Skill\n", "utf8"); + await fs.writeFile(path.join(skillDir, "references", "guide.md"), "# Guide\n", "utf8"); + + await db.insert(companies).values({ + id: companyId, + name: "Paperclip", + issuePrefix: `T${companyId.replace(/-/g, "").slice(0, 6).toUpperCase()}`, + requireBoardApprovalForNewAgents: false, + }); + await db.insert(companySkills).values({ + id: skillId, + companyId, + key: `company/${companyId}/stale-inventory-skill`, + slug: "stale-inventory-skill", + name: "Stale Inventory Skill", + description: null, + markdown: "# Stale Inventory Skill\n", + sourceType: "local_path", + sourceLocator: skillDir, + trustLevel: "markdown_only", + compatibility: "compatible", + fileInventory: [{ path: "SKILL.md", kind: "skill" }], + metadata: { sourceKind: "local_path" }, + }); + + const listed = await svc.list(companyId); + const skill = listed.find((entry) => entry.id === skillId); + + expect(new Set(skill?.fileInventory.map((entry) => `${entry.kind}:${entry.path}`))).toEqual(new Set([ + "skill:SKILL.md", + "reference:references/guide.md", + ])); + await expect(svc.readFile(companyId, skillId, "references/guide.md")).resolves.toMatchObject({ + path: "references/guide.md", + kind: "reference", + content: "# Guide\n", + }); + await expect(svc.getById(companyId, skillId)).resolves.toMatchObject({ + fileInventory: expect.arrayContaining([ + expect.objectContaining({ path: "SKILL.md", kind: "skill" }), + expect.objectContaining({ path: "references/guide.md", kind: "reference" }), + ]), + }); + }); + + it("imports sibling reference files when the source is a direct SKILL.md path", async () => { + const companyId = randomUUID(); + const skillDir = await fs.mkdtemp(path.join(os.tmpdir(), "paperclip-file-import-skill-")); + cleanupDirs.add(skillDir); + await fs.mkdir(path.join(skillDir, "references"), { recursive: true }); + await fs.writeFile( + path.join(skillDir, "SKILL.md"), + "---\nname: File Import Skill\n---\n\n# File Import Skill\n", + "utf8", + ); + await fs.writeFile(path.join(skillDir, "references", "checklist.md"), "# Checklist\n", "utf8"); + + await db.insert(companies).values({ + id: companyId, + name: "Paperclip", + issuePrefix: `T${companyId.replace(/-/g, "").slice(0, 6).toUpperCase()}`, + requireBoardApprovalForNewAgents: false, + }); + + const result = await svc.importFromSource(companyId, path.join(skillDir, "SKILL.md")); + + expect(result.imported).toHaveLength(1); + expect(new Set(result.imported[0]?.fileInventory.map((entry) => `${entry.kind}:${entry.path}`))).toEqual(new Set([ + "skill:SKILL.md", + "reference:references/checklist.md", + ])); + }); + + it("bounds direct root SKILL.md imports to known support directories", async () => { + const companyId = randomUUID(); + const repoDir = await fs.mkdtemp(path.join(os.tmpdir(), "paperclip-root-skill-")); + cleanupDirs.add(repoDir); + await fs.mkdir(path.join(repoDir, "references"), { recursive: true }); + await fs.mkdir(path.join(repoDir, "server", "src"), { recursive: true }); + await fs.writeFile( + path.join(repoDir, "SKILL.md"), + "---\nname: Root Skill\n---\n\n# Root Skill\n", + "utf8", + ); + await fs.writeFile(path.join(repoDir, "references", "guide.md"), "# Guide\n", "utf8"); + await fs.writeFile(path.join(repoDir, "README.md"), "# Repo readme\n", "utf8"); + await fs.writeFile(path.join(repoDir, "server", "src", "index.ts"), "export {};\n", "utf8"); + + await db.insert(companies).values({ + id: companyId, + name: "Paperclip", + issuePrefix: `T${companyId.replace(/-/g, "").slice(0, 6).toUpperCase()}`, + requireBoardApprovalForNewAgents: false, + }); + + const result = await svc.importFromSource(companyId, path.join(repoDir, "SKILL.md")); + + expect(result.imported).toHaveLength(1); + expect(result.imported[0]?.fileInventory.map((entry) => entry.path).sort()).toEqual([ + "SKILL.md", + "references/guide.md", + ]); + }); + it("rejects executable external package skills before persistence", async () => { const companyId = randomUUID(); await db.insert(companies).values({ diff --git a/server/src/__tests__/secrets-routes.test.ts b/server/src/__tests__/secrets-routes.test.ts index 33bdfc672a..b97b4b17b5 100644 --- a/server/src/__tests__/secrets-routes.test.ts +++ b/server/src/__tests__/secrets-routes.test.ts @@ -236,6 +236,55 @@ describe("secret routes", () => { expect(JSON.stringify(mockLogActivity.mock.calls)).not.toContain("paperclip/prod-use1/company-1/openai"); }); + it("returns actionable sanitized provider vault discovery errors", async () => { + mockSecretService.previewProviderConfigDiscovery.mockRejectedValue( + new HttpError( + 403, + "AWS Secrets Manager denied the request. Check IAM permissions for this provider vault.", + { + code: "access_denied", + provider: "aws_secrets_manager", + operation: "secret_provider_config.discovery.preview", + providerConfigId: "discovery-preview", + providerVaultContext: "draft_config", + region: "us-east-1", + credentialPath: "Paperclip server runtime/provider credential path", + requiredCapability: "secretsmanager:ListSecrets", + actionableMessage: + "AWS discovery preview needs secretsmanager:ListSecrets in the selected region for the Paperclip server runtime/provider credential path.", + safeAlternative: + "If the operator already knows the exact AWS Secrets Manager ARN, paste/link that ARN instead of using discovery. Exact-resource DescribeSecret and runtime read permissions are still required.", + }, + ), + ); + + const res = await request(createApp()) + .post("/api/companies/company-1/secret-provider-configs/discovery/preview") + .send({ + provider: "aws_secrets_manager", + config: { region: "us-east-1" }, + pageSize: 25, + }); + + expect(res.status).toBe(403); + expect(res.body).toMatchObject({ + error: "AWS Secrets Manager denied the request. Check IAM permissions for this provider vault.", + details: { + code: "access_denied", + provider: "aws_secrets_manager", + operation: "secret_provider_config.discovery.preview", + providerVaultContext: "draft_config", + region: "us-east-1", + requiredCapability: "secretsmanager:ListSecrets", + }, + }); + expect(res.body.details.actionableMessage).toContain("Paperclip server runtime/provider credential path"); + expect(res.body.details.safeAlternative).toContain("paste/link that ARN"); + expect(JSON.stringify(res.body)).not.toContain("arn:aws"); + expect(JSON.stringify(res.body)).not.toContain("123456789012"); + expect(mockLogActivity).not.toHaveBeenCalled(); + }); + it("rejects ready status for coming-soon provider vaults", async () => { const res = await request(createApp()).post("/api/companies/company-1/secret-provider-configs").send({ provider: "vault", diff --git a/server/src/__tests__/secrets-service.test.ts b/server/src/__tests__/secrets-service.test.ts index 93ae8bcf37..4d7ffd5a68 100644 --- a/server/src/__tests__/secrets-service.test.ts +++ b/server/src/__tests__/secrets-service.test.ts @@ -1432,7 +1432,20 @@ describeEmbeddedPostgres("secretService", () => { expect(thrown).toMatchObject({ status: 403, message: "AWS Secrets Manager denied the request. Check IAM permissions for this provider vault.", - details: { code: "access_denied" }, + details: { + code: "access_denied", + provider: "aws_secrets_manager", + operation: "secret_provider_config.discovery.preview", + providerConfigId: "discovery-preview", + providerVaultContext: "draft_config", + region: "us-east-1", + credentialPath: "Paperclip server runtime/provider credential path", + requiredCapability: "secretsmanager:ListSecrets", + actionableMessage: + "AWS discovery preview needs secretsmanager:ListSecrets in the selected region for the Paperclip server runtime/provider credential path.", + safeAlternative: + "If the operator already knows the exact AWS Secrets Manager ARN, paste/link that ARN instead of using discovery. Exact-resource DescribeSecret and runtime read permissions are still required.", + }, }); expect(JSON.stringify(thrown)).not.toContain("arn:aws"); expect(JSON.stringify(thrown)).not.toContain("123456789012"); diff --git a/server/src/services/company-skills.ts b/server/src/services/company-skills.ts index 712e5a2676..e0cfc930b6 100644 --- a/server/src/services/company-skills.ts +++ b/server/src/services/company-skills.ts @@ -939,6 +939,38 @@ async function collectLocalSkillInventory( .sort((left, right) => left.path.localeCompare(right.path)); } +function inventoryEntriesEqual( + left: CompanySkillFileInventoryEntry[], + right: CompanySkillFileInventoryEntry[], +) { + if (left.length !== right.length) return false; + const normalize = (entries: CompanySkillFileInventoryEntry[]) => + entries + .map((entry) => ({ + path: normalizePortablePath(entry.path), + kind: entry.kind, + })) + .sort((leftEntry, rightEntry) => leftEntry.path.localeCompare(rightEntry.path)); + const normalizedLeft = normalize(left); + const normalizedRight = normalize(right); + return normalizedLeft.every((entry, index) => { + const other = normalizedRight[index]; + return other?.path === entry.path && other.kind === entry.kind; + }); +} + +function inferLocalSkillInventoryMode( + skill: Pick, +): LocalSkillInventoryMode { + const metadata = isPlainRecord(skill.metadata) ? skill.metadata : null; + const sourceKind = asString(metadata?.sourceKind); + const workspaceCwd = asString(metadata?.workspaceCwd); + if (sourceKind === "project_scan" && workspaceCwd && skill.sourceLocator === workspaceCwd) { + return "project_root"; + } + return "full"; +} + export async function readLocalSkillImportFromDirectory( companyId: string, skillDir: string, @@ -1022,8 +1054,9 @@ async function readLocalSkillImports(companyId: string, sourcePath: string): Pro if (stat.isFile()) { const markdown = await fs.readFile(resolvedPath, "utf8"); + const sourceDir = path.dirname(resolvedPath); const parsed = parseFrontmatterMarkdown(markdown); - const slug = deriveImportedSkillSlug(parsed.frontmatter, path.basename(path.dirname(resolvedPath))); + const slug = deriveImportedSkillSlug(parsed.frontmatter, path.basename(sourceDir)); const parsedMetadata = isPlainRecord(parsed.frontmatter.metadata) ? parsed.frontmatter.metadata : null; const skillKey = readCanonicalSkillKey(parsed.frontmatter, parsedMetadata); const metadata = { @@ -1031,14 +1064,12 @@ async function readLocalSkillImports(companyId: string, sourcePath: string): Pro ...(parsedMetadata ?? {}), sourceKind: "local_path", }; - const inventory: CompanySkillFileInventoryEntry[] = [ - { path: "SKILL.md", kind: "skill" }, - ]; + const inventory = await collectLocalSkillInventory(sourceDir, "project_root"); return [{ key: deriveCanonicalSkillKey(companyId, { slug, sourceType: "local_path", - sourceLocator: path.dirname(resolvedPath), + sourceLocator: sourceDir, metadata, }), slug, @@ -1047,7 +1078,7 @@ async function readLocalSkillImports(companyId: string, sourcePath: string): Pro markdown, packageDir: path.dirname(resolvedPath), sourceType: "local_path", - sourceLocator: path.dirname(resolvedPath), + sourceLocator: sourceDir, sourceRef: null, trustLevel: deriveTrustLevel(inventory), compatibility: "compatible", @@ -1237,6 +1268,18 @@ async function readUrlSkillImports( throw unprocessable("Unsupported skill source. Use a local path or URL."); } +function normalizeFileInventory(row: { fileInventory: unknown }): CompanySkillFileInventoryEntry[] { + return Array.isArray(row.fileInventory) + ? row.fileInventory.flatMap((entry) => { + if (!isPlainRecord(entry)) return []; + return [{ + path: String(entry.path ?? ""), + kind: (String(entry.kind ?? "other") as CompanySkillFileInventoryEntry["kind"]), + }]; + }) + : []; +} + function toCompanySkill(row: CompanySkillRow): CompanySkill { return { ...row, @@ -1246,15 +1289,7 @@ function toCompanySkill(row: CompanySkillRow): CompanySkill { sourceRef: row.sourceRef ?? null, trustLevel: row.trustLevel as CompanySkillTrustLevel, compatibility: row.compatibility as CompanySkillCompatibility, - fileInventory: Array.isArray(row.fileInventory) - ? row.fileInventory.flatMap((entry) => { - if (!isPlainRecord(entry)) return []; - return [{ - path: String(entry.path ?? ""), - kind: (String(entry.kind ?? "other") as CompanySkillFileInventoryEntry["kind"]), - }]; - }) - : [], + fileInventory: normalizeFileInventory(row), iconUrl: row.iconUrl ?? null, color: row.color ?? null, tagline: row.tagline ?? null, @@ -1282,15 +1317,7 @@ function toCompanySkillListRow(row: CompanySkillListDbRow): CompanySkillListRow sourceRef: row.sourceRef ?? null, trustLevel: row.trustLevel as CompanySkillTrustLevel, compatibility: row.compatibility as CompanySkillCompatibility, - fileInventory: Array.isArray(row.fileInventory) - ? row.fileInventory.flatMap((entry) => { - if (!isPlainRecord(entry)) return []; - return [{ - path: String(entry.path ?? ""), - kind: (String(entry.kind ?? "other") as CompanySkillFileInventoryEntry["kind"]), - }]; - }) - : [], + fileInventory: normalizeFileInventory(row), iconUrl: row.iconUrl ?? null, color: row.color ?? null, tagline: row.tagline ?? null, @@ -2137,6 +2164,8 @@ export function companySkillService(db: Db) { slug: companySkills.slug, sourceType: companySkills.sourceType, sourceLocator: companySkills.sourceLocator, + trustLevel: companySkills.trustLevel, + fileInventory: companySkills.fileInventory, metadata: companySkills.metadata, }) .from(companySkills) @@ -2144,6 +2173,8 @@ export function companySkillService(db: Db) { const skills = rows.map((row) => ({ ...row, sourceType: row.sourceType as CompanySkillSourceType, + trustLevel: row.trustLevel as CompanySkillTrustLevel, + fileInventory: normalizeFileInventory(row), metadata: isPlainRecord(row.metadata) ? row.metadata : null, })); const missingIds = new Set(await findMissingLocalSkillIds(skills)); @@ -2152,11 +2183,23 @@ export function companySkillService(db: Db) { if (skill.sourceType !== "local_path") continue; if (!missingIds.has(skill.id)) { - if (getMissingSourceMarker(skill.metadata)) { + const metadata = getMissingSourceMarker(skill.metadata) + ? withoutMissingSourceMarker(skill.metadata) + : skill.metadata; + const sourceLocator = asString(skill.sourceLocator); + const nextInventory = sourceLocator + ? await collectLocalSkillInventory(sourceLocator, inferLocalSkillInventoryMode(skill)).catch(() => null) + : null; + const nextTrustLevel = nextInventory ? deriveTrustLevel(nextInventory) : skill.trustLevel; + const inventoryChanged = nextInventory ? !inventoryEntriesEqual(skill.fileInventory, nextInventory) : false; + const metadataChanged = JSON.stringify(metadata ?? {}) !== JSON.stringify(skill.metadata ?? {}); + if (inventoryChanged || metadataChanged || nextTrustLevel !== skill.trustLevel) { await db .update(companySkills) .set({ - metadata: withoutMissingSourceMarker(skill.metadata), + ...(nextInventory ? { fileInventory: serializeFileInventory(nextInventory) } : {}), + trustLevel: nextTrustLevel, + metadata, updatedAt: new Date(), }) .where(eq(companySkills.id, skill.id)); diff --git a/server/src/services/secrets.ts b/server/src/services/secrets.ts index c8de22cf6a..1f0bf36d65 100644 --- a/server/src/services/secrets.ts +++ b/server/src/services/secrets.ts @@ -72,6 +72,7 @@ function remoteProviderHttpError(error: unknown, context: { provider: SecretProvider; providerConfigId: string; operation: string; + providerConfig?: Record | null; }): HttpError { if (isSecretProviderClientError(error)) { logger.warn( @@ -85,7 +86,7 @@ function remoteProviderHttpError(error: unknown, context: { }, "remote secret provider request failed", ); - return new HttpError(error.status, error.message, { code: error.code }); + return new HttpError(error.status, error.message, safeRemoteProviderErrorDetails(error, context)); } if (error instanceof HttpError) return error; logger.warn( @@ -99,7 +100,46 @@ function remoteProviderHttpError(error: unknown, context: { }, "remote secret provider request failed", ); - return new HttpError(502, "Remote secret provider request failed.", { code: "provider_error" }); + return new HttpError(502, "Remote secret provider request failed.", safeRemoteProviderErrorDetails(null, context)); +} + +function safeRemoteProviderErrorDetails( + error: { code: string } | null, + context: { + provider: SecretProvider; + providerConfigId: string; + operation: string; + providerConfig?: Record | null; + }, +): Record { + if ( + context.provider !== "aws_secrets_manager" || + context.operation !== "secret_provider_config.discovery.preview" + ) { + return { code: error?.code ?? "provider_error" }; + } + const details: Record = { + code: error?.code ?? "provider_error", + provider: context.provider, + operation: context.operation, + providerConfigId: context.providerConfigId, + }; + const region = safeString(context.providerConfig?.region); + if (region) details.region = region; + details.providerVaultContext = context.providerConfigId === "discovery-preview" ? "draft_config" : "provider_config"; + details.credentialPath = "Paperclip server runtime/provider credential path"; + if (error?.code === "access_denied") { + details.requiredCapability = "secretsmanager:ListSecrets"; + details.actionableMessage = + "AWS discovery preview needs secretsmanager:ListSecrets in the selected region for the Paperclip server runtime/provider credential path."; + details.safeAlternative = + "If the operator already knows the exact AWS Secrets Manager ARN, paste/link that ARN instead of using discovery. Exact-resource DescribeSecret and runtime read permissions are still required."; + } + return details; +} + +function safeString(value: unknown): string | null { + return typeof value === "string" && value.trim() ? value.trim() : null; } function remoteImportRowFailureReason(error: unknown, fallback: string, context: { @@ -1091,6 +1131,7 @@ export function secretService(db: Db) { provider: providerId, providerConfigId: "discovery-preview", operation: "secret_provider_config.discovery.preview", + providerConfig: parsed.data.config, }); } }, diff --git a/ui/src/context/FileViewerContext.test.ts b/ui/src/context/FileViewerContext.test.ts index 24ee4a45e2..5388497bcc 100644 --- a/ui/src/context/FileViewerContext.test.ts +++ b/ui/src/context/FileViewerContext.test.ts @@ -120,6 +120,20 @@ describe("writeFileViewerStateToSearch", () => { expect(params.get("keep")).toBe("yes"); }); + it("clears browse-origin viewer params when closing a selected file", () => { + const next = writeFileViewerStateToSearch( + "?tab=thread&browse=1&q=FileViewer&folder=ui/src&file=ui/src/FileViewer.tsx&line=4", + null, + ); + const params = new URLSearchParams(next); + expect(params.get("file")).toBeNull(); + expect(params.get("line")).toBeNull(); + expect(params.get("browse")).toBeNull(); + expect(params.get("q")).toBeNull(); + expect(params.get("folder")).toBeNull(); + expect(params.get("tab")).toBe("thread"); + }); + it("returns empty string when no params remain", () => { const next = writeFileViewerStateToSearch("?file=a.ts", null); expect(next).toBe(""); diff --git a/ui/src/context/FileViewerContext.tsx b/ui/src/context/FileViewerContext.tsx index 6764b17276..77ce2dac34 100644 --- a/ui/src/context/FileViewerContext.tsx +++ b/ui/src/context/FileViewerContext.tsx @@ -285,7 +285,10 @@ function EnabledFileViewerProvider({ issueId, children }: Omit { - const params = new URLSearchParams(writeFileViewerStateToSearch(location.search, null).replace(/^\?/, "")); + const currentSearch = typeof window === "undefined" + ? location.search + : (window.location.search || location.search); + const params = new URLSearchParams(writeFileViewerStateToSearch(currentSearch, null).replace(/^\?/, "")); params.delete("browse"); params.delete("q"); params.delete("folder"); diff --git a/ui/src/pages/IssueDetail.tsx b/ui/src/pages/IssueDetail.tsx index 8ada911036..19774bead5 100644 --- a/ui/src/pages/IssueDetail.tsx +++ b/ui/src/pages/IssueDetail.tsx @@ -4525,6 +4525,13 @@ function IssueFileViewer({ const viewer = useRequiredFileViewer(); const open = viewer.state !== null || viewer.browse || promptOpen; const showPromptWhenEmpty = (promptOpen || viewer.browse) && viewer.state === null; + + useEffect(() => { + if (!promptOpen) return; + if (viewer.state === null && !viewer.browse) return; + onPromptOpenChange(false); + }, [onPromptOpenChange, promptOpen, viewer.browse, viewer.state]); + return ( []; +async function act(callback: () => void | Promise) { + let result: void | Promise = undefined; + flushSync(() => { + result = callback(); + }); + await result; +} + async function flushReact() { await act(async () => { await Promise.resolve(); @@ -564,9 +572,24 @@ describe("Secrets page layout", () => { }); it("shows AWS discovery errors without replacing manual vault form values", async () => { + const rawProviderMessage = + "AccessDeniedException: User: arn:aws:sts::123456789012:assumed-role/prod/Paperclip is not authorized"; mockSecretsApi.providerConfigDiscoveryPreview.mockRejectedValueOnce( new ApiError("AWS Secrets Manager denied the request. Check IAM permissions for this provider vault.", 403, { - details: { code: "access_denied" }, + details: { + code: "access_denied", + provider: "aws_secrets_manager", + operation: "secret_provider_config.discovery.preview", + providerConfigId: "discovery-preview", + providerVaultContext: "draft_config", + region: "us-west-2", + credentialPath: "Paperclip server runtime/provider credential path", + requiredCapability: "secretsmanager:ListSecrets", + actionableMessage: + "AWS discovery preview needs secretsmanager:ListSecrets in the selected region for the Paperclip server runtime/provider credential path.", + safeAlternative: + "If the operator already knows the exact AWS Secrets Manager ARN, paste/link that ARN instead of using discovery. Exact-resource DescribeSecret and runtime read permissions are still required.", + }, }), ); const root = createRoot(container); @@ -604,7 +627,20 @@ describe("Secrets page layout", () => { await flushReact(); await flushReact(); - expect(document.body.textContent).toContain("AWS Secrets Manager denied the request"); + const errorBanner = document.querySelector('[data-testid="aws-vault-discovery-error"]'); + expect(errorBanner).not.toBeNull(); + expect(errorBanner?.textContent).toContain("AWS discovery needs ListSecrets permission"); + expect(errorBanner?.textContent).toContain("secretsmanager:ListSecrets"); + expect(errorBanner?.textContent).toContain("Paperclip server runtime/provider credential path"); + expect(errorBanner?.textContent).toContain("paste/link that ARN"); + expect(errorBanner?.textContent).toContain("DescribeSecret"); + expect(errorBanner?.textContent).toContain("us-west-2"); + expect(errorBanner?.textContent).toContain("secret_provider_config.discovery.preview"); + expect(errorBanner?.textContent).toContain("aws_secrets_manager"); + expect(errorBanner?.textContent).toContain("Safe request/error details"); + expect(errorBanner?.textContent).not.toContain(rawProviderMessage); + expect(errorBanner?.textContent).not.toContain("arn:aws"); + expect(errorBanner?.textContent).not.toContain("123456789012"); expect(regionInput.value).toBe("us-west-2"); expect(namespaceInput.value).toBe("manual-prod"); @@ -613,6 +649,59 @@ describe("Secrets page layout", () => { }); }); + it("keeps generic AWS discovery 403 errors on the generic failure path", async () => { + mockSecretsApi.providerConfigDiscoveryPreview.mockRejectedValueOnce( + new ApiError("AWS discovery request failed before IAM evaluation.", 403, { + details: { + code: "proxy_forbidden", + provider: "aws_secrets_manager", + operation: "secret_provider_config.discovery.preview", + region: "us-west-1", + }, + }), + ); + const root = createRoot(container); + const queryClient = new QueryClient({ + defaultOptions: { queries: { retry: false } }, + }); + + await act(async () => { + root.render( + + + + + , + ); + }); + await flushReact(); + await flushReact(); + await openAwsVaultDialog(); + + const regionInput = document.getElementById("provider-vault-aws-region") as HTMLInputElement; + await act(async () => { + setInputValue(regionInput, "us-west-1"); + }); + await flushReact(); + + await act(async () => { + (document.querySelector('[data-testid="aws-vault-discovery-button"]') as HTMLButtonElement | null)?.click(); + }); + await flushReact(); + await flushReact(); + + const errorBanner = document.querySelector('[data-testid="aws-vault-discovery-error"]'); + expect(errorBanner).not.toBeNull(); + expect(errorBanner?.textContent).toContain("AWS discovery failed"); + expect(errorBanner?.textContent).toContain("AWS discovery request failed before IAM evaluation."); + expect(errorBanner?.textContent).toContain("proxy_forbidden"); + expect(errorBanner?.textContent).not.toContain("AWS discovery needs ListSecrets permission"); + + await act(async () => { + root.unmount(); + }); + }); + it("shows an empty AWS discovery result without blocking manual entry", async () => { mockSecretsApi.providerConfigDiscoveryPreview.mockResolvedValueOnce( makeDiscoveryPreview({ candidates: [], sampledSecretCount: 0 }), diff --git a/ui/src/pages/Secrets.tsx b/ui/src/pages/Secrets.tsx index 0050a8bbd3..9ca27931f3 100644 --- a/ui/src/pages/Secrets.tsx +++ b/ui/src/pages/Secrets.tsx @@ -99,6 +99,19 @@ type ProviderVaultForm = { secretPathPrefix: string; }; +type SafeProviderErrorDetails = { + code?: string; + provider?: string; + operation?: string; + providerConfigId?: string; + providerVaultContext?: string; + region?: string; + credentialPath?: string; + requiredCapability?: string; + actionableMessage?: string; + safeAlternative?: string; +}; + const PROVIDER_ORDER: SecretProvider[] = [ "local_encrypted", "aws_secrets_manager", @@ -137,6 +150,34 @@ function providerConfigValue(config: CompanySecretProviderConfig["config"], key: return typeof value === "string" ? value : ""; } +function apiErrorDetails(error: unknown): SafeProviderErrorDetails | null { + if (!(error instanceof ApiError)) return null; + const body = error.body; + if (!body || typeof body !== "object") return null; + const details = (body as Record).details; + if (!details || typeof details !== "object" || Array.isArray(details)) return null; + return details as SafeProviderErrorDetails; +} + +function apiErrorCode(error: unknown): string | null { + return apiErrorDetails(error)?.code ?? null; +} + +function isAwsDiscoveryAccessDenied(error: unknown): boolean { + const details = apiErrorDetails(error); + if (details?.provider === "aws_secrets_manager" && details.operation === "secret_provider_config.discovery.preview") { + return details.code === "access_denied"; + } + if (!(error instanceof ApiError)) return false; + return apiErrorCode(error) === "access_denied"; +} + +function readableErrorMessage(error: unknown): string { + if (error instanceof ApiError) return error.message || `Request failed: ${error.status}`; + if (error instanceof Error) return error.message; + return "Unexpected error"; +} + function providerVaultFormFromConfig(config: CompanySecretProviderConfig): ProviderVaultForm { return { ...emptyProviderVaultForm(config.provider), @@ -389,7 +430,7 @@ export function Secrets() { const [vaultError, setVaultError] = useState(null); const [vaultDiscovery, setVaultDiscovery] = useState(null); - const [vaultDiscoveryError, setVaultDiscoveryError] = useState(null); + const [vaultDiscoveryError, setVaultDiscoveryError] = useState(null); useEffect(() => { setBreadcrumbs([{ label: "Secrets" }]); @@ -678,7 +719,7 @@ export function Secrets() { }, onError: (error) => { setVaultDiscovery(null); - setVaultDiscoveryError(error instanceof ApiError ? error.message : (error as Error).message); + setVaultDiscoveryError(error); }, }); @@ -2145,7 +2186,7 @@ function AwsProviderVaultDiscoveryPanel({ }: { form: ProviderVaultForm; preview: SecretProviderConfigDiscoveryPreviewResult | null; - error: string | null; + error: unknown | null; loading: boolean; onDiscover: () => void; onApply: (candidate: SecretProviderConfigDiscoveryCandidate) => void; @@ -2191,13 +2232,7 @@ function AwsProviderVaultDiscoveryPanel({ ) : null} {error ? ( -
- - {error} -
+ ) : null} {warnings.length > 0 ? ( @@ -2241,6 +2276,96 @@ function AwsProviderVaultDiscoveryPanel({ ); } +function AwsProviderVaultDiscoveryError({ + form, + error, +}: { + form: ProviderVaultForm; + error: unknown; +}) { + const details = apiErrorDetails(error); + const isAccessDenied = isAwsDiscoveryAccessDenied(error); + const region = (details?.region ?? form.region.trim()) || "unspecified"; + const message = readableErrorMessage(error); + const safeDetails = { + message, + status: error instanceof ApiError ? error.status : undefined, + provider: details?.provider ?? form.provider, + operation: details?.operation ?? "secret_provider_config.discovery.preview", + providerVaultContext: details?.providerVaultContext ?? "draft_config", + region, + code: details?.code, + requiredCapability: details?.requiredCapability, + credentialPath: details?.credentialPath, + safeAlternative: details?.safeAlternative, + }; + const detailsText = JSON.stringify(safeDetails, null, 2); + + const copyDetails = () => { + void navigator.clipboard?.writeText(detailsText); + }; + + return ( +
+
+ +
+
+

+ {isAccessDenied ? "AWS discovery needs ListSecrets permission" : "AWS discovery failed"} +

+

+ {isAccessDenied + ? details?.actionableMessage ?? + "Discovery needs secretsmanager:ListSecrets in the selected region for the Paperclip server runtime/provider credential path." + : message} +

+
+ {isAccessDenied ? ( +

+ {details?.safeAlternative ?? + "If you already know the exact AWS Secrets Manager ARN, paste/link that ARN instead of using discovery. Exact-resource DescribeSecret and runtime read permissions are still required."} +

+ ) : null} +
+
+
Region
+
{region}
+
+
+
Operation
+
{details?.operation ?? "secret_provider_config.discovery.preview"}
+
+
+
Provider
+
{details?.provider ?? "aws_secrets_manager"}
+
+
+
Vault context
+
{details?.providerVaultContext ?? "draft_config"}
+
+
+
+
+ Safe request/error details + +
+ 
+              {detailsText}
+
+
+
+
+
+ ); +} + function AwsProviderVaultDiscoveryCandidateRow({ candidate, onApply,