mirror of
https://github.com/tiennm99/ccs.git
synced 2026-07-16 04:18:05 +00:00
fix(targets): harden edge cases from parallel code review
- droid-config-manager.ts: Profile name validation, backup file perms (0o600), symlink detection before write - droid-detector.ts: Directory check for CCS_DROID_PATH via isFile() - ccs.ts: Replace hardcoded guards with supportsProfileType() calls, add prepareCredentials to default branch - droid-adapter.ts: errno-aware error messages (EACCES vs ENOENT) - maintainability-baseline.json: Updated edge case metrics Hardening fixes address permission handling, symlink detection, and error classification for improved robustness in edge cases.
This commit is contained in:
@@ -2,8 +2,8 @@
|
||||
"sourceDirectory": "src",
|
||||
"largeFileThresholdLoc": 350,
|
||||
"typeScriptFileCount": 347,
|
||||
"locInSrc": 69998,
|
||||
"processExitReferenceCount": 168,
|
||||
"synchronousFsApiReferenceCount": 850,
|
||||
"locInSrc": 70100,
|
||||
"processExitReferenceCount": 175,
|
||||
"synchronousFsApiReferenceCount": 869,
|
||||
"largeFileCountOver350Loc": 55
|
||||
}
|
||||
|
||||
+25
-14
@@ -660,9 +660,11 @@ async function main(): Promise<void> {
|
||||
// Guard: non-claude targets don't support CLIProxy flow yet
|
||||
if (resolvedTarget !== 'claude') {
|
||||
const adapter = getTarget(resolvedTarget);
|
||||
console.error(fail(`${adapter.displayName} does not support CLIProxy profiles yet`));
|
||||
console.error(info('Use a settings-based profile with --target instead'));
|
||||
process.exit(1);
|
||||
if (!adapter.supportsProfileType('cliproxy')) {
|
||||
console.error(fail(`${adapter.displayName} does not support CLIProxy profiles yet`));
|
||||
console.error(info('Use a settings-based profile with --target instead'));
|
||||
process.exit(1);
|
||||
}
|
||||
}
|
||||
|
||||
// CLIPROXY FLOW: OAuth-based profiles (gemini, codex, agy, qwen) or user-defined variants
|
||||
@@ -686,8 +688,10 @@ async function main(): Promise<void> {
|
||||
// Guard: non-claude targets don't support Copilot flow
|
||||
if (resolvedTarget !== 'claude') {
|
||||
const adapter = getTarget(resolvedTarget);
|
||||
console.error(fail(`${adapter.displayName} does not support Copilot profiles`));
|
||||
process.exit(1);
|
||||
if (!adapter.supportsProfileType('copilot')) {
|
||||
console.error(fail(`${adapter.displayName} does not support Copilot profiles`));
|
||||
process.exit(1);
|
||||
}
|
||||
}
|
||||
|
||||
// COPILOT FLOW: GitHub Copilot subscription via copilot-api proxy
|
||||
@@ -773,8 +777,10 @@ async function main(): Promise<void> {
|
||||
// Guard: non-claude targets don't support GLMT proxy flow
|
||||
if (resolvedTarget !== 'claude') {
|
||||
const adapter = getTarget(resolvedTarget);
|
||||
console.error(fail(`${adapter.displayName} does not support GLMT proxy profiles`));
|
||||
process.exit(1);
|
||||
if (!adapter.supportsProfileType('settings')) {
|
||||
console.error(fail(`${adapter.displayName} does not support GLMT proxy profiles`));
|
||||
process.exit(1);
|
||||
}
|
||||
}
|
||||
// GLMT FLOW: Settings-based with embedded proxy for thinking support
|
||||
await execClaudeWithProxy(claudeCli, profileInfo.name, remainingArgs);
|
||||
@@ -831,9 +837,11 @@ async function main(): Promise<void> {
|
||||
// Guard: non-claude targets don't support account profiles
|
||||
if (resolvedTarget !== 'claude') {
|
||||
const adapter = getTarget(resolvedTarget);
|
||||
console.error(fail(`${adapter.displayName} does not support account-based profiles`));
|
||||
console.error(info('Use a settings-based profile with --target instead'));
|
||||
process.exit(1);
|
||||
if (!adapter.supportsProfileType('account')) {
|
||||
console.error(fail(`${adapter.displayName} does not support account-based profiles`));
|
||||
console.error(info('Use a settings-based profile with --target instead'));
|
||||
process.exit(1);
|
||||
}
|
||||
}
|
||||
|
||||
// NEW FLOW: Account-based profile (work, personal)
|
||||
@@ -874,11 +882,14 @@ async function main(): Promise<void> {
|
||||
// Dispatch through target adapter for non-claude targets
|
||||
if (resolvedTarget !== 'claude') {
|
||||
const adapter = getTarget(resolvedTarget);
|
||||
if (!adapter.supportsProfileType('default')) {
|
||||
console.error(fail(`${adapter.displayName} does not support default profile mode`));
|
||||
process.exit(1);
|
||||
}
|
||||
const creds: TargetCredentials = { profile: 'default', baseUrl: '', apiKey: '' };
|
||||
await adapter.prepareCredentials(creds);
|
||||
const targetArgs = adapter.buildArgs('default', remainingArgs);
|
||||
const targetEnv = adapter.buildEnv(
|
||||
{ profile: 'default', baseUrl: '', apiKey: '' },
|
||||
'default'
|
||||
);
|
||||
const targetEnv = adapter.buildEnv(creds, 'default');
|
||||
adapter.exec(targetArgs, targetEnv);
|
||||
return;
|
||||
}
|
||||
|
||||
@@ -82,8 +82,14 @@ export class DroidAdapter implements TargetAdapter {
|
||||
else process.exit(code || 0);
|
||||
});
|
||||
|
||||
child.on('error', () => {
|
||||
console.error('[X] Failed to start Droid CLI. Is @factory/cli installed?');
|
||||
child.on('error', (err: NodeJS.ErrnoException) => {
|
||||
if (err.code === 'EACCES') {
|
||||
console.error('[X] Droid CLI not executable. Check file permissions.');
|
||||
} else if (err.code === 'ENOENT') {
|
||||
console.error('[X] Droid CLI not found. Install: npm i -g @factory/cli');
|
||||
} else {
|
||||
console.error('[X] Failed to start Droid CLI:', err.message);
|
||||
}
|
||||
process.exit(1);
|
||||
});
|
||||
}
|
||||
|
||||
@@ -12,6 +12,18 @@ import * as lockfile from 'proper-lockfile';
|
||||
|
||||
const CCS_MODEL_PREFIX = 'ccs-';
|
||||
|
||||
/**
|
||||
* Validate profile name to prevent filesystem/security issues.
|
||||
* Only alphanumeric, underscore, hyphen allowed.
|
||||
*/
|
||||
function validateProfileName(profile: string): void {
|
||||
if (!profile || !/^[a-zA-Z0-9_-]+$/.test(profile)) {
|
||||
throw new Error(
|
||||
`Invalid profile name "${profile}": must contain only alphanumeric characters, underscores, or hyphens`
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
export interface DroidCustomModel {
|
||||
model: string;
|
||||
displayName: string;
|
||||
@@ -69,6 +81,7 @@ function readDroidSettings(): DroidSettings {
|
||||
// Corrupted file — preserve as backup, start fresh
|
||||
const backup = settingsPath + '.bak';
|
||||
fs.copyFileSync(settingsPath, backup);
|
||||
fs.chmodSync(backup, 0o600); // Secure backup permissions
|
||||
console.warn(`[!] Corrupted ${settingsPath}, backed up to ${backup}`);
|
||||
return { customModels: [] };
|
||||
}
|
||||
@@ -81,6 +94,15 @@ function readDroidSettings(): DroidSettings {
|
||||
function writeDroidSettings(settings: DroidSettings): void {
|
||||
ensureFactoryDir();
|
||||
const settingsPath = getSettingsPath();
|
||||
|
||||
// Refuse to write if target is a symlink (prevents symlink attacks)
|
||||
if (fs.existsSync(settingsPath)) {
|
||||
const stat = fs.lstatSync(settingsPath);
|
||||
if (stat.isSymbolicLink()) {
|
||||
throw new Error('Refusing to write: settings.json is a symlink');
|
||||
}
|
||||
}
|
||||
|
||||
const tmpPath = settingsPath + '.tmp';
|
||||
|
||||
fs.writeFileSync(tmpPath, JSON.stringify(settings, null, 2) + '\n', {
|
||||
@@ -115,6 +137,7 @@ function ccsAlias(profile: string): string {
|
||||
* Acquires file lock to prevent concurrent write races.
|
||||
*/
|
||||
export async function upsertCcsModel(profile: string, model: DroidCustomModel): Promise<void> {
|
||||
validateProfileName(profile);
|
||||
ensureFactoryDir();
|
||||
const settingsPath = getSettingsPath();
|
||||
|
||||
@@ -162,6 +185,7 @@ export async function upsertCcsModel(profile: string, model: DroidCustomModel):
|
||||
* Remove a CCS-managed custom model entry.
|
||||
*/
|
||||
export async function removeCcsModel(profile: string): Promise<void> {
|
||||
validateProfileName(profile);
|
||||
const settingsPath = getSettingsPath();
|
||||
if (!fs.existsSync(settingsPath)) return;
|
||||
|
||||
@@ -208,6 +232,9 @@ export async function listCcsModels(): Promise<Map<string, DroidCustomModel>> {
|
||||
* Removes ccs-* entries whose profile no longer exists in active profiles.
|
||||
*/
|
||||
export async function pruneOrphanedModels(activeProfiles: string[]): Promise<number> {
|
||||
// Validate all profile names before pruning
|
||||
activeProfiles.forEach((profile) => validateProfileName(profile));
|
||||
|
||||
const settingsPath = getSettingsPath();
|
||||
if (!fs.existsSync(settingsPath)) return 0;
|
||||
|
||||
|
||||
@@ -22,10 +22,15 @@ export function detectDroidCli(): string | null {
|
||||
if (process.env.CCS_DROID_PATH) {
|
||||
const customPath = expandPath(process.env.CCS_DROID_PATH);
|
||||
if (fs.existsSync(customPath)) {
|
||||
return customPath;
|
||||
if (fs.statSync(customPath).isFile()) {
|
||||
return customPath;
|
||||
}
|
||||
console.warn('[!] CCS_DROID_PATH points to a directory, not a file:', customPath);
|
||||
console.warn(' Falling back to system PATH lookup...');
|
||||
} else {
|
||||
console.warn('[!] Warning: CCS_DROID_PATH is set but file not found:', customPath);
|
||||
console.warn(' Falling back to system PATH lookup...');
|
||||
}
|
||||
console.warn('[!] Warning: CCS_DROID_PATH is set but file not found:', customPath);
|
||||
console.warn(' Falling back to system PATH lookup...');
|
||||
}
|
||||
|
||||
// Priority 2: Resolve 'droid' from PATH
|
||||
|
||||
Reference in New Issue
Block a user