From 978a0f255ea6ee40152530f723dcc8ba3853afe9 Mon Sep 17 00:00:00 2001 From: Bruno Clermont Date: Mon, 29 Jun 2026 09:54:30 -0400 Subject: [PATCH] Fix/skill violation display (#1304) * feat(skills): display structured security violations in upload error When skill upload fails due to security violations, show each violation as a clear list with line number and reason in the error display. Changes: - Add ApiError.getViolations() method to extract violations from response - Update HTTP upload client to capture violations in error details - Add violations field to SkillEntry type - Display violations as bulleted list in SkillSubtitle when present - Add i18n keys for security violations display (en, vi, zh, ko) New UI behavior: Instead of a generic "Upload failed" message, users now see structured violation details: Security violations: - Line 5: SQL DROP TABLE - Line 12: curl pipe to shell Backward compatible - falls back to generic error message if violations array is absent from response. Co-Authored-By: Claude Sonnet 4.6 * feat(skills): return structured JSON violations array in upload error response Replace flat string formatting with a structured violations array (line, reason) so clients can display per-line security rule details instead of a generic error. Co-Authored-By: Claude Sonnet 4.6 --------- Co-authored-by: Bruno Clermont Co-authored-by: Claude Sonnet 4.6 --- internal/http/skills_upload.go | 13 ++++++++++++- ui/web/src/api/errors.ts | 8 ++++++++ ui/web/src/api/http-client.ts | 4 +++- ui/web/src/i18n/locales/en/skills.json | 4 +++- ui/web/src/i18n/locales/ko/skills.json | 4 +++- ui/web/src/i18n/locales/vi/skills.json | 4 +++- ui/web/src/i18n/locales/zh/skills.json | 4 +++- ui/web/src/pages/skills/lib/skill-upload-types.ts | 2 ++ ui/web/src/pages/skills/skill-upload-dialog.tsx | 15 ++++++++++++++- ui/web/src/pages/skills/skill-upload-entry.tsx | 15 +++++++++++++++ 10 files changed, 66 insertions(+), 7 deletions(-) diff --git a/internal/http/skills_upload.go b/internal/http/skills_upload.go index ac6f2754..4e6358fd 100644 --- a/internal/http/skills_upload.go +++ b/internal/http/skills_upload.go @@ -140,9 +140,20 @@ func (h *SkillsHandler) handleUpload(w http.ResponseWriter, r *http.Request) { "user_id", userID, "violations", len(violations), "first_rule", violations[0].Reason) + type guardViolationResponse struct { + Line int `json:"line"` + Reason string `json:"reason"` + } + violationsResponse := make([]guardViolationResponse, len(violations)) + for idx, v := range violations { + violationsResponse[idx] = guardViolationResponse{ + Line: v.Line, + Reason: v.Reason, + } + } writeJSON(w, http.StatusBadRequest, map[string]any{ "error": i18n.T(locale, i18n.MsgInvalidRequest, "skill content failed security scan"), - "violations": skills.FormatGuardViolations(violations), + "violations": violationsResponse, }) return } diff --git a/ui/web/src/api/errors.ts b/ui/web/src/api/errors.ts index 73b8de52..96241fd7 100644 --- a/ui/web/src/api/errors.ts +++ b/ui/web/src/api/errors.ts @@ -24,4 +24,12 @@ export class ApiError extends Error { super(message); this.name = "ApiError"; } + + /** Extract violations from details if present (for skill upload security errors). */ + getViolations(): Array<{ line: number; reason: string }> | null { + if (!this.details || typeof this.details !== "object") return null; + const violations = (this.details as Record).violations; + if (!Array.isArray(violations)) return null; + return violations as Array<{ line: number; reason: string }>; + } } diff --git a/ui/web/src/api/http-client.ts b/ui/web/src/api/http-client.ts index 41f057e2..86023db4 100644 --- a/ui/web/src/api/http-client.ts +++ b/ui/web/src/api/http-client.ts @@ -87,7 +87,9 @@ export class HttpClient { const nested = typeof err.error === "object" && err.error !== null ? err.error : null; const code = nested?.code ?? err.code ?? "HTTP_ERROR"; const message = nested?.message ?? (typeof err.error === "string" ? err.error : null) ?? err.message ?? res.statusText; - throw new ApiError(code, message); + // Capture violations from API response for skill upload errors + const details = nested || err; + throw new ApiError(code, message, details); } return this.readJson(res); diff --git a/ui/web/src/i18n/locales/en/skills.json b/ui/web/src/i18n/locales/en/skills.json index 0a7c5fb0..2920c2ec 100644 --- a/ui/web/src/i18n/locales/en/skills.json +++ b/ui/web/src/i18n/locales/en/skills.json @@ -151,7 +151,9 @@ "summaryUploaded": "{{count}} uploaded", "summaryWarnings": "{{count}} warning(s)", "summaryUnchanged": "{{count}} unchanged", - "summaryFailed": "{{count}} failed" + "summaryFailed": "{{count}} failed", + "securityViolations": "Security violations:", + "violationLine": "Line {{line}}: {{reason}}" }, "edit": { "title": "Edit Skill", diff --git a/ui/web/src/i18n/locales/ko/skills.json b/ui/web/src/i18n/locales/ko/skills.json index d92e98db..7133503e 100644 --- a/ui/web/src/i18n/locales/ko/skills.json +++ b/ui/web/src/i18n/locales/ko/skills.json @@ -46,7 +46,9 @@ "validCount": "{{total}}개 중 {{valid}}개 유효", "uploadCount": "업로드 ({{count}}개)", "successCount": "{{total}}개 중 {{success}}개 성공적으로 업로드됨", - "remove": "파일 제거" + "remove": "파일 제거", + "securityViolations": "보안 위반:", + "violationLine": "{{line}}번 줄: {{reason}}" }, "edit": { "title": "스킬 편집", diff --git a/ui/web/src/i18n/locales/vi/skills.json b/ui/web/src/i18n/locales/vi/skills.json index cc871d6e..f6aa1da0 100644 --- a/ui/web/src/i18n/locales/vi/skills.json +++ b/ui/web/src/i18n/locales/vi/skills.json @@ -101,7 +101,9 @@ "summaryUploaded": "{{count}} đã tải lên", "summaryWarnings": "{{count}} cảnh báo", "summaryUnchanged": "{{count}} không đổi", - "summaryFailed": "{{count}} thất bại" + "summaryFailed": "{{count}} thất bại", + "securityViolations": "Vi phạm bảo mật:", + "violationLine": "Dòng {{line}}: {{reason}}" }, "edit": { "title": "Chỉnh sửa skill", diff --git a/ui/web/src/i18n/locales/zh/skills.json b/ui/web/src/i18n/locales/zh/skills.json index 5effbd72..8d09cd79 100644 --- a/ui/web/src/i18n/locales/zh/skills.json +++ b/ui/web/src/i18n/locales/zh/skills.json @@ -101,7 +101,9 @@ "summaryUploaded": "已上传 {{count}} 个", "summaryWarnings": "{{count}} 个警告", "summaryUnchanged": "{{count}} 个未更改", - "summaryFailed": "{{count}} 个失败" + "summaryFailed": "{{count}} 个失败", + "securityViolations": "安全违规：", + "violationLine": "第 {{line}} 行：{{reason}}" }, "edit": { "title": "编辑Skill", diff --git a/ui/web/src/pages/skills/lib/skill-upload-types.ts b/ui/web/src/pages/skills/lib/skill-upload-types.ts index 5295fe34..823d5d3c 100644 --- a/ui/web/src/pages/skills/lib/skill-upload-types.ts +++ b/ui/web/src/pages/skills/lib/skill-upload-types.ts @@ -19,6 +19,8 @@ export interface SkillEntry { slug?: string; contentHash?: string; error?: string; + /** Structured security violations (line number + reason) from failed upload. */ + violations?: Array<{ line: number; reason: string }>; } export interface FileEntry { diff --git a/ui/web/src/pages/skills/skill-upload-dialog.tsx b/ui/web/src/pages/skills/skill-upload-dialog.tsx index 32ef6b74..501e406c 100644 --- a/ui/web/src/pages/skills/skill-upload-dialog.tsx +++ b/ui/web/src/pages/skills/skill-upload-dialog.tsx @@ -1,6 +1,7 @@ import { useEffect, useState, useRef } from "react"; import { useTranslation } from "react-i18next"; import { Upload } from "lucide-react"; +import { ApiError } from "@/api/errors"; import { Dialog, DialogContent, @@ -188,6 +189,17 @@ export function SkillUploadDialog({ open, onOpenChange, onUpload }: SkillUploadD ), ); } catch (err) { + // Extract violations if present (from security scan failure) + let violations: Array<{ line: number; reason: string }> | undefined; + let errorMsg = t("upload.failed"); + + if (err instanceof ApiError) { + errorMsg = err.message; + violations = err.getViolations() ?? undefined; + } else if (err instanceof Error) { + errorMsg = err.message; + } + setEntries((prev) => prev.map((e) => e.id === fileEntry.id @@ -198,7 +210,8 @@ export function SkillUploadDialog({ open, onOpenChange, onUpload }: SkillUploadD ? { ...s, status: "error" as SkillStatus, - error: err instanceof Error ? err.message : t("upload.failed"), + error: errorMsg, + violations, } : s, ), diff --git a/ui/web/src/pages/skills/skill-upload-entry.tsx b/ui/web/src/pages/skills/skill-upload-entry.tsx index 39a60817..f4c7cac8 100644 --- a/ui/web/src/pages/skills/skill-upload-entry.tsx +++ b/ui/web/src/pages/skills/skill-upload-entry.tsx @@ -153,6 +153,21 @@ export function SkillSubtitle({ t: TFunc; }) { if (skill.status === "invalid" || skill.status === "error") { + // Display security violations as structured list if available + if (skill.violations && skill.violations.length > 0) { + return ( +
+

{t("upload.securityViolations")}

+
    + {skill.violations.map((v, i) => ( +
  • + {t("upload.violationLine", { line: v.line, reason: v.reason })} +
    • + ))} +
+
+ ); + } return (

{skill.error ? t(skill.error) : t("upload.failed")}