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 <noreply@anthropic.com>

* 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 <noreply@anthropic.com>

---------

Co-authored-by: Bruno Clermont <bruno.clermont@gmail.com>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
authored and GitHub committed 2026-06-29 20:54:30 +07:00
1 parent 969a5879ae
commit 978a0f255e
10 files changed
+66 -7

No files matched your search

+12 -1
View File
@@ -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
}
+8
View File
@@ -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<string, unknown>).violations;
if (!Array.isArray(violations)) return null;
return violations as Array<{ line: number; reason: string }>;
}
}
+3 -1
View File
@@ -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<T>(res);
+3 -1
View File
@@ -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",
+3 -1
View File
@@ -46,7 +46,9 @@
"validCount": "{{total}}개 중 {{valid}}개 유효",
"uploadCount": "업로드 ({{count}}개)",
"successCount": "{{total}}개 중 {{success}}개 성공적으로 업로드됨",
"remove": "파일 제거"
"remove": "파일 제거",
"securityViolations": "보안 위반:",
"violationLine": "{{line}}번 줄: {{reason}}"
},
"edit": {
"title": "스킬 편집",
+3 -1
View File
@@ -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",
+3 -1
View File
@@ -101,7 +101,9 @@
"summaryUploaded": "已上传 {{count}} 个",
"summaryWarnings": "{{count}} 个警告",
"summaryUnchanged": "{{count}} 个未更改",
"summaryFailed": "{{count}} 个失败"
"summaryFailed": "{{count}} 个失败",
"securityViolations": "安全违规:",
"violationLine": "第 {{line}} 行:{{reason}}"
},
"edit": {
"title": "编辑Skill",
@@ -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 {
@@ -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,
),
@@ -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 (
<div className="text-xs text-destructive space-y-0.5">
<p className="font-medium">{t("upload.securityViolations")}</p>
<ul className="list-disc list-inside space-y-0.5 pl-0">
{skill.violations.map((v, i) => (
<li key={i} className="text-destructive">
{t("upload.violationLine", { line: v.line, reason: v.reason })}
</li>
))}
</ul>
</div>
);
}
return (
<p className="text-xs text-destructive truncate">
{skill.error ? t(skill.error) : t("upload.failed")}