Skip to content

PR #6: Exit Code Standardization & --fail-on Flag#396

Merged
shivasurya merged 3 commits into
mainfrom
shiva/output-exit-codes
Nov 22, 2025
Merged

PR #6: Exit Code Standardization & --fail-on Flag#396
shivasurya merged 3 commits into
mainfrom
shiva/output-exit-codes

Conversation

@shivasurya
Copy link
Copy Markdown
Owner

Summary

Implements standardized exit codes and the --fail-on flag for both scan and ci commands, enabling selective CI/CD pipeline failures based on security finding severities.

Changes

Core Exit Code Logic (output/exit_code.go)

  • Exit Code Constants:
    • ExitCodeSuccess (0): No findings or no --fail-on match
    • ExitCodeFindings (1): Findings match --fail-on severities
    • ExitCodeError (2): Configuration or execution errors
  • DetermineExitCode(): Calculates appropriate exit code with error precedence
  • ParseFailOn(): Parses comma-separated severity values
  • ValidateSeverities(): Validates severity names (case-insensitive)
  • InvalidSeverityError: Custom error type for validation failures

Command Integration

  • scan command: Add --fail-on flag and integrate exit code logic
  • ci command: Add --fail-on flag and integrate exit code logic
  • Both commands now:
    • Exit 0 by default (regardless of findings)
    • Exit 1 only when findings match --fail-on severities
    • Exit 2 on configuration/execution errors
    • Support case-insensitive severity validation

Bug Fixes

  • Fixed SARIF output always exiting 0 (now respects --fail-on)

Testing

Unit Tests (output/exit_code_test.go)

  • 16 tests for DetermineExitCode() covering all exit scenarios
  • 12 tests for ParseFailOn() covering edge cases
  • 13 tests for ValidateSeverities() covering validation
  • All tests verify case-insensitive behavior

Integration Tests (cmd/exit_code_integration_test.go)

  • Tests actual binary exit codes for both scan and ci commands
  • Tests all output formats (SARIF, JSON, CSV)
  • Tests invalid severity handling
  • Tests case-insensitive severity matching
  • Requires INTEGRATION=1 and pre-built binary

Test Results: All tests passing ✅

$ gradle testGo
ok  	.../output	0.223s
ok  	.../cmd	0.317s

Examples

# Default: no exit on findings
pathfinder scan --rules rules/ --project .
# Exit: 0 (even if vulnerabilities found)

# Fail on critical findings
pathfinder scan --rules rules/ --project . --fail-on critical
# Exit: 1 if critical findings, 0 otherwise

# Fail on critical or high findings
pathfinder ci --rules rules/ --project . --output sarif --fail-on critical,high
# Exit: 1 if critical/high findings, 0 otherwise

# Case insensitive
pathfinder scan --rules rules/ --project . --fail-on CRITICAL,High,MeDiUm
# Exit: 1 if any match

# Invalid severity
pathfinder scan --rules rules/ --project . --fail-on invalid
# Error: invalid severity 'invalid', must be one of: critical, high, medium, low, info

Migration Notes

Breaking Changes

  • Default behavior changed: Previously, any findings caused exit 1. Now requires explicit --fail-on flag.
  • CI/CD pipelines: Add --fail-on critical,high to maintain previous fail-on-findings behavior.

Non-Breaking

  • Existing commands without --fail-on continue to work (exit 0)
  • All output formats work identically with exit codes

Checklist

  • Core exit code logic implemented
  • Integrated with scan command
  • Integrated with ci command
  • Unit tests (95%+ coverage)
  • Integration tests
  • All tests passing
  • Linter passing
  • Binary builds successfully
  • Documentation in commit messages

Stacked PRs

This PR stacks on top of:

🤖 Generated with Claude Code

@safedep
Copy link
Copy Markdown

safedep Bot commented Nov 22, 2025

SafeDep Report Summary

Green Malicious Packages Badge Green Vulnerable Packages Badge Green Risky License Badge

No dependency changes detected. Nothing to scan.

This report is generated by SafeDep Github App

@codecov
Copy link
Copy Markdown

codecov Bot commented Nov 22, 2025

Codecov Report

❌ Patch coverage is 63.79310% with 21 lines in your changes missing coverage. Please review.
✅ Project coverage is 79.76%. Comparing base (40c3ebe) to head (8eda45b).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
sourcecode-parser/cmd/ci.go 8.33% 11 Missing ⚠️
sourcecode-parser/cmd/scan.go 9.09% 10 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #396      +/-   ##
==========================================
- Coverage   79.76%   79.76%   -0.01%     
==========================================
  Files          78       79       +1     
  Lines        7799     7846      +47     
==========================================
+ Hits         6221     6258      +37     
- Misses       1333     1343      +10     
  Partials      245      245              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Copy Markdown
Owner Author

shivasurya commented Nov 22, 2025

Copy link
Copy Markdown
Owner Author

shivasurya commented Nov 22, 2025

Merge activity

  • Nov 22, 12:38 AM UTC: A user started a stack merge that includes this pull request via Graphite.
  • Nov 22, 12:45 AM UTC: Graphite rebased this pull request as part of a merge.
  • Nov 22, 12:46 AM UTC: @shivasurya merged this pull request with Graphite.

@shivasurya shivasurya changed the base branch from shiva/output-sarif-formatter to graphite-base/396 November 22, 2025 00:43
@shivasurya shivasurya changed the base branch from graphite-base/396 to main November 22, 2025 00:44
shivasurya and others added 3 commits November 22, 2025 00:45
Implement standardized exit codes for security scan results:
- Exit 0: Success (no findings or no fail-on match)
- Exit 1: Findings match fail-on severities
- Exit 2: Configuration or execution errors

Add core functionality:
- DetermineExitCode() with error precedence handling
- ParseFailOn() for comma-separated severity parsing
- ValidateSeverities() with case-insensitive validation
- InvalidSeverityError type for validation failures

All functions support case-insensitive severity matching for:
critical, high, medium, low, info.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
Add --fail-on flag to both scan and ci commands for controlling
exit codes based on finding severities.

Changes to scan command:
- Add --fail-on flag for severity-based exit control
- Track scan errors with scanErrors boolean
- Replace hardcoded exit(1) with DetermineExitCode()
- Validate --fail-on severities at startup

Changes to ci command:
- Add --fail-on flag matching scan behavior
- Track execution errors with hadErrors boolean
- Remove per-format exit code logic
- Centralize exit code determination after output formatting
- Validate --fail-on severities at startup

Both commands now:
- Exit 0 by default regardless of findings
- Exit 1 only when findings match --fail-on severities
- Exit 2 on configuration/execution errors
- Support case-insensitive severity validation

Fixes bug where SARIF output always exited 0.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
Add comprehensive integration tests verifying actual binary exit codes
across different scenarios and commands.

Test coverage:
- Clean projects (exit 0)
- Findings without --fail-on (exit 0)
- Findings matching --fail-on (exit 1)
- Findings not matching --fail-on (exit 0)
- Invalid severities (exit via error)
- Case-insensitive severity matching

Tests for both scan and ci commands across all output formats
(SARIF, JSON, CSV).

Integration tests require:
- INTEGRATION=1 environment variable
- Pre-built binary: gradle buildGo
- Test fixtures in test/fixtures/

All tests use context.WithTimeout for safety and proper cleanup.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@shivasurya shivasurya force-pushed the shiva/output-exit-codes branch from a160faa to 8eda45b Compare November 22, 2025 00:45
@shivasurya shivasurya merged commit 0e246ee into main Nov 22, 2025
3 checks passed
@shivasurya shivasurya deleted the shiva/output-exit-codes branch November 22, 2025 00:46
shivasurya added a commit that referenced this pull request Nov 22, 2025
## Summary

Final PR in the output standardization stack. Removes deprecated `query` and `analyze` commands and adds comprehensive documentation for all output formats, verbosity levels, and exit codes.

## Changes

### Deprecated Commands Removed
- **`cmd/query.go`** - Removed entirely
- **`cmd/query_test.go`** - Removed entirely
- **`cmd/analyze.go`** - Removed entirely
- **`main_test.go`** - Updated to remove references to deprecated commands

**BREAKING CHANGE**: No backward compatibility provided per requirements.

### Documentation Updates

#### README.md
- **Usage Examples**: Scan and CI command examples
- **Output Formats**: Text, JSON, CSV, SARIF examples with real output
- **Verbosity Levels**: Table showing default/verbose/debug behavior
- **Exit Codes**: Table and examples for exit code 0, 1, 2

#### docs/CLI.md (New)
- **Command Reference**: Complete flag documentation for all commands
- **Output Format Reference**: JSON schema, CSV columns, SARIF features
- **Exit Code Reference**: Detailed exit code behavior and --fail-on syntax

### Verification Tests
- **`cmd/command_cleanup_test.go`**: Integration tests verifying:
  - `query` command returns "unknown command"
  - `analyze` command returns "unknown command"
  - Help text no longer mentions removed commands

## Test Results

All tests passing ✅

```bash
$ gradle testGo
ok  	.../cmd	0.343s

$ ./build/go/pathfinder query
Error: unknown command "query" for "pathfinder"

$ ./build/go/pathfinder analyze
Error: unknown command "analyze" for "pathfinder"

$ ./build/go/pathfinder --help | grep -E "(query|analyze)"
# (no output - commands not shown)
```

## Documentation Examples

### Scan Command
```bash
pathfinder scan --rules rules/ --project /path/to/project
pathfinder scan --rules rules/ --project . --verbose
pathfinder scan --rules rules/ --project . --fail-on=critical,high
```

### CI Command
```bash
pathfinder ci --rules rules/ --project . --output json > results.json
pathfinder ci --rules rules/ --project . --output sarif > results.sarif
pathfinder ci --rules rules/ --project . --output csv --fail-on=critical
```

### Exit Code Behavior
```bash
# Default: always exit 0
pathfinder scan --rules rules/ --project .
echo $?  # 0 even with findings

# Fail on critical or high
pathfinder scan --rules rules/ --project . --fail-on=critical,high
echo $?  # 1 if critical/high found, 0 otherwise
```

## Migration Notes

### Breaking Changes
- **`query` command removed**: Use `scan` command instead
- **`analyze` command removed**: Use `scan` or `ci` command instead
- No migration path provided per requirements

### Non-Breaking
- All existing `scan` and `ci` commands continue to work
- Documentation is backwards compatible

## Checklist

- [x] query command removed
- [x] analyze command removed
- [x] main_test.go updated
- [x] Verification tests added
- [x] README.md updated with comprehensive docs
- [x] docs/CLI.md created
- [x] All tests passing
- [x] Linter passing
- [x] Binary builds successfully
- [x] Help text verified

## Stacked PRs

This PR stacks on top of:
- PR #6: Exit Code Standardization (#396)

This is the **final PR** in the output standardization feature stack.

## Verification

Commands removed successfully:
```bash
$ ./build/go/pathfinder query
Error: unknown command "query" for "pathfinder"

$ ./build/go/pathfinder analyze
Error: unknown command "analyze" for "pathfinder"
```

Valid commands work:
```bash
$ ./build/go/pathfinder scan --help
# Shows scan command help

$ ./build/go/pathfinder ci --help
# Shows ci command help
```

🤖 Generated with [Claude Code](https://claude.com/claude-code)
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