fix: security audit findings - critical + high + medium + low

[ryanmcmillan]

Critical

• Bind generated docker-compose bootstrap API to 127.0.0.1 by default.
• Print a localhost-only notice after Docker startup.

High

• Fail fast on corrupted ~/.delega/config.json instead of silently falling back.
• Stop passing macOS keychain secrets in process argv by using a 0600 temp file.
• Raise the CLI Node engine floor to >=20 to match commander@14.

Medium

• Use getApiUrl() in login for the correct env/config precedence.
• Remove the non-functional init --api-url help example.
• Make agents rotate --dry-run tolerate 404s via apiRequest().
• Validate numeric task flags instead of plain parseInt.
• Prevent tasks show from crashing on malformed labels.
• Give each status fetch its own 15s timeout budget.

Low

• Add whoami --json output.
• Fix the README delegate example to include --content.
• Ignore generated docker-compose.yml and .tgz artifacts.
• Switch npm run dev to npx tsx.

[greptile-apps]

Greptile Summary

This PR addresses a batch of security audit findings across the CLI, touching secret storage, network binding, config error handling, input validation, and several medium/low quality-of-life fixes. The changes are generally well-scoped and well-motivated, but one of the "High" severity fixes — the macOS keychain write path — contains an implementation gap that leaves the plaintext API key still exposed in the security subprocess's process arguments.

Key changes:

• src/templates/docker-compose.yml: Port binding restricted to 127.0.0.1 by default (critical fix — correct).
• src/config.ts: loadConfig() now throws on corrupt JSON instead of silently returning {} (correct).
• src/secret-store.ts: writeMacosKeychain switches from execFileSync + direct -w <key> arg to a 0600 temp file + execSync with $(cat file) shell substitution. However, the shell command substitution still expands the key into security's argv, so the key remains visible in process listings during the brief window security runs — the stated security goal is not fully achieved.
• src/commands/status.ts: Shared AbortController replaced with per-request AbortSignal.timeout(15_000), giving each of the three fetch calls an independent 15-second budget (correct improvement).
• src/commands/login.ts: Replaces manual config.api_url resolution with getApiUrl() for proper env/config precedence (correct).
• src/commands/tasks.ts: parsePriority and parsePositiveInt validators added for --priority and --limit; tasks show labels display hardened against malformed JSON strings (correct).
• src/commands/agents.ts: --dry-run switch from apiCall to apiRequest so 404s on unknown agents are tolerated gracefully (correct).
• src/commands/whoami.ts: --json flag added (correct).
• README.md: The delegate example fix introduces a duplicate near-identical example line.

Confidence Score: 3/5

• Safe to merge with one targeted fix: the macOS keychain write path still exposes the API key in the security subprocess's argv, contradicting the stated "High" severity fix goal.
• The vast majority of the PR's 15+ changes are correct and well-implemented. The one substantive issue is that writeMacosKeychain uses execSync with $(cat file) shell substitution, which still passes the plaintext key as -w to the security subprocess — visible in ps. This is exactly the issue the PR claims to fix at "High" severity. The README duplicate is minor (P2). Everything else (localhost binding, corrupt-config fail-fast, per-request timeouts, numeric validation, labels crash fix) is solid and correct.
• src/secret-store.ts — writeMacosKeychain still exposes the secret in the security subprocess argv

Important Files Changed

Filename	Overview
src/secret-store.ts	writeMacosKeychain is refactored to use a 0600 temp file and shell $(cat …) substitution, but the key still ends up in the security subprocess's argv — the stated goal of keeping the secret out of process arguments is not fully achieved.
src/config.ts	loadConfig now throws on corrupted JSON instead of silently returning {}; getApiUrl() centralises URL resolution. Both changes are clean and correct.
src/commands/login.ts	Replaces manual config/URL wiring with getApiUrl() and wraps loadConfig/getApiUrl in a try/catch that exits on corruption. Logic is sound; TypeScript control-flow correctly handles the process.exit(1) in the catch so config is always assigned by line 137.
src/commands/status.ts	Replaces a single shared AbortController/clearTimeout pattern with per-request AbortSignal.timeout(15_000), giving each of the three fetch calls its own independent 15-second budget. A clear improvement.
src/commands/agents.ts	--dry-run now uses apiRequest instead of apiCall so a 404 for an unknown agent is tolerated gracefully instead of exiting; agent display name improved.
src/commands/tasks.ts	parsePriority and parsePositiveInt validators replace bare parseInt for --priority and --limit flags; labels display is hardened against malformed JSON strings. All changes correct.
src/commands/whoami.ts	Adds --json flag; the fallback code path (when /agent/me returns 404) also checks opts.json before outputting. Logic is consistent and correct.
src/api.ts	getApiKey() is now wrapped in try/catch so a corrupted config.json produces a friendly error message instead of an uncaught exception.
src/templates/docker-compose.yml	Port binding tightened from 0.0.0.0 (all interfaces) to 127.0.0.1 (localhost only) by default; comment updated to explain how to expose on the network.
src/commands/init.ts	Adds a post-startup notice that Delega is bound to localhost only; removes the non-functional --api-url help example from the help text.
README.md	Fixes the delegate example to include --content, but introduces a duplicate near-identical example line that should be cleaned up.
package.json	Node engine floor raised from >=18 to >=20 to match commander@14 requirements; dev script switches from ts-node to npx tsx.

Sequence Diagram

sequenceDiagram
    participant CLI as delega CLI
    participant Config as config.ts (loadConfig / getApiUrl)
    participant Store as secret-store.ts (writeMacosKeychain)
    participant Shell as /bin/sh (execSync)
    participant Security as security (macOS CLI)
    participant Keychain as macOS Keychain

    CLI->>Config: loadConfig() — throws on corrupt JSON (NEW)
    Config-->>CLI: DelegaConfig or Error

    CLI->>Config: getApiUrl() — env → config → default (NEW unified path)
    Config-->>CLI: normalized API URL

    CLI->>Store: storeApiKey(apiKey)
    Store->>Store: writeFileSync(tmpPath, apiKey, mode 0o600)
    Store->>Shell: execSync("security ... -w $(cat tmpPath)")
    Note over Shell,Security: Shell expands $(cat tmpPath) → key still in security argv ⚠️
    Shell->>Security: security add-generic-password -w <plaintext_key>
    Security->>Keychain: store key
    Security-->>Shell: exit 0
    Store->>Store: unlinkSync(tmpPath) [finally]

Loading

_{Reviews (1): Last reviewed commit: "fix: security audit fixes (docker bind, ..." | Re-trigger Greptile}

[greptile-apps]

Secret still appears in security process argv

The stated goal of this fix is "Stop passing macOS keychain secrets in process argv," but the implementation doesn't fully achieve it.

The shell command substitution "$(cat '${tmpPath}')" reads the key from the temp file and substitutes its value directly into the argument string before passing it to security. When execSync runs, the shell spawns security with -w <actual_key_value> in its argv — which is still visible in ps -ef output during the brief window that security runs.

The original code:

execFileSync("security", [..., "-w", apiKey])

…and the new code both result in the same state: security's process argv contains -w <plaintext_key>.

To truly keep the key out of all child-process argv, consider piping the key via stdin to the security command (many macOS security commands accept password input from stdin or a prompt):

// Alternative: use execFileSync without -w in argv and pipe via stdin
// Note: verify whether `security add-generic-password` supports this on your target macOS version.
node_child_process.execFileSync(
  "security",
  ["add-generic-password", "-U", "-a", ACCOUNT_NAME, "-s", SERVICE_NAME, "-w"],
  { input: apiKey, encoding: "utf-8", stdio: ["pipe", "ignore", "ignore"] },
);

If stdin is not supported, document the limitation rather than suggesting it is fixed.

[greptile-apps]

Duplicate delegate example lines

The fix adds a new example line for delegate with --content "description", but the existing line immediately below it (--content "subtask description") already demonstrates the same pattern. This leaves two nearly identical lines in the README.

Suggested change

	delega tasks delegate <task-id> <agent-id> --content "description"
	delega tasks delegate <task-id> <agent-id> --content "subtask description"
	delega tasks delegate <task-id> <agent-id> --content "subtask description"

One of the two lines should be removed (the older example already includes --content and is sufficient).

_{Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!}

[ryanmcmillan]

/greptile review