refactor: remove ~70 low-value comments across 13 files

- Remove empty section markers (// === ... ===, // --- ... ---) that duplicate JSDoc or function names
- Remove "what" comments that restate the next line of code (e.g. // Save to disk, // Check for retryable patterns)
- Remove file-level descriptions that restate the filename (e.g. // Pure functions for formatting console output)
- Fix "Added by client" comment referencing implementation history → "Used for audit correlation"
- Preserve all WHY comments: error classification groups, billing/session limit explanations, ESM interop, exactOptionalPropertyTypes, mutex reasoning
This commit is contained in:
ajmallesh
2026-02-16 18:08:11 -08:00
parent b208949345
commit 16de74e0be
13 changed files with 3 additions and 100 deletions
-4
View File
@@ -4,8 +4,6 @@
// it under the terms of the GNU Affero General Public License version 3
// as published by the Free Software Foundation.
// Pure functions for processing SDK message types
import { PentestError } from '../services/error-handling.js';
import { ErrorCode } from '../types/errors.js';
import { matchesBillingTextPattern } from '../utils/billing-detection.js';
@@ -263,14 +261,12 @@ function handleToolResultMessage(message: ToolResultMessage): ToolResultData {
};
}
// Output helper for console logging
function outputLines(lines: string[]): void {
for (const line of lines) {
console.log(line);
}
}
// Message dispatch result types
export type MessageDispatchAction =
| { type: 'continue'; apiErrorDetected?: boolean | undefined; model?: string | undefined }
| { type: 'complete'; result: string | null; cost: number }
-10
View File
@@ -4,14 +4,10 @@
// it under the terms of the GNU Affero General Public License version 3
// as published by the Free Software Foundation.
// Pure functions for formatting console output
import { extractAgentType, formatDuration } from '../utils/formatting.js';
import { AGENTS } from '../session-manager.js';
import type { ExecutionContext, ResultData } from './types.js';
// --- Types for tool call filtering ---
interface ToolCallInput {
url?: string;
element?: string;
@@ -32,8 +28,6 @@ interface ToolCall {
input?: ToolCallInput;
}
// --- Agent prefix logic ---
/**
* Get agent prefix for parallel execution
*/
@@ -70,8 +64,6 @@ export function getAgentPrefix(description: string): string {
return '[Agent]';
}
// --- Tool call filtering ---
/**
* Extract domain from URL for display
*/
@@ -273,8 +265,6 @@ export function filterJsonToolCalls(content: string | null | undefined): string
return processedLines.join('\n');
}
// --- Console output formatting ---
export function detectExecutionContext(description: string): ExecutionContext {
const isParallelExecution =
description.includes('vuln agent') || description.includes('exploit agent');
-10
View File
@@ -112,24 +112,19 @@ export class AuditSession {
await AgentLogger.savePrompt(this.sessionMetadata, agentName, promptContent);
}
// Track current agent name for workflow logging
this.currentAgentName = agentName;
// Create and initialize logger for this attempt
this.currentLogger = new AgentLogger(this.sessionMetadata, agentName, attemptNumber);
await this.currentLogger.initialize();
// Start metrics tracking
this.metricsTracker.startAgent(agentName, attemptNumber);
// Log start event
await this.currentLogger.logEvent('agent_start', {
agentName,
attemptNumber,
timestamp: formatTimestamp(),
});
// Log to unified workflow log
await this.workflowLogger.logAgent(agentName, 'start', { attemptNumber });
}
@@ -177,7 +172,6 @@ export class AuditSession {
* End agent execution (mutex-protected)
*/
async endAgent(agentName: string, result: AgentEndResult): Promise<void> {
// Log end event
if (this.currentLogger) {
await this.currentLogger.logEvent('agent_end', {
agentName,
@@ -187,15 +181,12 @@ export class AuditSession {
timestamp: formatTimestamp(),
});
// Close logger
await this.currentLogger.close();
this.currentLogger = null;
}
// Reset current agent name
this.currentAgentName = null;
// Log to unified workflow log
const agentLogDetails: AgentLogDetails = {
attemptNumber: result.attemptNumber,
duration_ms: result.duration_ms,
@@ -211,7 +202,6 @@ export class AuditSession {
// Reload inside mutex to prevent lost updates during parallel exploitation phase
await this.metricsTracker.reload();
// Update metrics
await this.metricsTracker.endAgent(agentName, result);
} finally {
unlock();
-1
View File
@@ -42,7 +42,6 @@ export class AgentLogger {
this.attemptNumber = attemptNumber;
this.timestamp = Date.now();
// Generate log file path and create stream
const logPath = generateLogPath(sessionMetadata, agentName, this.timestamp, attemptNumber);
this.logStream = new LogStream(logPath);
}
-1
View File
@@ -227,7 +227,6 @@ export class MetricsTracker {
// Recalculate aggregations
this.recalculateAggregations();
// Save to disk
await this.save();
}
-1
View File
@@ -59,7 +59,6 @@ export class WorkflowLogger {
return;
}
// Open the stream (LogStream.open() handles directory creation)
await this.logStream.open();
// Write header only if file is new (empty)
-34
View File
@@ -22,11 +22,9 @@ import type {
const require = createRequire(import.meta.url);
const addFormats: FormatsPlugin = require('ajv-formats');
// Initialize AJV with formats
const ajv = new Ajv({ allErrors: true, verbose: true });
addFormats(ajv);
// Load JSON Schema
let configSchema: object;
let validateSchema: ValidateFunction;
@@ -45,7 +43,6 @@ try {
);
}
// Security patterns to block
const DANGEROUS_PATTERNS: RegExp[] = [
/\.\.\//, // Path traversal
/[<>]/, // HTML/XML injection
@@ -179,10 +176,8 @@ function formatAjvErrors(errors: ErrorObject[]): string[] {
return errors.map(formatAjvError);
}
// Parse and load YAML configuration file with enhanced safety
export const parseConfig = async (configPath: string): Promise<Config> => {
try {
// File existence check
if (!(await fs.pathExists(configPath))) {
throw new PentestError(
`Configuration file not found: ${configPath}`,
@@ -193,7 +188,6 @@ export const parseConfig = async (configPath: string): Promise<Config> => {
);
}
// File size check (prevent extremely large files)
const stats = await fs.stat(configPath);
const maxFileSize = 1024 * 1024; // 1MB
if (stats.size > maxFileSize) {
@@ -206,10 +200,8 @@ export const parseConfig = async (configPath: string): Promise<Config> => {
);
}
// Read file content
const configContent = await fs.readFile(configPath, 'utf8');
// Basic content validation
if (!configContent.trim()) {
throw new PentestError(
'Configuration file is empty',
@@ -220,7 +212,6 @@ export const parseConfig = async (configPath: string): Promise<Config> => {
);
}
// Parse YAML with safety options
let config: unknown;
try {
config = yaml.load(configContent, {
@@ -239,7 +230,6 @@ export const parseConfig = async (configPath: string): Promise<Config> => {
);
}
// Additional safety check
if (config === null || config === undefined) {
throw new PentestError(
'Configuration file resulted in null/undefined after parsing',
@@ -250,7 +240,6 @@ export const parseConfig = async (configPath: string): Promise<Config> => {
);
}
// Validate the configuration structure and content
validateConfig(config as Config);
return config as Config;
@@ -259,7 +248,6 @@ export const parseConfig = async (configPath: string): Promise<Config> => {
if (error instanceof PentestError) {
throw error;
}
// Wrap other errors with context
const errMsg = error instanceof Error ? error.message : String(error);
throw new PentestError(
`Failed to parse configuration file '${configPath}': ${errMsg}`,
@@ -271,9 +259,7 @@ export const parseConfig = async (configPath: string): Promise<Config> => {
}
};
// Validate overall configuration structure using JSON Schema
const validateConfig = (config: Config): void => {
// Basic structure validation
if (!config || typeof config !== 'object') {
throw new PentestError(
'Configuration must be a valid object',
@@ -294,7 +280,6 @@ const validateConfig = (config: Config): void => {
);
}
// JSON Schema validation
const isValid = validateSchema(config);
if (!isValid) {
const errors = validateSchema.errors || [];
@@ -308,10 +293,8 @@ const validateConfig = (config: Config): void => {
);
}
// Additional security validation
performSecurityValidation(config);
// Ensure at least some configuration is provided
if (!config.rules && !config.authentication) {
console.warn(
'⚠️ Configuration file contains no rules or authentication. The pentest will run without any scoping restrictions or login capabilities.'
@@ -323,9 +306,7 @@ const validateConfig = (config: Config): void => {
}
};
// Perform additional security validation beyond JSON Schema
const performSecurityValidation = (config: Config): void => {
// Validate authentication section for security issues
if (config.authentication) {
const auth = config.authentication;
@@ -344,7 +325,6 @@ const performSecurityValidation = (config: Config): void => {
}
}
// Check for dangerous patterns in credentials
if (auth.credentials) {
for (const pattern of DANGEROUS_PATTERNS) {
if (pattern.test(auth.credentials.username)) {
@@ -368,7 +348,6 @@ const performSecurityValidation = (config: Config): void => {
}
}
// Check login flow for dangerous patterns
if (auth.login_flow) {
auth.login_flow.forEach((step, index) => {
for (const pattern of DANGEROUS_PATTERNS) {
@@ -386,24 +365,20 @@ const performSecurityValidation = (config: Config): void => {
}
}
// Validate rules section for security issues
if (config.rules) {
validateRulesSecurity(config.rules.avoid, 'avoid');
validateRulesSecurity(config.rules.focus, 'focus');
// Check for duplicate and conflicting rules
checkForDuplicates(config.rules.avoid || [], 'avoid');
checkForDuplicates(config.rules.focus || [], 'focus');
checkForConflicts(config.rules.avoid, config.rules.focus);
}
};
// Validate rules for security issues
const validateRulesSecurity = (rules: Rule[] | undefined, ruleType: string): void => {
if (!rules) return;
rules.forEach((rule, index) => {
// Security validation
for (const pattern of DANGEROUS_PATTERNS) {
if (pattern.test(rule.url_path)) {
throw new PentestError(
@@ -425,12 +400,10 @@ const validateRulesSecurity = (rules: Rule[] | undefined, ruleType: string): voi
}
}
// Type-specific validation
validateRuleTypeSpecific(rule, ruleType, index);
});
};
// Validate rule based on its specific type
const validateRuleTypeSpecific = (rule: Rule, ruleType: string, index: number): void => {
const field = `rules.${ruleType}[${index}].url_path`;
@@ -486,7 +459,6 @@ const validateRuleTypeSpecific = (rule: Rule, ruleType: string, index: number):
}
case 'header':
// Header name validation (basic)
if (!rule.url_path.match(/^[a-zA-Z0-9\-_]+$/)) {
throw new PentestError(
`${field} for type 'header' must be a valid header name (alphanumeric, hyphens, underscores only)`,
@@ -499,7 +471,6 @@ const validateRuleTypeSpecific = (rule: Rule, ruleType: string, index: number):
break;
case 'parameter':
// Parameter name validation (basic)
if (!rule.url_path.match(/^[a-zA-Z0-9\-_]+$/)) {
throw new PentestError(
`${field} for type 'parameter' must be a valid parameter name (alphanumeric, hyphens, underscores only)`,
@@ -513,7 +484,6 @@ const validateRuleTypeSpecific = (rule: Rule, ruleType: string, index: number):
}
};
// Check for duplicate rules
const checkForDuplicates = (rules: Rule[], ruleType: string): void => {
const seen = new Set<string>();
rules.forEach((rule, index) => {
@@ -531,7 +501,6 @@ const checkForDuplicates = (rules: Rule[], ruleType: string): void => {
});
};
// Check for conflicting rules between avoid and focus
const checkForConflicts = (avoidRules: Rule[] = [], focusRules: Rule[] = []): void => {
const avoidSet = new Set(avoidRules.map((rule) => `${rule.type}:${rule.url_path}`));
@@ -549,7 +518,6 @@ const checkForConflicts = (avoidRules: Rule[] = [], focusRules: Rule[] = []): vo
});
};
// Sanitize and normalize rule values
const sanitizeRule = (rule: Rule): Rule => {
return {
description: rule.description.trim(),
@@ -558,7 +526,6 @@ const sanitizeRule = (rule: Rule): Rule => {
};
};
// Distribute configuration sections to different agents with sanitization
export const distributeConfig = (config: Config | null): DistributedConfig => {
const avoid = config?.rules?.avoid || [];
const focus = config?.rules?.focus || [];
@@ -571,7 +538,6 @@ export const distributeConfig = (config: Config | null): DistributedConfig => {
};
};
// Sanitize and normalize authentication values
const sanitizeAuthentication = (auth: Authentication): Authentication => {
return {
login_type: auth.login_type.toLowerCase().trim() as Authentication['login_type'],
-6
View File
@@ -15,7 +15,6 @@ import {
matchesBillingTextPattern,
} from '../utils/billing-detection.js';
// Custom error class for pentest operations
export class PentestError extends Error {
override name = 'PentestError' as const;
type: PentestErrorType;
@@ -43,8 +42,6 @@ export class PentestError extends Error {
}
}
// Handle tool execution errors
// Handle prompt loading errors
export function handlePromptError(
promptName: string,
error: Error
@@ -60,7 +57,6 @@ export function handlePromptError(
};
}
// Patterns that indicate retryable errors
const RETRYABLE_PATTERNS = [
// Network and connection errors
'network',
@@ -104,12 +100,10 @@ const NON_RETRYABLE_PATTERNS = [
export function isRetryableError(error: Error): boolean {
const message = error.message.toLowerCase();
// Check for explicit non-retryable patterns first
if (NON_RETRYABLE_PATTERNS.some((pattern) => message.includes(pattern))) {
return false;
}
// Check for retryable patterns
return RETRYABLE_PATTERNS.some((pattern) => message.includes(pattern));
}
-10
View File
@@ -194,8 +194,6 @@ async function runAgentActivity(
}
}
// === Individual Agent Activity Exports ===
export async function runPreReconAgent(input: ActivityInput): Promise<AgentMetrics> {
return runAgentActivity('pre-recon', input);
}
@@ -248,8 +246,6 @@ export async function runReportAgent(input: ActivityInput): Promise<AgentMetrics
return runAgentActivity('report', input);
}
// === Report Assembly Activities ===
/**
* Assemble the final report by concatenating exploitation evidence files.
*/
@@ -282,8 +278,6 @@ export async function injectReportMetadataActivity(input: ActivityInput): Promis
}
}
// === Exploitation Queue Check ===
/**
* Check if exploitation should run for a given vulnerability type.
*
@@ -304,8 +298,6 @@ export async function checkExploitationQueue(
return checker.checkQueue(vulnType, repoPath, logger);
}
// === Resume Activities ===
interface SessionJson {
session: {
id: string;
@@ -511,8 +503,6 @@ export async function recordResumeAttempt(
});
}
// === Phase Transition Activities ===
/**
* Log phase transition to the unified workflow log.
*/
+2 -6
View File
@@ -229,7 +229,6 @@ async function startPipeline(): Promise<void> {
const workspaceExists = await fileExists(sessionPath);
if (workspaceExists) {
// === Resume Mode: existing workspace ===
isResume = true;
console.log('=== RESUME MODE ===');
console.log(`Workspace: ${resumeFromWorkspace}\n`);
@@ -255,7 +254,6 @@ async function startPipeline(): Promise<void> {
workflowId = `${resumeFromWorkspace}_resume_${Date.now()}`;
sessionId = resumeFromWorkspace;
} else {
// === New Named Workspace ===
if (!isValidWorkspaceName(resumeFromWorkspace)) {
console.error(`ERROR: Invalid workspace name: "${resumeFromWorkspace}"`);
console.error(' Must be 1-128 characters, alphanumeric/hyphens/underscores, starting with alphanumeric');
@@ -269,7 +267,6 @@ async function startPipeline(): Promise<void> {
sessionId = resumeFromWorkspace;
}
} else {
// === New Auto-Named Workflow ===
const hostname = sanitizeHostname(webUrl);
workflowId = customWorkflowId || `${hostname}_shannon-${Date.now()}`;
sessionId = workflowId;
@@ -278,8 +275,8 @@ async function startPipeline(): Promise<void> {
const input: PipelineInput = {
webUrl,
repoPath,
workflowId, // Add for audit correlation
sessionId, // Workspace directory name
workflowId,
sessionId,
...(configPath && { configPath }),
...(outputPath && { outputPath }),
...(pipelineTestingMode && { pipelineTestingMode }),
@@ -287,7 +284,6 @@ async function startPipeline(): Promise<void> {
...(terminatedWorkflows.length > 0 && { terminatedWorkflows }),
};
// Determine output directory for display (use sessionId for persistent directory)
// Use displayOutputPath (host path) if provided, otherwise fall back to outputPath or default
const effectiveDisplayPath = displayOutputPath || outputPath || './audit-logs';
const outputDir = `${effectiveDisplayPath}/${sessionId}`;
+1 -5
View File
@@ -3,15 +3,13 @@ import { defineQuery } from '@temporalio/workflow';
export type { AgentMetrics } from '../types/metrics.js';
import type { AgentMetrics } from '../types/metrics.js';
// === Types ===
export interface PipelineInput {
webUrl: string;
repoPath: string;
configPath?: string;
outputPath?: string;
pipelineTestingMode?: boolean;
workflowId?: string; // Added by client, used for audit correlation
workflowId?: string; // Used for audit correlation
sessionId?: string; // Workspace directory name (distinct from workflowId for named workspaces)
resumeFromWorkspace?: string; // Workspace name to resume from
terminatedWorkflows?: string[]; // Workflows terminated during resume
@@ -62,6 +60,4 @@ export interface VulnExploitPipelineResult {
error: string | null;
}
// === Queries ===
export const getProgress = defineQuery<PipelineProgress>('getProgress');
-10
View File
@@ -105,11 +105,9 @@ export async function pentestPipelineWorkflow(
): Promise<PipelineState> {
const { workflowId } = workflowInfo();
// Select activity proxy based on testing mode
// Pipeline testing uses fast retry intervals (10s) for quick iteration
const a = input.pipelineTestingMode ? testActs : acts;
// Workflow state (queryable)
const state: PipelineState = {
status: 'running',
currentPhase: null,
@@ -122,7 +120,6 @@ export async function pentestPipelineWorkflow(
summary: null,
};
// Register query handler for real-time progress inspection
setHandler(getProgress, (): PipelineProgress => ({
...state,
workflowId,
@@ -147,18 +144,15 @@ export async function pentestPipelineWorkflow(
}),
};
// === RESUME LOGIC ===
let resumeState: ResumeState | null = null;
if (input.resumeFromWorkspace) {
// Load resume state from existing workspace
resumeState = await a.loadResumeState(
input.resumeFromWorkspace,
input.webUrl,
input.repoPath
);
// Restore git checkpoint and clean up partial deliverables
const incompleteAgents = ALL_AGENTS.filter(
(agentName) => !resumeState!.completedAgents.includes(agentName)
) as AgentName[];
@@ -169,7 +163,6 @@ export async function pentestPipelineWorkflow(
incompleteAgents
);
// Check if all agents are already complete
if (resumeState.completedAgents.length === ALL_AGENTS.length) {
log.info(`All ${ALL_AGENTS.length} agents already completed. Nothing to resume.`);
state.status = 'completed';
@@ -178,7 +171,6 @@ export async function pentestPipelineWorkflow(
return state;
}
// Record resume attempt in session.json and write resume header to workflow.log
await a.recordResumeAttempt(
activityInput,
input.terminatedWorkflows || [],
@@ -190,7 +182,6 @@ export async function pentestPipelineWorkflow(
log.info('Resume state loaded and workspace restored');
}
// Helper to check if an agent should be skipped
const shouldSkip = (agentName: string): boolean => {
return resumeState?.completedAgents.includes(agentName) ?? false;
};
@@ -413,7 +404,6 @@ export async function pentestPipelineWorkflow(
state.completedAgents.push('report');
}
// === Complete ===
state.status = 'completed';
state.currentPhase = null;
state.currentAgent = null;
-2
View File
@@ -4,8 +4,6 @@
// it under the terms of the GNU Affero General Public License version 3
// as published by the Free Software Foundation.
// Timing utilities
export class Timer {
name: string;
startTime: number;