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:
Tam Nhu Tran
2026-02-16 15:32:34 +07:00
parent 7d7054e2c0
commit 3191a4ab38
5 changed files with 71 additions and 22 deletions
+3 -3
View File
@@ -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
View File
@@ -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;
}
+8 -2
View File
@@ -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);
});
}
+27
View File
@@ -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;
+8 -3
View File
@@ -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