Skip to content

feat: hardening#18

Merged
kzhivotov merged 1 commit intomainfrom
feat/hardening
Mar 22, 2026
Merged

feat: hardening#18
kzhivotov merged 1 commit intomainfrom
feat/hardening

Conversation

@kzhivotov
Copy link
Copy Markdown
Contributor

@kzhivotov kzhivotov commented Mar 22, 2026

What

Harden the CLI with input validation, exec timeouts, structured error handling, and CI permission scoping.

Why

Multiple defense-in-depth improvements: process.exit() scattered across command handlers prevented testability and clean shutdown; execSync calls had no timeouts and could hang indefinitely on large repos; numeric CLI options (--timeout, --commits) silently accepted garbage input; CI workflow had overly broad default permissions.

How I tested

  • npm test passes
  • Tested against a real repo:
  • --dry-run output looks correct (if applicable)

Checklist

  • Changes are focused on a single feature or fix
  • Tests added or updated for any logic changes
  • No new dependencies added (or justified in the PR description)

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 22, 2026

Walkthrough

Introduces structured CLI error handling via a new CliError class, refactors command handlers to throw errors instead of calling process.exit(), adds SIGINT/SIGTERM signal handlers, enforces validation for --timeout and --commits options, and adds execution timeouts to long-running subprocess calls.

Changes

Cohort / File(s) Summary
Error Handling Framework
src/lib/errors.js
New CliError class with exitCode and logged properties for structured error propagation.
CLI Core
bin/cli.js
Added signal handlers for SIGINT/SIGTERM, custom parsers for --timeout (positive integer) and --commits (1–50), and error detection in the catch handler that respects err.logged and err.exitCode.
Command Handlers
src/commands/add.js, src/commands/customize.js, src/commands/doc-init.js, src/commands/doc-sync.js
Replaced process.exit() calls with throw new CliError(...) for error paths; replaced process.exit(0) with return for cancel/abort paths; refined timeout computation to check type before applying custom values.
Process Execution Timeouts
src/lib/context-builder.js, src/lib/runner.js
Added explicit timeout options to execSync calls (git log, which/where claude, git diff/status commands) to prevent unbounded blocking.
GitHub Actions
.github/workflows/ci.yml
Added top-level permissions: { contents: read } block.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested reviewers

  • mvoutov
🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 23.53% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive The title 'feat: hardening' is vague and does not convey the specific changes made, such as error handling improvements, timeout enforcement, or signal handlers. Use a more descriptive title that reflects the main changes, e.g., 'feat: add CLI error handling and execution timeouts' or 'feat: improve error handling with CliError and process signal handlers'.
✅ Passed checks (1 passed)
Check name Status Explanation
Description check ✅ Passed The PR description adequately addresses 'What' and 'Why' but the testing checklist items are unchecked and lack specifics.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/hardening

Comment @coderabbitai help to get the list of available commands and usage tips.

@kzhivotov kzhivotov merged commit 97c7ec9 into main Mar 22, 2026
3 checks passed
@kzhivotov kzhivotov deleted the feat/hardening branch March 22, 2026 17:11
This was referenced Mar 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant