07e5e70c9b
---ci---
project: ci
phase: 4
milestone: v0.8
status: complete
decisions:
- id: D-031
decision: 3-persona quality review: security, performance, maintainability
rationale: Each persona detects different class of issues; aggregate gives complete picture
confidence: 0.82
- id: D-032
decision: L4 P0>0 = fail (not P0>3); P1 = warning (not pass)
rationale: Any P0 finding is critical; P1 findings should never pass silently
confidence: 0.95
requirements:
covered: [QUAL-01, QUAL-02, QUAL-03, QUAL-04, QUAL-05]
---/ci---
QUAL-01: Added 3-persona review with distinct pattern sets: SecurityReviewer
(injection, auth, crypto), PerformanceReviewer (sync I/O, timer leaks,
DoS), MaintainabilityReviewer (type safety, dead code, tech debt).
QUAL-02: CodeReviewerAgent fleshed with mechanical 3-persona review. Works
without backend by running regex-based scan across all personas.
QUAL-03: L4 passed=false when ANY P0 finding exists (was >3). P1 findings
now return status='warning' (was always 'pass').
QUAL-04: TypeScript strict mode check remains in quality layer.
QUAL-05: CodeReviewerAgent.mechanicalReview() provides regex-based review
as fallback when no backend is available.
428 lines
14 KiB
TypeScript
428 lines
14 KiB
TypeScript
import * as fs from "node:fs";
|
|
import * as path from "node:path";
|
|
import { execSync } from "node:child_process";
|
|
import { VerificationLayer, VerificationResult, VerificationCheck } from "./types.js";
|
|
|
|
interface CodeFinding {
|
|
severity: "P0" | "P1" | "P2" | "P3";
|
|
category: string;
|
|
persona: "security" | "performance" | "maintainability";
|
|
message: string;
|
|
file?: string;
|
|
}
|
|
|
|
const SECURITY_REVIEW_PATTERNS: Array<{
|
|
pattern: RegExp;
|
|
severity: "P0" | "P1" | "P2";
|
|
category: string;
|
|
message: string;
|
|
}> = [
|
|
{
|
|
pattern: /(?:exec|execSync|spawn|spawnSync)\s*\(\s*[^'"]*[\$`]/g,
|
|
severity: "P0",
|
|
category: "command_injection",
|
|
message: "Command execution with dynamic input — injection risk",
|
|
},
|
|
{
|
|
pattern: /eval\s*\(\s*[^'"]*\$\{/g,
|
|
severity: "P0",
|
|
category: "code_injection",
|
|
message: "eval() with dynamic content — code injection risk",
|
|
},
|
|
{
|
|
pattern: /(?:innerHTML|outerHTML|insertAdjacentHTML)\s*=/g,
|
|
severity: "P0",
|
|
category: "xss",
|
|
message: "Unsanitized HTML assignment — XSS risk",
|
|
},
|
|
{
|
|
pattern: /(?:password|secret|api[_-]?key|token)\s*[:=]\s*['"][^'"]{3,}['"]/gi,
|
|
severity: "P0",
|
|
category: "credential_exposure",
|
|
message: "Hardcoded credential in source",
|
|
},
|
|
{
|
|
pattern: /(?:__proto__|constructor\s*\[|prototype\s*\[)/g,
|
|
severity: "P0",
|
|
category: "prototype_pollution",
|
|
message: "Prototype chain manipulation — privilege escalation risk",
|
|
},
|
|
{
|
|
pattern: /jwt\.decode\s*\(/g,
|
|
severity: "P0",
|
|
category: "auth_bypass",
|
|
message: "JWT decoded without verification — authentication bypass",
|
|
},
|
|
{
|
|
pattern: /(?:md5|sha1|des|rc4)\s*\(/gi,
|
|
severity: "P1",
|
|
category: "weak_crypto",
|
|
message: "Weak cryptographic algorithm",
|
|
},
|
|
{
|
|
pattern: /JSON\.parse\s*\(\s*(?:req|ctx|input|data|body|params)\.\w+/g,
|
|
severity: "P1",
|
|
category: "unsafe_deserialization",
|
|
message: "Unsafe deserialization of untrusted input",
|
|
},
|
|
{
|
|
pattern: /catch\s*\(\w*\)\s*\{\s*\}/g,
|
|
severity: "P0",
|
|
category: "swallowed_errors",
|
|
message: "Empty catch block — errors silently swallowed",
|
|
},
|
|
];
|
|
|
|
const PERFORMANCE_REVIEW_PATTERNS: Array<{
|
|
pattern: RegExp;
|
|
severity: "P1" | "P2";
|
|
category: string;
|
|
message: string;
|
|
}> = [
|
|
{
|
|
pattern: /await\s+.*(?:readFileSync|writeFileSync|execSync)/g,
|
|
severity: "P1",
|
|
category: "blocking_io",
|
|
message: "Synchronous I/O in async context — blocks event loop",
|
|
},
|
|
{
|
|
pattern: /(?:execSync|spawnSync)\s*\(\s*['"]/g,
|
|
severity: "P1",
|
|
category: "sync_exec",
|
|
message: "Synchronous process spawn — blocks event loop",
|
|
},
|
|
{
|
|
pattern: /setTimeout\s*\((?![^)]*clearTimeout)/g,
|
|
severity: "P2",
|
|
category: "timer_leak",
|
|
message: "setTimeout without clearTimeout — potential timer leak",
|
|
},
|
|
{
|
|
pattern: /\.(?:on|addEventListener)\s*\(['"]\w+['"]/g,
|
|
severity: "P2",
|
|
category: "listener_leak",
|
|
message: "Event listener registration — verify corresponding .off() exists",
|
|
},
|
|
{
|
|
pattern: /\.map\s*\(\s*(?:async\s+)?\([^)]*\)\s*=>\s*(?!.*(?:filter|slice|take|limit))/g,
|
|
severity: "P2",
|
|
category: "unbounded_iteration",
|
|
message: "Full array traversal without pagination or limit",
|
|
},
|
|
{
|
|
pattern: /express\.json\s*\(\s*\)/g,
|
|
severity: "P1",
|
|
category: "no_body_limit",
|
|
message: "JSON body parser without size limit — DoS risk",
|
|
},
|
|
];
|
|
|
|
const MAINTAINABILITY_REVIEW_PATTERNS: Array<{
|
|
pattern: RegExp;
|
|
severity: "P1" | "P2" | "P3";
|
|
category: string;
|
|
message: string;
|
|
}> = [
|
|
{
|
|
pattern: /(?:as\s+any\b|:\s*any\b|<any>|any\[\s*\])/g,
|
|
severity: "P1",
|
|
category: "type_safety",
|
|
message: "Use of 'any' type — loses type safety",
|
|
},
|
|
{
|
|
pattern: /\bvar\s+/g,
|
|
severity: "P1",
|
|
category: "modern_js",
|
|
message: "Use of 'var' — prefer 'const' or 'let'",
|
|
},
|
|
{
|
|
pattern: /\b(?:TODO|FIXME|HACK|XXX)\b/g,
|
|
severity: "P2",
|
|
category: "tech_debt",
|
|
message: "Technical debt marker found",
|
|
},
|
|
{
|
|
pattern: /console\.(log|warn|error)\s*\(/g,
|
|
severity: "P2",
|
|
category: "logging",
|
|
message: "Direct console.log usage — consider structured logging",
|
|
},
|
|
{
|
|
pattern: /(?:return|throw)\s+[^;]+;\s*\n\s*(?:return|throw|const|let|var|function)/g,
|
|
severity: "P1",
|
|
category: "dead_code",
|
|
message: "Code after return/throw — unreachable dead code",
|
|
},
|
|
];
|
|
|
|
export class QualityVerification extends VerificationLayer {
|
|
readonly layer = 4;
|
|
readonly name = "Code Quality";
|
|
|
|
async verify(projectPath: string, phase: number): Promise<VerificationResult> {
|
|
const start = Date.now();
|
|
const checks: VerificationCheck[] = [];
|
|
|
|
const securityFindings = this.scanWithPersona(projectPath, SECURITY_REVIEW_PATTERNS, "security");
|
|
const perfFindings = this.scanWithPersona(projectPath, PERFORMANCE_REVIEW_PATTERNS, "performance");
|
|
const maintFindings = this.scanWithPersona(projectPath, MAINTAINABILITY_REVIEW_PATTERNS, "maintainability");
|
|
const allFindings = [...securityFindings, ...perfFindings, ...maintFindings];
|
|
|
|
const p0Findings = allFindings.filter((f) => f.severity === "P0");
|
|
const p1Findings = allFindings.filter((f) => f.severity === "P1");
|
|
const p2p3Findings = allFindings.filter((f) => f.severity === "P2" || f.severity === "P3");
|
|
|
|
checks.push(this.checkP0Findings(p0Findings));
|
|
checks.push(this.checkP1Findings(p1Findings));
|
|
checks.push(this.checkP2P3Findings(p2p3Findings));
|
|
checks.push(this.checkSecurityReview(securityFindings));
|
|
checks.push(this.checkPerformanceReview(perfFindings));
|
|
checks.push(this.checkMaintainabilityReview(maintFindings));
|
|
checks.push(this.checkTypeScriptStrictness(projectPath));
|
|
checks.push(this.checkConsistentNaming(projectPath));
|
|
checks.push(this.checkTypeScriptCompilation(projectPath));
|
|
|
|
const hasP0Fail = p0Findings.length > 0;
|
|
const passed = !hasP0Fail;
|
|
|
|
return {
|
|
layer: this.layer,
|
|
name: this.name,
|
|
passed,
|
|
checks,
|
|
summary: `${allFindings.length} findings across 3 personas (P0: ${p0Findings.length}, P1: ${p1Findings.length}, P2/P3: ${p2p3Findings.length})`,
|
|
duration_ms: Date.now() - start,
|
|
};
|
|
}
|
|
|
|
private scanWithPersona(
|
|
projectPath: string,
|
|
patterns: Array<{ pattern: RegExp; severity: "P0" | "P1" | "P2" | "P3"; category: string; message: string }>,
|
|
persona: CodeFinding["persona"]
|
|
): CodeFinding[] {
|
|
const findings: CodeFinding[] = [];
|
|
const srcDir = path.join(projectPath, "src");
|
|
|
|
if (!fs.existsSync(srcDir)) {
|
|
return findings;
|
|
}
|
|
|
|
this.scanDirectory(srcDir, projectPath, patterns, persona, findings);
|
|
return findings;
|
|
}
|
|
|
|
private scanDirectory(
|
|
dir: string,
|
|
projectPath: string,
|
|
patterns: Array<{ pattern: RegExp; severity: "P0" | "P1" | "P2" | "P3"; category: string; message: string }>,
|
|
persona: CodeFinding["persona"],
|
|
findings: CodeFinding[]
|
|
): void {
|
|
const entries = fs.readdirSync(dir, { withFileTypes: true });
|
|
for (const entry of entries) {
|
|
const fullPath = path.join(dir, entry.name);
|
|
if (entry.isDirectory() && entry.name !== "node_modules") {
|
|
this.scanDirectory(fullPath, projectPath, patterns, persona, findings);
|
|
} else if (
|
|
entry.isFile() &&
|
|
entry.name.endsWith(".ts") &&
|
|
!entry.name.endsWith(".test.ts") &&
|
|
!entry.name.endsWith(".d.ts")
|
|
) {
|
|
const content = fs.readFileSync(fullPath, "utf-8");
|
|
for (const { pattern, severity, category, message } of patterns) {
|
|
pattern.lastIndex = 0;
|
|
if (pattern.test(content)) {
|
|
findings.push({
|
|
severity,
|
|
category,
|
|
persona,
|
|
message: `${message} (${path.relative(projectPath, fullPath)})`,
|
|
file: path.relative(projectPath, fullPath),
|
|
});
|
|
}
|
|
}
|
|
}
|
|
}
|
|
}
|
|
|
|
private checkP0Findings(p0Findings: CodeFinding[]): VerificationCheck {
|
|
if (p0Findings.length === 0) {
|
|
return this.check(
|
|
"P0 findings (auto-fix)",
|
|
"pass",
|
|
"No critical code quality issues found"
|
|
);
|
|
}
|
|
return this.check(
|
|
"P0 findings (auto-fix)",
|
|
"fail",
|
|
`${p0Findings.length} P0 finding(s) — must be fixed`,
|
|
p0Findings.map((f) => `[${f.persona}|${f.category}] ${f.message}`).join("\n")
|
|
);
|
|
}
|
|
|
|
private checkP1Findings(p1Findings: CodeFinding[]): VerificationCheck {
|
|
if (p1Findings.length === 0) {
|
|
return this.check(
|
|
"P1 findings (review)",
|
|
"pass",
|
|
"No P1 findings"
|
|
);
|
|
}
|
|
return this.check(
|
|
"P1 findings (review)",
|
|
"warning",
|
|
`${p1Findings.length} P1 finding(s) flagged for post-hoc review`,
|
|
p1Findings.map((f) => `[${f.persona}|${f.category}] ${f.message}`).join("\n")
|
|
);
|
|
}
|
|
|
|
private checkP2P3Findings(findings: CodeFinding[]): VerificationCheck {
|
|
if (findings.length === 0) {
|
|
return this.check(
|
|
"P2/P3 findings (informational)",
|
|
"pass",
|
|
"No P2/P3 findings"
|
|
);
|
|
}
|
|
return this.check(
|
|
"P2/P3 findings (informational)",
|
|
"pass",
|
|
`${findings.length} informational finding(s)`,
|
|
findings.map((f) => `[${f.persona}|${f.category}] ${f.message}`).join("\n")
|
|
);
|
|
}
|
|
|
|
private checkSecurityReview(findings: CodeFinding[]): VerificationCheck {
|
|
if (findings.length === 0) {
|
|
return this.check("Security persona review", "pass", "No security review findings");
|
|
}
|
|
const p0 = findings.filter((f) => f.severity === "P0").length;
|
|
return this.check(
|
|
"Security persona review",
|
|
p0 > 0 ? "fail" : "warning",
|
|
`${findings.length} finding(s) from security reviewer (P0: ${p0})`,
|
|
findings.map((f) => `[${f.category}] ${f.message}`).join("\n")
|
|
);
|
|
}
|
|
|
|
private checkPerformanceReview(findings: CodeFinding[]): VerificationCheck {
|
|
if (findings.length === 0) {
|
|
return this.check("Performance persona review", "pass", "No performance review findings");
|
|
}
|
|
return this.check(
|
|
"Performance persona review",
|
|
"warning",
|
|
`${findings.length} finding(s) from performance reviewer`,
|
|
findings.map((f) => `[${f.category}] ${f.message}`).join("\n")
|
|
);
|
|
}
|
|
|
|
private checkMaintainabilityReview(findings: CodeFinding[]): VerificationCheck {
|
|
if (findings.length === 0) {
|
|
return this.check("Maintainability persona review", "pass", "No maintainability review findings");
|
|
}
|
|
return this.check(
|
|
"Maintainability persona review",
|
|
"pass",
|
|
`${findings.length} finding(s) from maintainability reviewer`,
|
|
findings.map((f) => `[${f.category}] ${f.message}`).join("\n")
|
|
);
|
|
}
|
|
|
|
private checkTypeScriptStrictness(projectPath: string): VerificationCheck {
|
|
const tsconfigPath = path.join(projectPath, "tsconfig.json");
|
|
if (!fs.existsSync(tsconfigPath)) {
|
|
return this.check("TypeScript strictness", "warning", "No tsconfig.json found");
|
|
}
|
|
|
|
const content = fs.readFileSync(tsconfigPath, "utf-8");
|
|
const jsonContent = content.replace(/\/\/.*$/gm, "").replace(/\/\*[\s\S]*?\*\//g, "");
|
|
|
|
try {
|
|
const tsconfig = JSON.parse(jsonContent);
|
|
const strict = tsconfig.compilerOptions?.strict;
|
|
const noImplicitAny = tsconfig.compilerOptions?.noImplicitAny;
|
|
|
|
if (strict) {
|
|
return this.check("TypeScript strictness", "pass", "TypeScript strict mode is enabled");
|
|
}
|
|
if (noImplicitAny) {
|
|
return this.check("TypeScript strictness", "pass", "noImplicitAny is enabled");
|
|
}
|
|
return this.check(
|
|
"TypeScript strictness",
|
|
"warning",
|
|
"TypeScript strict mode is not enabled — consider enabling for better type safety"
|
|
);
|
|
} catch {
|
|
return this.check("TypeScript strictness", "warning", "Could not parse tsconfig.json");
|
|
}
|
|
}
|
|
|
|
private checkConsistentNaming(projectPath: string): VerificationCheck {
|
|
const srcDir = path.join(projectPath, "src");
|
|
if (!fs.existsSync(srcDir)) {
|
|
return this.check("Consistent naming", "skipped", "No src/ directory found");
|
|
}
|
|
|
|
const tsFiles: string[] = [];
|
|
this.collectFiles(srcDir, tsFiles);
|
|
|
|
const kebabCaseFiles = tsFiles.filter((f) => path.basename(f).includes("-") && !path.basename(f).includes(".test."));
|
|
const camelCaseFiles = tsFiles.filter((f) => /^[a-z][a-zA-Z0-9]*\.ts$/.test(path.basename(f)) && !path.basename(f).includes("-"));
|
|
const pascalCaseFiles = tsFiles.filter((f) => /^[A-Z]/.test(path.basename(f)));
|
|
|
|
const conventions: string[] = [];
|
|
if (kebabCaseFiles.length > camelCaseFiles.length && kebabCaseFiles.length > pascalCaseFiles.length) {
|
|
conventions.push("kebab-case dominant");
|
|
} else if (camelCaseFiles.length > 0) {
|
|
conventions.push("camelCase files present");
|
|
}
|
|
|
|
return this.check(
|
|
"Consistent naming",
|
|
"pass",
|
|
`${tsFiles.length} source files, naming conventions: ${conventions.join(", ") || "mixed"}`
|
|
);
|
|
}
|
|
|
|
private checkTypeScriptCompilation(projectPath: string): VerificationCheck {
|
|
const tsconfigPath = path.join(projectPath, "tsconfig.json");
|
|
if (!fs.existsSync(tsconfigPath)) {
|
|
return this.check("TypeScript compilation", "skipped", "No tsconfig.json found");
|
|
}
|
|
|
|
try {
|
|
execSync("npx tsc --noEmit 2>&1", {
|
|
cwd: projectPath,
|
|
encoding: "utf-8",
|
|
timeout: 60000,
|
|
stdio: ["pipe", "pipe", "pipe"],
|
|
});
|
|
return this.check("TypeScript compilation", "pass", "TypeScript compiles without errors");
|
|
} catch (err) {
|
|
const execErr = err as { stdout?: string };
|
|
const output = execErr.stdout || "";
|
|
const errorCount = (output.match(/error TS/g) || []).length;
|
|
return this.check(
|
|
"TypeScript compilation",
|
|
errorCount > 5 ? "fail" : "warning",
|
|
`${errorCount} TypeScript compilation error(s)`
|
|
);
|
|
}
|
|
}
|
|
|
|
private collectFiles(dir: string, files: string[]): void {
|
|
const entries = fs.readdirSync(dir, { withFileTypes: true });
|
|
for (const entry of entries) {
|
|
const fullPath = path.join(dir, entry.name);
|
|
if (entry.isDirectory() && entry.name !== "node_modules") {
|
|
this.collectFiles(fullPath, files);
|
|
} else if (entry.name.endsWith(".ts") && !entry.name.endsWith(".d.ts")) {
|
|
files.push(fullPath);
|
|
}
|
|
}
|
|
}
|
|
} |