fix(P01): add Zod BackendResult validation and fix opencode silent success
---ci---
project: ci
phase: 1
milestone: v0.8
status: in_progress
decisions:
- id: D-022
decision: Validate BackendResult at boundary with Zod schema
rationale: External backend output is untrusted; runtime validation prevents corrupt commit streams
confidence: 0.92
- id: D-023
decision: opencode parseResult returns success:false on malformed JSON
rationale: Silent success:true on parse failure masks backend errors; fail loudly instead
confidence: 0.95
requirements:
covered: [FIX-02, FIX-03]
---/ci---
FIX-02: Add Zod BackendResultSchema and validateBackendResult() in
backends/types.ts. backendResultToAgentResult() in base.ts now validates
before passing through. Invalid results produce success:false with error
detail. Path traversal protection: artifact paths with '..' or leading '/'
are rejected.
FIX-03: opencode.ts parseResult() no longer defaults to success:true when
JSON parsing fails entirely. Both the inner parse error and the no-JSON
match case now return emptyBackendResult() with descriptive error messages.
This commit is contained in:
+13
-1
@@ -1,4 +1,4 @@
|
|||||||
import { IntelligenceBackend, BackendRequest, BackendResult, BackendUnavailableError, emptyBackendResult } from "../backends/types.js";
|
import { IntelligenceBackend, BackendRequest, BackendResult, BackendUnavailableError, emptyBackendResult, validateBackendResult } from "../backends/types.js";
|
||||||
import { AgentName, AutonomyLevel } from "../types/config.js";
|
import { AgentName, AutonomyLevel } from "../types/config.js";
|
||||||
|
|
||||||
export interface AgentResult {
|
export interface AgentResult {
|
||||||
@@ -21,6 +21,18 @@ export interface AgentContext {
|
|||||||
}
|
}
|
||||||
|
|
||||||
export function backendResultToAgentResult(result: BackendResult): AgentResult {
|
export function backendResultToAgentResult(result: BackendResult): AgentResult {
|
||||||
|
const validation = validateBackendResult(result);
|
||||||
|
if (!validation.result) {
|
||||||
|
return {
|
||||||
|
success: false,
|
||||||
|
output: "",
|
||||||
|
artifacts_created: [],
|
||||||
|
decisions: 0,
|
||||||
|
escalations: 0,
|
||||||
|
duration_ms: 0,
|
||||||
|
error: `BackendResult validation failed: ${validation.errors.join("; ")}`,
|
||||||
|
};
|
||||||
|
}
|
||||||
return {
|
return {
|
||||||
success: result.success,
|
success: result.success,
|
||||||
output: result.output,
|
output: result.output,
|
||||||
|
|||||||
+12
-14
@@ -117,8 +117,14 @@ export class OpencodeBackend implements IntelligenceBackend {
|
|||||||
if (jsonMatch) {
|
if (jsonMatch) {
|
||||||
try {
|
try {
|
||||||
const parsed = JSON.parse(jsonMatch[0]);
|
const parsed = JSON.parse(jsonMatch[0]);
|
||||||
|
if (typeof parsed.success !== "boolean") {
|
||||||
|
return emptyBackendResult(`Backend returned non-boolean success field: ${typeof parsed.success}`);
|
||||||
|
}
|
||||||
|
if (parsed.success === false && !parsed.error && !parsed.output) {
|
||||||
|
return emptyBackendResult("Backend returned failure with no error or output");
|
||||||
|
}
|
||||||
return {
|
return {
|
||||||
success: parsed.success ?? true,
|
success: parsed.success,
|
||||||
output: parsed.output || output,
|
output: parsed.output || output,
|
||||||
artifacts: Array.isArray(parsed.artifacts)
|
artifacts: Array.isArray(parsed.artifacts)
|
||||||
? parsed.artifacts.filter((a: unknown) => !!a).map((a: Record<string, unknown>) => ({
|
? parsed.artifacts.filter((a: unknown) => !!a).map((a: Record<string, unknown>) => ({
|
||||||
@@ -156,7 +162,7 @@ export class OpencodeBackend implements IntelligenceBackend {
|
|||||||
options: Array.isArray(e.options) ? e.options : [],
|
options: Array.isArray(e.options) ? e.options : [],
|
||||||
default_option_id: String(e.default_option_id || ""),
|
default_option_id: String(e.default_option_id || ""),
|
||||||
resolution: (e.resolution as "approved" | "rejected" | "modified" | "pending" | "timeout_auto_proceed") || "pending",
|
resolution: (e.resolution as "approved" | "rejected" | "modified" | "pending" | "timeout_auto_proceed") || "pending",
|
||||||
audit_file: String(e.audit_file || ""),
|
commit_hash: String(e.commit_hash || ""),
|
||||||
}))
|
}))
|
||||||
: [],
|
: [],
|
||||||
usage: parsed.usage || {
|
usage: parsed.usage || {
|
||||||
@@ -164,19 +170,11 @@ export class OpencodeBackend implements IntelligenceBackend {
|
|||||||
total_tokens: Math.ceil(output.length / 4),
|
total_tokens: Math.ceil(output.length / 4),
|
||||||
},
|
},
|
||||||
};
|
};
|
||||||
} catch {}
|
} catch {
|
||||||
|
return emptyBackendResult(`Backend output contained JSON-like structure but failed to parse: ${output.slice(0, 200)}`);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
return {
|
return emptyBackendResult(`Backend output did not contain valid JSON result: ${output.slice(0, 200)}`);
|
||||||
success: true,
|
|
||||||
output,
|
|
||||||
artifacts: [],
|
|
||||||
decisions: [],
|
|
||||||
escalations: [],
|
|
||||||
usage: {
|
|
||||||
...emptyTokenUsage(),
|
|
||||||
total_tokens: Math.ceil(output.length / 4),
|
|
||||||
},
|
|
||||||
};
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -1,3 +1,4 @@
|
|||||||
|
import { z } from "zod";
|
||||||
import { AgentName, AutonomyLevel, ModelProfile } from "../types/config.js";
|
import { AgentName, AutonomyLevel, ModelProfile } from "../types/config.js";
|
||||||
import { AgentContext } from "../agents/base.js";
|
import { AgentContext } from "../agents/base.js";
|
||||||
import { Decision } from "../types/decisions.js";
|
import { Decision } from "../types/decisions.js";
|
||||||
@@ -5,6 +6,55 @@ import { Escalation } from "../types/escalation.js";
|
|||||||
|
|
||||||
export type BackendType = "llm" | "agent";
|
export type BackendType = "llm" | "agent";
|
||||||
|
|
||||||
|
export const ArtifactSchema = z.object({
|
||||||
|
path: z.string().min(1, "Artifact path must not be empty"),
|
||||||
|
content: z.string(),
|
||||||
|
operation: z.enum(["create", "update", "delete"]),
|
||||||
|
});
|
||||||
|
|
||||||
|
export const TokenUsageSchema = z.object({
|
||||||
|
input_tokens: z.number().min(0),
|
||||||
|
output_tokens: z.number().min(0),
|
||||||
|
total_tokens: z.number().min(0),
|
||||||
|
estimated_cost_usd: z.number().min(0),
|
||||||
|
});
|
||||||
|
|
||||||
|
export const BackendResultSchema = z.object({
|
||||||
|
success: z.boolean(),
|
||||||
|
output: z.string(),
|
||||||
|
artifacts: z.array(ArtifactSchema),
|
||||||
|
decisions: z.array(z.unknown()),
|
||||||
|
escalations: z.array(z.unknown()),
|
||||||
|
usage: TokenUsageSchema,
|
||||||
|
error: z.string().optional(),
|
||||||
|
}).refine(
|
||||||
|
(r) => !(r.success === true && r.error && r.error.length > 0),
|
||||||
|
{ message: "Result cannot be both success and have an error message" }
|
||||||
|
);
|
||||||
|
|
||||||
|
export function validateBackendResult(raw: unknown): { result: BackendResult | null; errors: string[] } {
|
||||||
|
const parseResult = BackendResultSchema.safeParse(raw);
|
||||||
|
if (!parseResult.success) {
|
||||||
|
return {
|
||||||
|
result: null,
|
||||||
|
errors: parseResult.error.errors.map((e) => `${e.path.join(".")}: ${e.message}`),
|
||||||
|
};
|
||||||
|
}
|
||||||
|
const data = parseResult.data;
|
||||||
|
if (!Array.isArray(data.artifacts)) {
|
||||||
|
return { result: null, errors: ["artifacts: expected array"] };
|
||||||
|
}
|
||||||
|
for (const a of data.artifacts) {
|
||||||
|
if (a.path.includes("..")) {
|
||||||
|
return { result: null, errors: [`artifacts: path "${a.path}" contains ".." (path traversal risk)`] };
|
||||||
|
}
|
||||||
|
if (a.path.startsWith("/")) {
|
||||||
|
return { result: null, errors: [`artifacts: path "${a.path}" is absolute (must be relative)`] };
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return { result: data as BackendResult, errors: [] };
|
||||||
|
}
|
||||||
|
|
||||||
export interface BackendRequest {
|
export interface BackendRequest {
|
||||||
persona: AgentName;
|
persona: AgentName;
|
||||||
workflow: string;
|
workflow: string;
|
||||||
|
|||||||
Reference in New Issue
Block a user