Skip to content

fix(compose): surface per-service scan failures in user-facing summary - #160

Open
myukitty wants to merge 1 commit into
OWASP:mainfrom
myukitty:fix/surface-compose-scan-failures
Open

fix(compose): surface per-service scan failures in user-facing summary#160
myukitty wants to merge 1 commit into
OWASP:mainfrom
myukitty:fix/surface-compose-scan-failures

Conversation

@myukitty

@myukitty myukitty commented Aug 15, 2026

Copy link
Copy Markdown

Pull Request

Description

Rebased onto current main and rewritten now that #161 has merged.

#161 added failed_services tracking inside ComposeOrchestrator.run_full_scan, which covers the first two acceptance criteria of #131. Nothing reads that field back out, so the third one is still open: a compose run where some services could not be scanned still prints a Quick take and a security score as if every service had been covered. That is the misleading behaviour #131 was filed about.

This PR adds only the missing user-facing half, on top of #161:

  1. compose_scanner.py carries total_services alongside failed_services, so the summary has a denominator.
  2. cli.py prints one Quick take line — N of M services could not be scanned: <names> — and includes failed_services / total_services in --json output.
  3. A service that fails both its Dockerfile scan and its image scan is appended twice by run_full_scan, so names are de-duplicated before counting.

No change to exit codes, scoring, or scan behaviour — this is purely the visibility fix #131 asked for.

Before (current main, 2 of 3 services unscannable):

Quick take
  - 16 security findings (0 critical, 0 high)
  - Run without --scan-only to add AI-powered explanations and fixes

After:

Quick take
  - 16 security findings (0 critical, 0 high)
  - 2 of 3 services could not be scanned: web, db
  - Run without --scan-only to add AI-powered explanations and fixes

The per-service reason strings are available in --json under scan_info.failed_services; I kept them out of the one-line Quick take because they vary per service. Happy to inline the reason if you would rather match the issue's example wording exactly.

Closes #131

Type of Change

  • Bug fix

How Has This Been Tested?

  • Unit tests
  • Manual testing

Added 7 tests: 4 in tests/test_cli.py covering the summary line, the de-duplication, the no-total fallback, and silence when nothing failed; 3 in tests/test_compose_scanner.py covering total_services, populated failed_services, and the unparseable-compose path.

  • pytest tests/ — 254 passed, 2 skipped. The 2 failures in tests/test_utils.py on my machine are the optional docksec[ai] extras not being installed, and reproduce unchanged on main.
  • ruff check . — all checks passed.
  • Ran the real orchestrator and renderer against a 3-service compose file with 2 unresolvable images to produce the output above.

Test Configuration:

  • Python version: 3.12.13
  • Operating System: Windows 11
  • DockSec version: main @ 77c0cd1

Checklist

  • Code follows the style guidelines of this project
  • Self-review completed
  • Hard-to-understand areas are commented
  • No new warnings or errors introduced
  • Tests added that prove the fix works
  • All existing tests pass
  • Spelling checked

Related Issues / PRs


By submitting this pull request, I confirm that my contribution is made under the terms of the MIT license.

@github-actions github-actions Bot added cli Changes to the CLI entry point core Changes to core scanning logic tests Changes to the test suite labels Aug 15, 2026
@myukitty
myukitty force-pushed the fix/surface-compose-scan-failures branch from f76a1d8 to 04f8171 Compare August 17, 2026 17:27
@advaitpatel

Copy link
Copy Markdown
Collaborator

Hello @myukitty - can you please update your branch and PR and resubmit this PR again?

OWASP#161 records failed services in the compose results dict, but nothing
reads them back out. A run where some services could not be scanned
still prints a summary and a score as if every service had been
covered, which is the visibility gap OWASP#131 describes.

Add the missing user-facing half:

- carry total_services next to failed_services so the summary has a
  denominator, and
- print "N of M services could not be scanned: <names>" in the Quick
  take block, and include failed_services/total_services in --json.

run_full_scan records a service twice when both its Dockerfile and its
image scan fail, so the names are de-duplicated before counting.

Fixes OWASP#131. Builds on OWASP#161.
@myukitty
myukitty force-pushed the fix/surface-compose-scan-failures branch from 04f8171 to e596d9b Compare August 21, 2026 01:52
@myukitty

Copy link
Copy Markdown
Author

Thanks @advaitpatel — updated and rebased onto current main (77c0cd1); the branch is mergeable again.

Since #161 merged in the meantime, I rewrote the PR rather than just resolving the conflict. #161 already tracks failed_services in run_full_scan, so I dropped everything this PR duplicated and kept only the part of #131 that is still unfixed: nothing reads that field back out, so a compose run where some services could not be scanned still prints a Quick take and a score as if every service was covered.

What is left is 4 files, +141/-3:

  • total_services carried alongside failed_services so the summary has a denominator
  • one Quick take line: 2 of 3 services could not be scanned: web, db
  • failed_services / total_services in --json
  • de-duplication, because a service that fails both its Dockerfile and image scan is currently appended twice
  • 7 tests

No change to exit codes, scoring, or scan behaviour. Full details and before/after output are in the updated description.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cli Changes to the CLI entry point core Changes to core scanning logic tests Changes to the test suite

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[GOOD FIRST ISSUE] Surface per-service compose scan failures in user-facing output

2 participants