mirror of
https://github.com/KeygraphHQ/shannon.git
synced 2026-08-15 16:00:29 +02:00
feat(worker): record severity in analysis mode and fix prompt substitutions (#413)
* feat(worker): record severity in analysis mode alongside confidence * fix(worker): align add_finding severity with the exploit collector's four levels * refactor(worker): drop the dead REPORT_VULN_HEADING substitution No prompt in the tree uses the placeholder, so the replacement was a no-op on every render. * fix(worker): strip all whitespace from TOTP secrets, not just the ends * fix(worker): render rule type and value in the agent prompt * refactor(worker): drop the dead vuln-summary subsection substitution
This commit is contained in:
@@ -37,7 +37,7 @@ const OWASP_CATEGORY_VALUES = [
|
||||
'A10:2025 — Mishandling of Exceptional Conditions',
|
||||
] as const;
|
||||
|
||||
const SEVERITY_VALUES = ['critical', 'high', 'medium', 'low', 'informational'] as const;
|
||||
const SEVERITY_VALUES = ['critical', 'high', 'medium', 'low'] as const;
|
||||
const STATUS_VALUES = ['exploited', 'out_of_scope', 'blocked_by_constraints', 'false_positive'] as const;
|
||||
const CONFIDENCE_VALUES = ['high', 'medium', 'low'] as const;
|
||||
|
||||
@@ -117,8 +117,18 @@ const AdditionalSectionSchema = Type.Object({
|
||||
}),
|
||||
});
|
||||
|
||||
function identityFields() {
|
||||
/**
|
||||
* `severity` is recorded in both modes, but it does not mean the same thing in each: an exploit
|
||||
* run measures it from what the exploit demonstrated, an analysis run assesses it from the class
|
||||
* of flaw. The description says which, so the agent never presents an assessment as a measurement.
|
||||
*/
|
||||
function identityFields(exploit: boolean) {
|
||||
const severityDescription = exploit
|
||||
? 'Severity of the finding, based on the impact the exploit demonstrated.'
|
||||
: 'Severity of the finding, assessed from the vulnerability class and the impact it would have.';
|
||||
|
||||
return {
|
||||
severity: stringEnum(SEVERITY_VALUES, { description: severityDescription }),
|
||||
finding_id: Type.String({
|
||||
minLength: 1,
|
||||
description: 'Finding identifier (e.g., "AUTH-VULN-07", "INJ-VULN-03"). Must be unique per report.',
|
||||
@@ -178,9 +188,6 @@ function narrativeFields(exploit: boolean) {
|
||||
/** Fields that only mean something once an exploit has run. Absent from the analysis schema. */
|
||||
function exploitOnlyFields() {
|
||||
return {
|
||||
severity: stringEnum(SEVERITY_VALUES, {
|
||||
description: 'Severity of the finding, based on the impact the exploit demonstrated.',
|
||||
}),
|
||||
auth_state: Type.String({
|
||||
minLength: 1,
|
||||
description: 'Authentication state during testing (e.g., "Unauthenticated", "Any authenticated user").',
|
||||
@@ -205,7 +212,7 @@ function exploitOnlyFields() {
|
||||
};
|
||||
}
|
||||
|
||||
/** Replaces `severity` when nothing was exploited. */
|
||||
/** Accompanies `severity` when nothing was exploited — the rating the analysis deliverable itself carries. */
|
||||
function analysisOnlyFields() {
|
||||
return {
|
||||
confidence: stringEnum(CONFIDENCE_VALUES, {
|
||||
@@ -233,7 +240,7 @@ function sharedOptionalFields() {
|
||||
|
||||
export function buildAddFindingSchema(exploit: boolean) {
|
||||
return Type.Object({
|
||||
...identityFields(),
|
||||
...identityFields(exploit),
|
||||
...(exploit ? exploitOnlyFields() : analysisOnlyFields()),
|
||||
...locationFields(),
|
||||
...narrativeFields(exploit),
|
||||
@@ -243,12 +250,12 @@ export function buildAddFindingSchema(exploit: boolean) {
|
||||
|
||||
/**
|
||||
* Superset of both modes, for typing only. Consumers must check presence rather than assume:
|
||||
* `report.json` from an analysis run has no `severity` or `exploitation_steps` key at all.
|
||||
* `report.json` from an analysis run has no `exploitation_steps` key at all. `severity` is the
|
||||
* exception — both modes record it, so it is required here too.
|
||||
*/
|
||||
const AddFindingSupersetSchema = Type.Object({
|
||||
...identityFields(),
|
||||
...identityFields(true),
|
||||
code_locations: Type.Optional(Type.Array(CodeLocationSchema)),
|
||||
severity: Type.Optional(stringEnum(SEVERITY_VALUES)),
|
||||
auth_state: Type.Optional(Type.String()),
|
||||
prerequisites: Type.Optional(Type.String()),
|
||||
exploitation_steps: Type.Optional(Type.Array(StructuredStepSchema)),
|
||||
|
||||
@@ -514,7 +514,7 @@ const validateRulesSecurity = (rules: Rule[] | undefined, ruleType: string): voi
|
||||
ErrorCode.CONFIG_VALIDATION_FAILED,
|
||||
);
|
||||
}
|
||||
if (pattern.test(rule.description)) {
|
||||
if (rule.description !== undefined && pattern.test(rule.description)) {
|
||||
throw new PentestError(
|
||||
`rules.${ruleType}[${index}].description contains potentially dangerous pattern: ${pattern.source}`,
|
||||
'config',
|
||||
@@ -656,11 +656,15 @@ const checkForConflicts = (avoidRules: Rule[] = [], focusRules: Rule[] = []): vo
|
||||
};
|
||||
|
||||
const sanitizeRule = (rule: Rule): Rule => {
|
||||
return {
|
||||
description: rule.description.trim(),
|
||||
const sanitized: Rule = {
|
||||
type: rule.type.toLowerCase().trim() as Rule['type'],
|
||||
value: rule.value.trim(),
|
||||
};
|
||||
const description = rule.description?.trim();
|
||||
if (description) {
|
||||
sanitized.description = description;
|
||||
}
|
||||
return sanitized;
|
||||
};
|
||||
|
||||
export const distributeConfig = (config: Config | null): DistributedConfig => {
|
||||
@@ -702,13 +706,15 @@ const sanitizeAuthentication = (auth: Authentication): Authentication => {
|
||||
credentials: {
|
||||
username: auth.credentials.username.trim(),
|
||||
...(auth.credentials.password && { password: auth.credentials.password }),
|
||||
...(auth.credentials.totp_secret && { totp_secret: auth.credentials.totp_secret.trim() }),
|
||||
...(auth.credentials.totp_secret && {
|
||||
totp_secret: auth.credentials.totp_secret.replace(/\s/g, ''),
|
||||
}),
|
||||
...(auth.credentials.email_login && {
|
||||
email_login: {
|
||||
address: auth.credentials.email_login.address.trim(),
|
||||
password: auth.credentials.email_login.password,
|
||||
...(auth.credentials.email_login.totp_secret && {
|
||||
totp_secret: auth.credentials.email_login.totp_secret.trim(),
|
||||
totp_secret: auth.credentials.email_login.totp_secret.replace(/\s/g, ''),
|
||||
}),
|
||||
},
|
||||
}),
|
||||
|
||||
@@ -179,7 +179,7 @@ type RuleKind = 'avoid' | 'focus';
|
||||
interface MissingCodePath {
|
||||
kind: RuleKind;
|
||||
value: string;
|
||||
description: string;
|
||||
description?: string;
|
||||
}
|
||||
|
||||
async function validateCodePathsExist(
|
||||
@@ -202,12 +202,16 @@ async function validateCodePathsExist(
|
||||
const missing: MissingCodePath[] = [];
|
||||
for (const { kind, rule } of tagged) {
|
||||
if (!(await patternMatchesAny(repoPath, rule.value))) {
|
||||
missing.push({ kind, value: rule.value, description: rule.description });
|
||||
const entry: MissingCodePath = { kind, value: rule.value };
|
||||
if (rule.description) {
|
||||
entry.description = rule.description;
|
||||
}
|
||||
missing.push(entry);
|
||||
}
|
||||
}
|
||||
|
||||
if (missing.length > 0) {
|
||||
const lines = missing.map((m) => `[${m.kind}] '${m.value}' — ${m.description}`);
|
||||
const lines = missing.map((m) => `[${m.kind}] '${m.value}'${m.description ? ` - ${m.description}` : ''}`);
|
||||
return err(
|
||||
new PentestError(
|
||||
`code_path rules don't match any file or directory in the repo:\n - ${lines.join('\n - ')}\n` +
|
||||
|
||||
@@ -12,61 +12,32 @@ import type { Authentication, DistributedConfig, DistributedReportConfig, Rule,
|
||||
import { isGlobPattern } from '../utils/glob.js';
|
||||
import { handlePromptError, PentestError } from './error-handling.js';
|
||||
|
||||
function renderRuleLine(tag: string, value: string, description?: string): string {
|
||||
const base = `- ${tag} ${value}`;
|
||||
return description ? `${base} - ${description}` : base;
|
||||
}
|
||||
|
||||
function renderUrlRules(rules: Rule[]): string {
|
||||
if (rules.length === 0) return 'None';
|
||||
return rules.map((r) => renderRuleLine(`[${r.type.toUpperCase()}]`, r.value, r.description)).join('\n');
|
||||
}
|
||||
|
||||
function renderCodePathRules(rules: Rule[]): string {
|
||||
const filtered = rules.filter((r) => r.type === 'code_path');
|
||||
if (filtered.length === 0) return 'None';
|
||||
return filtered
|
||||
.map((r) => {
|
||||
const kind = isGlobPattern(r.value) ? '[GLOB]' : '[FILE]';
|
||||
return `- ${r.value} ${kind} — ${r.description}`;
|
||||
})
|
||||
.map((r) => renderRuleLine(isGlobPattern(r.value) ? '[GLOB]' : '[FILE]', r.value, r.description))
|
||||
.join('\n');
|
||||
}
|
||||
|
||||
interface VulnSummarySpec {
|
||||
readonly heading: string;
|
||||
readonly evidenceSection: string;
|
||||
readonly noneFoundLabel: string;
|
||||
}
|
||||
|
||||
const VULN_SUMMARY_SPECS: Record<VulnClass, VulnSummarySpec> = {
|
||||
auth: {
|
||||
heading: 'Authentication Vulnerabilities',
|
||||
evidenceSection: 'Authentication Exploitation Evidence',
|
||||
noneFoundLabel: 'authentication',
|
||||
},
|
||||
authz: {
|
||||
heading: 'Authorization Vulnerabilities',
|
||||
evidenceSection: 'Authorization Exploitation Evidence',
|
||||
noneFoundLabel: 'authorization',
|
||||
},
|
||||
xss: {
|
||||
heading: 'Cross-Site Scripting (XSS) Vulnerabilities',
|
||||
evidenceSection: 'XSS Exploitation Evidence',
|
||||
noneFoundLabel: 'XSS',
|
||||
},
|
||||
injection: {
|
||||
heading: 'SQL/Command Injection Vulnerabilities',
|
||||
evidenceSection: 'Injection Exploitation Evidence',
|
||||
noneFoundLabel: 'SQL or command injection',
|
||||
},
|
||||
ssrf: {
|
||||
heading: 'Server-Side Request Forgery (SSRF) Vulnerabilities',
|
||||
evidenceSection: 'SSRF Exploitation Evidence',
|
||||
noneFoundLabel: 'SSRF',
|
||||
},
|
||||
const VULN_CLASS_HEADINGS: Record<VulnClass, string> = {
|
||||
auth: 'Authentication Vulnerabilities',
|
||||
authz: 'Authorization Vulnerabilities',
|
||||
xss: 'Cross-Site Scripting (XSS) Vulnerabilities',
|
||||
injection: 'SQL/Command Injection Vulnerabilities',
|
||||
ssrf: 'Server-Side Request Forgery (SSRF) Vulnerabilities',
|
||||
};
|
||||
|
||||
function renderVulnSummarySubsections(selected: readonly VulnClass[]): string {
|
||||
const classes = selected.length > 0 ? selected : (Object.keys(VULN_SUMMARY_SPECS) as VulnClass[]);
|
||||
return classes
|
||||
.map((cls) => {
|
||||
const spec = VULN_SUMMARY_SPECS[cls];
|
||||
return `**${spec.heading}:**\n{Check for "${spec.evidenceSection}" section. Include actually exploited vulnerabilities and those blocked by security controls. Exclude theoretical vulnerabilities requiring internal network access. If vulnerabilities exist, summarize their impact and severity. If section is missing or empty, state: "No ${spec.noneFoundLabel} vulnerabilities were found."}`;
|
||||
})
|
||||
.join('\n\n');
|
||||
}
|
||||
|
||||
/**
|
||||
* Renders the <not_assessed_classes> block. Empty when every class completed.
|
||||
*
|
||||
@@ -86,9 +57,8 @@ function renderNotAssessedClassesBlock(failed: readonly VulnClass[] = []): strin
|
||||
];
|
||||
|
||||
for (const cls of classes) {
|
||||
const spec = VULN_SUMMARY_SPECS[cls];
|
||||
lines.push(
|
||||
`- ${spec.heading}: analysis did not complete; this class was NOT assessed. Absence of findings here does not indicate the class is clean.`,
|
||||
`- ${VULN_CLASS_HEADINGS[cls]}: analysis did not complete; this class was NOT assessed. Absence of findings here does not indicate the class is clean.`,
|
||||
);
|
||||
}
|
||||
|
||||
@@ -103,13 +73,13 @@ function renderNotAssessedClassesBlock(failed: readonly VulnClass[] = []): strin
|
||||
/**
|
||||
* Which configured filters this run can actually enforce.
|
||||
*
|
||||
* The two ratings are mode-exclusive (see ../collectors/finding-collector.ts): an exploited
|
||||
* finding carries `severity`, an analysed one carries `confidence`. Handing the agent a
|
||||
* threshold for the rating its findings do not have is a directive it cannot honor.
|
||||
* Every finding carries `severity` (see ../collectors/finding-collector.ts), so a severity
|
||||
* threshold always applies. `confidence` exists only on an analysed finding — handing an
|
||||
* exploit run a confidence threshold is a directive it cannot honor.
|
||||
*/
|
||||
function applicableFilters(report: DistributedReportConfig | undefined, exploitEnabled: boolean) {
|
||||
return {
|
||||
severity: Boolean(report?.min_severity) && exploitEnabled,
|
||||
severity: Boolean(report?.min_severity),
|
||||
confidence: Boolean(report?.min_confidence) && !exploitEnabled,
|
||||
guidance: Boolean(report?.guidance?.trim()),
|
||||
};
|
||||
@@ -375,8 +345,8 @@ async function interpolateVariables(
|
||||
if (avoidUrlRules.length === 0 && focusUrlRules.length === 0) {
|
||||
result = result.replace(/<rules>[\s\S]*?<\/rules>\s*/g, '');
|
||||
} else {
|
||||
const avoidStr = avoidUrlRules.length > 0 ? avoidUrlRules.map((r) => `- ${r.description}`).join('\n') : 'None';
|
||||
const focusStr = focusUrlRules.length > 0 ? focusUrlRules.map((r) => `- ${r.description}`).join('\n') : 'None';
|
||||
const avoidStr = renderUrlRules(avoidUrlRules);
|
||||
const focusStr = renderUrlRules(focusUrlRules);
|
||||
result = replaceLiteral(result, /{{RULES_AVOID}}/g, avoidStr);
|
||||
result = replaceLiteral(result, /{{RULES_FOCUS}}/g, focusStr);
|
||||
}
|
||||
@@ -416,7 +386,6 @@ async function interpolateVariables(
|
||||
/{{VULN_CLASSES_TESTED}}/g,
|
||||
vulnClasses.length > 0 ? vulnClasses.join(', ') : 'injection, xss, auth, authz, ssrf',
|
||||
);
|
||||
result = replaceLiteral(result, /{{VULN_SUMMARY_SUBSECTIONS}}/g, renderVulnSummarySubsections(vulnClasses));
|
||||
result = replaceLiteral(
|
||||
result,
|
||||
/{{NOT_ASSESSED_CLASSES}}/g,
|
||||
@@ -432,19 +401,12 @@ async function interpolateVariables(
|
||||
result = result.replace(/<\/?(?:exploit|analysis)_mode_[a-z_]+>\n?/g, '');
|
||||
|
||||
result = replaceLiteral(result, /{{EXPLOITATION}}/g, exploitEnabled ? 'enabled' : 'disabled');
|
||||
result = replaceLiteral(result, /{{REPORT_VULN_HEADING}}/g, exploitEnabled ? 'Exploitation Evidence' : 'Findings');
|
||||
result = replaceLiteral(
|
||||
result,
|
||||
/{{REPORT_VULN_SUBHEADING}}/g,
|
||||
exploitEnabled ? 'Successfully Exploited Vulnerabilities' : 'Identified Vulnerabilities',
|
||||
);
|
||||
|
||||
if (config?.report?.min_severity && !exploitEnabled) {
|
||||
logger.warn(
|
||||
`report.min_severity="${config.report.min_severity}" is ignored when exploit=false: an ` +
|
||||
'analysis-only run rates findings by confidence, not severity. Use report.min_confidence.',
|
||||
);
|
||||
}
|
||||
if (config?.report?.min_confidence && exploitEnabled) {
|
||||
logger.warn(
|
||||
`report.min_confidence="${config.report.min_confidence}" is ignored when exploit=true: an ` +
|
||||
|
||||
@@ -263,7 +263,16 @@ export function renderReport(data: ReportData): string {
|
||||
sections.push(`### ${cat}`);
|
||||
sections.push('');
|
||||
for (const f of catFindings) {
|
||||
const suffix = f.severity ? ` (${titleCase(f.severity)})` : '';
|
||||
// Both ratings when the mode produced both. Confidence is labelled so it is never
|
||||
// read as a severity in the position where a severity usually sits.
|
||||
const ratings: string[] = [];
|
||||
if (f.severity) {
|
||||
ratings.push(titleCase(f.severity));
|
||||
}
|
||||
if (f.confidence) {
|
||||
ratings.push(`${titleCase(f.confidence)} confidence`);
|
||||
}
|
||||
const suffix = ratings.length > 0 ? ` (${ratings.join(', ')})` : '';
|
||||
sections.push(`- **${f.finding_id}:** ${f.title}${suffix}`);
|
||||
}
|
||||
sections.push('');
|
||||
|
||||
@@ -11,7 +11,7 @@
|
||||
export type RuleType = 'url_path' | 'subdomain' | 'domain' | 'method' | 'header' | 'parameter' | 'code_path';
|
||||
|
||||
export interface Rule {
|
||||
description: string;
|
||||
description?: string;
|
||||
type: RuleType;
|
||||
value: string;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user