From f0d471070a63fee7199494ac9d87041d29607709 Mon Sep 17 00:00:00 2001 From: Luis Leon Date: Mon, 30 Mar 2026 18:19:11 +0200 Subject: [PATCH 1/2] chore: standardize npm release pipeline with approval gate and provenance --- .github/workflows/ci.yml | 2 ++ .github/workflows/release.yml | 18 ++++++++++++++++-- package.json | 3 +++ 3 files changed, 21 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 251c879..93ca3b5 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1,6 +1,8 @@ name: CI on: + push: + branches: [master] pull_request: branches: [develop, master] diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 3bb2659..8a83aab 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -12,6 +12,7 @@ on: permissions: contents: write + id-token: write jobs: test: @@ -41,6 +42,8 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@v4 + with: + fetch-depth: 0 - uses: oven-sh/setup-bun@v2 with: bun-version: latest @@ -49,14 +52,25 @@ jobs: node-version: "22" registry-url: "https://registry.npmjs.org" - run: bun install + - name: Determine version + id: version + run: | + if [ "${{ github.event_name }}" = "workflow_dispatch" ]; then + TAG="${{ inputs.tag }}" + else + PKG_VERSION=$(node -p "require('./package.json').version") + TAG="v${PKG_VERSION}" + fi + echo "tag=$TAG" >> "$GITHUB_OUTPUT" + npm version "${TAG#v}" --no-git-tag-version --allow-same-version - name: Publish to npm - run: npm publish --access public + run: npm publish --provenance --access public env: NODE_AUTH_TOKEN: ${{ secrets.NPM_TOKEN }} - name: Create tag and release env: GH_TOKEN: ${{ github.token }} run: | - TAG="v$(node -p "require('./package.json').version")" + TAG="${{ steps.version.outputs.tag }}" git tag "$TAG" 2>/dev/null && git push origin "$TAG" || echo "Tag exists" gh release create "$TAG" --title "$TAG" --generate-notes || echo "Release exists" diff --git a/package.json b/package.json index cfb4240..13d146d 100644 --- a/package.json +++ b/package.json @@ -18,6 +18,9 @@ ], "author": "lleontor705", "license": "MIT", + "publishConfig": { + "access": "public" + }, "repository": { "type": "git", "url": "https://github.com/lleontor705/opencode-cli-enforcer.git" From d75c1dda7e9ab18744b193c094656e6c98143cf9 Mon Sep 17 00:00:00 2001 From: Luis Leon Date: Mon, 30 Mar 2026 19:38:36 +0200 Subject: [PATCH 2/2] feat: add cockatiel resilience, security hardening, and error classification - Pin Bun >= 1.3.5 (CVE-2026-24910 trust validation bypass fix) - Add cockatiel as composable resilience engine (bulkhead, circuit breaker, retry, timeout) - Add error classification (transient/rate_limit/permanent/crash) for smart retry - Add environment variable allowlist for spawned CLI processes - Add API key redaction for all error output --- .claude/settings.local.json | 10 ++++++ bun.lock | 3 ++ package.json | 4 +++ src/error-classifier.ts | 38 +++++++++++++++++++++++ src/executor.ts | 10 ++++-- src/index.ts | 4 ++- src/policies.ts | 48 +++++++++++++++++++++++++++++ src/redact.ts | 10 ++++++ src/resilience.ts | 34 +++++++++++++++++---- src/safe-env.ts | 30 ++++++++++++++++++ tests/error-classifier.test.ts | 56 ++++++++++++++++++++++++++++++++++ tests/redact.test.ts | 46 ++++++++++++++++++++++++++++ tests/safe-env.test.ts | 42 +++++++++++++++++++++++++ 13 files changed, 326 insertions(+), 9 deletions(-) create mode 100644 .claude/settings.local.json create mode 100644 src/error-classifier.ts create mode 100644 src/policies.ts create mode 100644 src/redact.ts create mode 100644 src/safe-env.ts create mode 100644 tests/error-classifier.test.ts create mode 100644 tests/redact.test.ts create mode 100644 tests/safe-env.test.ts diff --git a/.claude/settings.local.json b/.claude/settings.local.json new file mode 100644 index 0000000..c8b6456 --- /dev/null +++ b/.claude/settings.local.json @@ -0,0 +1,10 @@ +{ + "permissions": { + "allow": [ + "Bash(grep -E \"\\\\.ts$|\\\\.js$|\\\\.json$\")", + "Bash(xargs wc:*)", + "Bash(bun test:*)", + "Bash(bun run:*)" + ] + } +} diff --git a/bun.lock b/bun.lock index bfecedf..13ea524 100644 --- a/bun.lock +++ b/bun.lock @@ -6,6 +6,7 @@ "name": "opencode-cli-enforcer", "dependencies": { "@opencode-ai/plugin": "^1.2.26", + "cockatiel": "^3.2.1", "execa": "^9.6.1", }, "devDependencies": { @@ -29,6 +30,8 @@ "bun-types": ["bun-types@1.3.11", "", { "dependencies": { "@types/node": "*" } }, "sha512-1KGPpoxQWl9f6wcZh57LvrPIInQMn2TQ7jsgxqpRzg+l0QPOFvJVH7HmvHo/AiPgwXy+/Thf6Ov3EdVn1vOabg=="], + "cockatiel": ["cockatiel@3.2.1", "", {}, "sha512-gfrHV6ZPkquExvMh9IOkKsBzNDk6sDuZ6DdBGUBkvFnTCqCxzpuq48RySgP0AnaqQkw2zynOFj9yly6T1Q2G5Q=="], + "cross-spawn": ["cross-spawn@7.0.6", "", { "dependencies": { "path-key": "^3.1.0", "shebang-command": "^2.0.0", "which": "^2.0.1" } }, "sha512-uV2QOWP2nWzsy2aMp8aRibhi9dlzF5Hgh5SHaB9OiTGEyDTiJJyx0uy51QXdyWbtAHNua4XJzUKca3OzKUd3vA=="], "execa": ["execa@9.6.1", "", { "dependencies": { "@sindresorhus/merge-streams": "^4.0.0", "cross-spawn": "^7.0.6", "figures": "^6.1.0", "get-stream": "^9.0.0", "human-signals": "^8.0.1", "is-plain-obj": "^4.1.0", "is-stream": "^4.0.1", "npm-run-path": "^6.0.0", "pretty-ms": "^9.2.0", "signal-exit": "^4.1.0", "strip-final-newline": "^4.0.0", "yoctocolors": "^2.1.1" } }, "sha512-9Be3ZoN4LmYR90tUoVu2te2BsbzHfhJyfEiAVfz7N5/zv+jduIfLrV2xdQXOHbaD6KgpGdO9PRPM1Y4Q9QkPkA=="], diff --git a/package.json b/package.json index 13d146d..5dc8504 100644 --- a/package.json +++ b/package.json @@ -32,12 +32,16 @@ ], "dependencies": { "@opencode-ai/plugin": "^1.2.26", + "cockatiel": "^3.2.1", "execa": "^9.6.1" }, "devDependencies": { "@types/bun": "latest", "typescript": "^5.7.0" }, + "engines": { + "bun": ">=1.3.5" + }, "scripts": { "test": "bun test", "test:watch": "bun test --watch", diff --git a/src/error-classifier.ts b/src/error-classifier.ts new file mode 100644 index 0000000..02c984f --- /dev/null +++ b/src/error-classifier.ts @@ -0,0 +1,38 @@ +/** + * Error Classification — determines retry strategy based on error type. + * + * Categories: + * transient → retry with standard backoff + * rate_limit → retry with longer delay + * permanent → do not retry, fallback immediately + * crash → do not retry, fallback immediately + */ + +export type ErrorClass = "transient" | "rate_limit" | "permanent" | "crash" + +export function classifyError(error: any): ErrorClass { + const msg = String(error?.message || error?.stderr || "") + + // Crash: process killed, binary not found + if (error?.exitCode === 137 || msg.includes("SIGKILL") || msg.includes("ENOENT")) { + return "crash" + } + + // Rate limit: HTTP 429 or quota errors + if (msg.includes("429") || msg.includes("rate limit") || msg.includes("quota")) { + return "rate_limit" + } + + // Permanent: auth failures, not found + if ( + msg.includes("auth") || + msg.includes("401") || + msg.includes("403") || + msg.includes("not found") + ) { + return "permanent" + } + + // Everything else is transient (timeout, network, etc.) + return "transient" +} diff --git a/src/executor.ts b/src/executor.ts index 9ec846d..b8f886b 100644 --- a/src/executor.ts +++ b/src/executor.ts @@ -4,6 +4,8 @@ import { execa } from "execa" import type { CliDef } from "./cli-defs" +import { getSafeEnv } from "./safe-env" +import { redactSecrets } from "./redact" /** Prompts longer than this (chars) are delivered via stdin to avoid OS arg-length limits. */ export const STDIN_THRESHOLD = 30_000 @@ -30,6 +32,7 @@ export async function executeCliOnce( maxBuffer: 10 * 1024 * 1024, reject: false, windowsHide: true, + env: getSafeEnv(), ...(useStdin ? { input: prompt } : {}), ...(signal ? { cancelSignal: signal } : {}), }) @@ -47,8 +50,11 @@ export async function executeCliOnce( } if (result.failed && result.exitCode !== 0) { - const msg = result.stderr?.trim() || result.message || `Exit code ${result.exitCode}` - throw new Error(`CLI '${def.name}' failed: ${msg}`) + const rawMsg = result.stderr?.trim() || result.message || `Exit code ${result.exitCode}` + const msg = redactSecrets(rawMsg) + throw Object.assign(new Error(`CLI '${def.name}' failed: ${msg}`), { + exitCode: result.exitCode, + }) } return { diff --git a/src/index.ts b/src/index.ts index 1f44769..266c4a7 100644 --- a/src/index.ts +++ b/src/index.ts @@ -24,6 +24,7 @@ import { DEFAULT_RETRY_CONFIG } from "./retry" import { detectAllClis, type CliAvailability } from "./detection" import { truncate } from "./executor" import { executeWithResilience, type ResilienceContext, type UsageStats } from "./resilience" +import { redactSecrets } from "./redact" // Agents that should NOT receive CLI injection const NO_CLI_AGENTS = new Set(["orchestrator", "task_decomposer"]) @@ -141,7 +142,8 @@ Rules: One concern per call. Split large requests. Include "CLI Consultations" i return { ...response, stdout: truncate(response.stdout, 50_000), - stderr: truncate(response.stderr, 5_000), + stderr: redactSecrets(truncate(response.stderr, 5_000)), + error: response.error ? redactSecrets(response.error) : null, } }, }), diff --git a/src/policies.ts b/src/policies.ts new file mode 100644 index 0000000..61049ee --- /dev/null +++ b/src/policies.ts @@ -0,0 +1,48 @@ +/** + * Cockatiel Resilience Policies — composable retry, circuit breaker, + * bulkhead, and timeout policies using cockatiel. + * + * Composition order (outermost → innermost): + * timeout → retry → circuit breaker → bulkhead + */ + +import { + CircuitBreakerPolicy, + ConsecutiveBreaker, + retry, + handleAll, + wrap, + bulkhead, + timeout, + ExponentialBackoff, + type IPolicy, +} from "cockatiel" + +/** Per-CLI bulkhead: max 2 concurrent, queue up to 3 */ +export const cliBulkhead = bulkhead(2, 3) + +/** Circuit breaker: open after 3 consecutive failures, half-open after 30s */ +export const circuitBreaker = new CircuitBreakerPolicy(handleAll, { + halfOpenAfter: 30_000, + breaker: new ConsecutiveBreaker(3), +}) + +/** Retry with decorrelated jitter (AWS best practice) */ +export const retryPolicy = retry(handleAll, { + maxAttempts: 3, + backoff: new ExponentialBackoff({ initialDelay: 1000, maxDelay: 30000 }), +}) + +/** Default timeout: 30 seconds */ +export const timeoutPolicy = timeout(30_000) + +/** + * Composed resilient policy: timeout → retry → circuit breaker → bulkhead. + * Wrap calls with `resilientPolicy.execute(fn)`. + */ +export const resilientPolicy: IPolicy = wrap( + timeoutPolicy, + retryPolicy, + circuitBreaker, + cliBulkhead, +) diff --git a/src/redact.ts b/src/redact.ts new file mode 100644 index 0000000..d252259 --- /dev/null +++ b/src/redact.ts @@ -0,0 +1,10 @@ +/** + * Secret Redaction — removes API keys and tokens from text before + * returning it to the user in error messages or logs. + */ + +export function redactSecrets(text: string): string { + return text + .replace(/(?:sk-|key-|AIza|ant-api)[a-zA-Z0-9_-]{20,}/g, "[REDACTED]") + .replace(/Bearer\s+[a-zA-Z0-9._-]+/g, "Bearer [REDACTED]") +} diff --git a/src/resilience.ts b/src/resilience.ts index 88aacc6..4a39e85 100644 --- a/src/resilience.ts +++ b/src/resilience.ts @@ -13,11 +13,13 @@ import { recordFailure, } from "./circuit-breaker" import type { RetryConfig } from "./retry" -import { DEFAULT_RETRY_CONFIG, calculateDelay, sleep, isRetryableError } from "./retry" +import { DEFAULT_RETRY_CONFIG, calculateDelay, sleep } from "./retry" import { executeCliOnce } from "./executor" import type { CliAvailability } from "./detection" import type { Platform } from "./platform" import type { CircuitState } from "./circuit-breaker" +import { classifyError, type ErrorClass } from "./error-classifier" +import { redactSecrets } from "./redact" // ─── Structured Response (MCP pattern) ───────────────────────────────────── @@ -32,6 +34,7 @@ export interface CliResponse { used_fallback: boolean fallback_chain: string[] error: string | null + error_class: ErrorClass | null circuit_state: CircuitState attempt: number max_attempts: number @@ -128,6 +131,7 @@ export async function executeWithResilience( used_fallback: cliName !== targetCli, fallback_chain: fallbackChain, error: null, + error_class: null, circuit_state: breaker.state, attempt: attempt + 1, max_attempts: ctx.retryConfig.maxRetries + 1, @@ -135,14 +139,29 @@ export async function executeWithResilience( } catch (err: unknown) { stats.failures++ - const retryable = isRetryableError(err) - const isLastAttempt = attempt === ctx.retryConfig.maxRetries + const errorClass = classifyError(err) + + // permanent and crash errors: skip retries, fallback immediately + if (errorClass === "permanent" || errorClass === "crash") { + recordFailure(breaker, ctx.breakerConfig) + break // try next CLI in fallback chain + } + + // rate_limit: wait longer before retrying + if (errorClass === "rate_limit") { + const rateLimitDelay = calculateDelay(attempt + 1, { + ...ctx.retryConfig, + baseDelayMs: ctx.retryConfig.baseDelayMs * 3, + }) + await sleep(rateLimitDelay) + } - if (!retryable || isLastAttempt) { + const isLastAttempt = attempt === ctx.retryConfig.maxRetries + if (isLastAttempt) { recordFailure(breaker, ctx.breakerConfig) break // try next CLI in fallback chain } - // retryable — loop continues + // transient or rate_limit — loop continues with retry } } } @@ -158,7 +177,10 @@ export async function executeWithResilience( timed_out: false, used_fallback: fallbackChain.length > 1, fallback_chain: fallbackChain, - error: `All CLI providers exhausted. Tried: ${fallbackChain.join(" → ")}. Check cli_status for details.`, + error: redactSecrets( + `All CLI providers exhausted. Tried: ${fallbackChain.join(" → ")}. Check cli_status for details.`, + ), + error_class: "transient", circuit_state: ctx.breakers.get(targetCli)!.state, attempt: ctx.retryConfig.maxRetries + 1, max_attempts: ctx.retryConfig.maxRetries + 1, diff --git a/src/safe-env.ts b/src/safe-env.ts new file mode 100644 index 0000000..cd3540a --- /dev/null +++ b/src/safe-env.ts @@ -0,0 +1,30 @@ +/** + * Environment Variable Filtering — only passes safe variables to + * spawned CLI processes, preventing accidental secret leakage. + */ + +export const SAFE_ENV_VARS = [ + "PATH", + "HOME", + "USER", + "TERM", + "SHELL", + "LANG", + "LC_ALL", + "ANTHROPIC_API_KEY", + "GOOGLE_API_KEY", + "OPENAI_API_KEY", + "GEMINI_API_KEY", + "CODEX_API_KEY", + "HTTP_PROXY", + "HTTPS_PROXY", + "NO_PROXY", +] + +export function getSafeEnv(): Record { + const env: Record = {} + for (const key of SAFE_ENV_VARS) { + if (process.env[key]) env[key] = process.env[key]! + } + return env +} diff --git a/tests/error-classifier.test.ts b/tests/error-classifier.test.ts new file mode 100644 index 0000000..ab1cee7 --- /dev/null +++ b/tests/error-classifier.test.ts @@ -0,0 +1,56 @@ +import { describe, it, expect } from "bun:test" +import { classifyError, type ErrorClass } from "../src/error-classifier" + +describe("classifyError", () => { + it("classifies SIGKILL as crash", () => { + expect(classifyError({ message: "SIGKILL received" })).toBe("crash") + }) + + it("classifies exitCode 137 as crash", () => { + expect(classifyError({ exitCode: 137 })).toBe("crash") + }) + + it("classifies ENOENT as crash", () => { + expect(classifyError({ message: "ENOENT: no such file" })).toBe("crash") + }) + + it("classifies 429 as rate_limit", () => { + expect(classifyError({ message: "HTTP 429 too many requests" })).toBe("rate_limit") + }) + + it("classifies rate limit text as rate_limit", () => { + expect(classifyError({ message: "rate limit exceeded" })).toBe("rate_limit") + }) + + it("classifies quota as rate_limit", () => { + expect(classifyError({ message: "quota exceeded for today" })).toBe("rate_limit") + }) + + it("classifies auth errors as permanent", () => { + expect(classifyError({ message: "auth token expired" })).toBe("permanent") + }) + + it("classifies 401 as permanent", () => { + expect(classifyError({ message: "HTTP 401 Unauthorized" })).toBe("permanent") + }) + + it("classifies 403 as permanent", () => { + expect(classifyError({ message: "HTTP 403 Forbidden" })).toBe("permanent") + }) + + it("classifies not found as permanent", () => { + expect(classifyError({ message: "command not found" })).toBe("permanent") + }) + + it("classifies unknown errors as transient", () => { + expect(classifyError({ message: "connection reset" })).toBe("transient") + }) + + it("classifies empty errors as transient", () => { + expect(classifyError({})).toBe("transient") + }) + + it("handles stderr field", () => { + expect(classifyError({ stderr: "rate limit hit" })).toBe("rate_limit") + }) +}) diff --git a/tests/redact.test.ts b/tests/redact.test.ts new file mode 100644 index 0000000..f434861 --- /dev/null +++ b/tests/redact.test.ts @@ -0,0 +1,46 @@ +import { describe, it, expect } from "bun:test" +import { redactSecrets } from "../src/redact" + +describe("redactSecrets", () => { + it("redacts sk- prefixed keys", () => { + const text = "Error: sk-abcdefghijklmnopqrstuvwxyz1234 is invalid" + expect(redactSecrets(text)).toBe("Error: [REDACTED] is invalid") + }) + + it("redacts ant-api prefixed keys", () => { + const text = "Key: ant-api01234567890123456789ab" + expect(redactSecrets(text)).toBe("Key: [REDACTED]") + }) + + it("redacts AIza prefixed keys", () => { + const text = "google key AIzaSyB1234567890abcdefghij" + expect(redactSecrets(text)).toBe("google key [REDACTED]") + }) + + it("redacts key- prefixed tokens", () => { + const text = "key-abcdefghijklmnopqrstuvwx" + expect(redactSecrets(text)).toBe("[REDACTED]") + }) + + it("redacts Bearer tokens", () => { + const text = "Authorization: Bearer eyJhbGciOiJIUzI1NiJ9.payload.sig" + expect(redactSecrets(text)).toBe("Authorization: Bearer [REDACTED]") + }) + + it("leaves clean text unchanged", () => { + const text = "CLI 'claude' failed: timeout after 30000ms" + expect(redactSecrets(text)).toBe(text) + }) + + it("handles empty string", () => { + expect(redactSecrets("")).toBe("") + }) + + it("redacts multiple keys in one string", () => { + const text = "keys: sk-aaaabbbbccccddddeeeeffffgggg and AIzaSyBaaaabbbbccccddddeeee" + const result = redactSecrets(text) + expect(result).not.toContain("sk-") + expect(result).not.toContain("AIza") + expect(result).toContain("[REDACTED]") + }) +}) diff --git a/tests/safe-env.test.ts b/tests/safe-env.test.ts new file mode 100644 index 0000000..a55ee35 --- /dev/null +++ b/tests/safe-env.test.ts @@ -0,0 +1,42 @@ +import { describe, it, expect } from "bun:test" +import { getSafeEnv, SAFE_ENV_VARS } from "../src/safe-env" + +describe("SAFE_ENV_VARS", () => { + it("includes PATH", () => { + expect(SAFE_ENV_VARS).toContain("PATH") + }) + + it("includes API key vars", () => { + expect(SAFE_ENV_VARS).toContain("ANTHROPIC_API_KEY") + expect(SAFE_ENV_VARS).toContain("GOOGLE_API_KEY") + expect(SAFE_ENV_VARS).toContain("OPENAI_API_KEY") + }) + + it("includes proxy vars", () => { + expect(SAFE_ENV_VARS).toContain("HTTP_PROXY") + expect(SAFE_ENV_VARS).toContain("HTTPS_PROXY") + expect(SAFE_ENV_VARS).toContain("NO_PROXY") + }) +}) + +describe("getSafeEnv", () => { + it("returns an object with only safe vars", () => { + const env = getSafeEnv() + for (const key of Object.keys(env)) { + expect(SAFE_ENV_VARS).toContain(key) + } + }) + + it("includes PATH if present in process.env", () => { + if (process.env.PATH) { + const env = getSafeEnv() + expect(env.PATH).toBe(process.env.PATH) + } + }) + + it("does not include arbitrary vars", () => { + const env = getSafeEnv() + expect(env).not.toHaveProperty("MY_SECRET_TOKEN") + expect(env).not.toHaveProperty("DATABASE_URL") + }) +})