mirror of
https://github.com/tiennm99/ccs.git
synced 2026-07-16 02:11:28 +00:00
fix(hooks): isolate image type check before error-prone processing (#514)
* fix(hooks): isolate image type check before error-prone processing Restructure processHook() into two phases so non-image Read calls never see hook error messages. Phase 1 defensively checks tool name and file extension, exiting 0 silently on any failure. Phase 2 only runs for confirmed image/PDF files where errors are relevant. Closes #511 * fix(hooks): sync image analyzer hook file on every profile launch Add installImageAnalyzerHook() call to cliproxy executor, matching the existing installWebSearchHook() pattern. This ensures the .cjs file in ~/.ccs/hooks/ gets refreshed from the npm package on every launch, so users receive hook updates after npm update.
This commit is contained in:
@@ -761,14 +761,16 @@ process.stdin.on('error', () => {
|
||||
|
||||
/**
|
||||
* Main hook processing logic
|
||||
*
|
||||
* Two-phase design: Phase 1 filters non-image Read calls silently (exit 0).
|
||||
* Phase 2 only runs for confirmed image/PDF files, so error messages are
|
||||
* always relevant and never confuse users reading code or text files.
|
||||
*/
|
||||
async function processHook() {
|
||||
// Phase 1: Fast bail-out for non-image files
|
||||
// Any failure here → pass through silently to native Read
|
||||
let filePath;
|
||||
try {
|
||||
// Skip for native accounts or explicit disable
|
||||
if (shouldSkipHook()) {
|
||||
process.exit(0);
|
||||
}
|
||||
|
||||
const data = JSON.parse(input);
|
||||
|
||||
// Only handle Read tool
|
||||
@@ -776,23 +778,35 @@ async function processHook() {
|
||||
process.exit(0);
|
||||
}
|
||||
|
||||
const filePath = data.tool_input?.file_path || '';
|
||||
filePath = data.tool_input?.file_path || '';
|
||||
|
||||
if (!filePath) {
|
||||
process.exit(0);
|
||||
}
|
||||
|
||||
// Check file extension BEFORE any other processing — this is the key gate
|
||||
// that ensures non-image Read calls never see hook errors
|
||||
if (!isAnalyzableFile(filePath)) {
|
||||
process.exit(0);
|
||||
}
|
||||
} catch {
|
||||
// stdin parse failure or unexpected error → pass through silently
|
||||
process.exit(0);
|
||||
}
|
||||
|
||||
// Phase 2: Image/PDF file processing — errors here are relevant to the user
|
||||
try {
|
||||
// Skip for native accounts or explicit disable
|
||||
if (shouldSkipHook()) {
|
||||
process.exit(0);
|
||||
}
|
||||
|
||||
// Check if file exists
|
||||
if (!fs.existsSync(filePath)) {
|
||||
// Let native Read handle the error
|
||||
process.exit(0);
|
||||
}
|
||||
|
||||
// Check if file is analyzable
|
||||
if (!isAnalyzableFile(filePath)) {
|
||||
process.exit(0);
|
||||
}
|
||||
|
||||
// Check file size
|
||||
const stats = fs.statSync(filePath);
|
||||
if (stats.size >= MAX_FILE_SIZE_BYTES) {
|
||||
@@ -843,14 +857,7 @@ async function processHook() {
|
||||
console.error('[CCS Hook] Error:', err.message);
|
||||
}
|
||||
|
||||
// Try to extract file path from parsed input
|
||||
let filePath = 'unknown file';
|
||||
try {
|
||||
const data = JSON.parse(input);
|
||||
filePath = data.tool_input?.file_path || 'unknown file';
|
||||
} catch {
|
||||
// Ignore parse errors
|
||||
}
|
||||
// filePath is guaranteed set by Phase 1 — only image files reach here
|
||||
|
||||
// Categorize error by message pattern
|
||||
const errMsg = err.message || '';
|
||||
|
||||
@@ -50,6 +50,7 @@ import {
|
||||
displayWebSearchStatus,
|
||||
} from '../../utils/websearch-manager';
|
||||
import { loadOrCreateUnifiedConfig } from '../../config/unified-config-loader';
|
||||
import { installImageAnalyzerHook } from '../../utils/hooks';
|
||||
import { HttpsTunnelProxy } from '../https-tunnel-proxy';
|
||||
|
||||
// Import modular components
|
||||
@@ -162,6 +163,9 @@ export async function execClaudeWithCLIProxy(
|
||||
installWebSearchHook();
|
||||
displayWebSearchStatus();
|
||||
|
||||
// Sync image analyzer hook from npm package to ~/.ccs/hooks/
|
||||
installImageAnalyzerHook();
|
||||
|
||||
const providerConfig = getProviderConfig(provider);
|
||||
log(`Provider: ${providerConfig.displayName}`);
|
||||
|
||||
|
||||
@@ -425,8 +425,8 @@ describe('Image Analyzer Hook', () => {
|
||||
},
|
||||
});
|
||||
|
||||
// Should exit with error (code 2)
|
||||
expect(hookProcess.status).toBe(2);
|
||||
// Should pass through silently (exit 0) — can't determine file type from malformed input
|
||||
expect(hookProcess.status).toBe(0);
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
Reference in New Issue
Block a user