Skip to content

fix(security): enforce discovery and analysis completeness - #410

Open
Spectorian wants to merge 11 commits into
mainfrom
codex/security-discovery-completeness
Open

fix(security): enforce discovery and analysis completeness#410
Spectorian wants to merge 11 commits into
mainfrom
codex/security-discovery-completeness

Conversation

@Spectorian

Copy link
Copy Markdown
Collaborator

Summary

  • include bounded dot-prefixed child skills while preserving explicit skip directories and link safeguards
  • keep requested and executed optional-analysis state distinct across reports and MCP output
  • require a complete requested analysis pass for an install-safe MCP verdict while preserving explicit static-only behavior

Validation

  • 276 focused and adversarial regressions
  • 2,806 non-integration/non-provider tests
  • cross-surface JSON and MCP accounting checks
  • Ruff lint, format check, targeted mypy, and git diff --check

@rng1995
rng1995 marked this pull request as ready for review August 24, 2026 18:15

@rng1995 rng1995 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Requesting changes because the requested semantic pass can still be skipped entirely while this PR reports it as used and complete, producing an install-safe verdict. The current-head end-to-end repro executes zero semantic calls but returns llm_used=True, scan_mode=static+llm, recommendation=SAFE, and safe_to_install=True.

Comment thread src/skillspector/mcp_server.py Outdated
if not enabled:
return False, False
call_log = result.get("llm_call_log")
if not isinstance(call_log, list) or not call_log:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] Do not count an empty call log as a completed LLM pass. If the graph is imported before provider credentials are available, credential-gated semantic nodes are omitted. Binding a supported provider later through use_provider makes preflight succeed, but no semantic nodes or calls run and llm_call_log remains empty. This branch then returns (used=True, complete=True), allowing a SAFE/safe_to_install=True result even though meta_analysis_applied=False. Treat an empty log as not used/incomplete, or retain the expected analyzers and account their explicit statuses before allowing an install approval.

Signed-off-by: Nir Paz <npaz@nvidia.com>
Signed-off-by: Nir Paz <npaz@nvidia.com>
Signed-off-by: Nir Paz <npaz@nvidia.com>
Signed-off-by: Nir Paz <npaz@nvidia.com>
Signed-off-by: Nir Paz <npaz@nvidia.com>
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.

2 participants