Repository navigation
fix: scan step stubs return ErrNotImplemented - #97
Merged
Merged
Conversation
Scan steps (scan_sast, scan_container, scan_deps) previously succeeded
silently without performing any work, giving false confidence in CI/CD
pipelines. Now they return a wrapped ErrNotImplemented sentinel error so
callers can detect the unimplemented state via errors.Is().
Changes:
- Add module/errors.go with ErrNotImplemented sentinel
- Update Execute() on ScanSASTStep, ScanContainerStep, ScanDepsStep to
return fmt.Errorf("<type> step %q: %w", name, ErrNotImplemented)
- Remove dead buildCommand() helpers and unused code paths
- Add documentation notes on each type and Execute method
- Add module/pipeline_step_scan_test.go with three tests verifying that
each Execute returns a wrapped ErrNotImplemented
Co-Authored-By: Claude Opus 4.6 <[email protected]>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR addresses issue #67 by making three security scan pipeline steps (scan_sast, scan_container, scan_deps) explicitly fail with ErrNotImplemented instead of silently succeeding without performing actual scans. This prevents false confidence in CI/CD pipelines where these stubs might be used.
Changes:
- Introduced
ErrNotImplementedsentinel error to clearly signal unimplemented functionality - Modified all three scan step
Execute()methods to return wrappedErrNotImplemented - Removed dead code (
buildCommand()helpers) and unused imports that supported the previous silent no-op behavior - Added comprehensive documentation explaining the limitation
- Added unit tests verifying the error behavior
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| module/errors.go | New file defining ErrNotImplemented sentinel error for unimplemented steps |
| module/pipeline_step_scan_sast.go | Returns ErrNotImplemented, removes dead buildCommand() and unused strings import, adds documentation |
| module/pipeline_step_scan_deps.go | Returns ErrNotImplemented, removes dead buildCommand() and unused strings import, adds documentation |
| module/pipeline_step_scan_container.go | Returns ErrNotImplemented, removes dead buildCommand(), adds documentation (keeps strings import for validateSeverity) |
| module/pipeline_step_scan_test.go | New test file with three tests verifying each scan step returns wrapped ErrNotImplemented |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes #67
The three security scan pipeline steps (
scan_sast,scan_container,scan_deps) previously executed silently without performing any actual scanning work, returning a successful result with an emptyScanResult. This gave false confidence in CI/CD pipelines — a pipeline using these steps would appear to pass security scans when no scanning occurred.module/errors.gowithErrNotImplementedsentinel errorExecute()on all three scan steps to returnfmt.Errorf("<step_type> step %q: %w", name, ErrNotImplemented)so callers can useerrors.Is()to check the specific errorbuildCommand()helpers and associated unused imports that were only called fromExecute()Execute()method explaining the limitationmodule/pipeline_step_scan_test.gowith three tests verifying each step'sExecutereturns a wrappedErrNotImplementedTest plan
go build ./...passesgolangci-lint runreports 0 issuesgo test ./module/...passes — new testsTestScanSASTStep_ExecuteReturnsErrNotImplemented,TestScanContainerStep_ExecuteReturnsErrNotImplemented,TestScanDepsStep_ExecuteReturnsErrNotImplementedall passplugins/cicdtests continue to pass (step factories still register correctly)🤖 Generated with Claude Code