Skip to content

fix(config): stop dropping sieve gating metadata on the effective config path#317

Merged
Marc-cn merged 2 commits into
kusari-oss:mainfrom
Jaydeep869:OSPS-QA-02.01-false-negative
Jul 9, 2026
Merged

fix(config): stop dropping sieve gating metadata on the effective config path#317
Marc-cn merged 2 commits into
kusari-oss:mainfrom
Jaydeep869:OSPS-QA-02.01-false-negative

Conversation

@Jaydeep869

@Jaydeep869 Jaydeep869 commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes a framework-level bug where the effective-config path (merge_controlcontrol_from_effective) was silently dropping when, depends_on, inferred_from, and on_pass from all controls. This meant every when gate and inferred_from auto-pass was dead in production audits — 0 of 62 baseline controls carried gating metadata despite 53 uses in the TOML.

The parallel control_from_framework path already carried these fields correctly, which is why unit tests stayed green.

Note: This PR does not fix OSPS-QA-02.01 false negative (#308). That is tracked separately and will be addressed in a follow-up PR, now that the underlying machinery works correctly.

Root Cause

merge_control in config/merger.py constructed EffectiveControl without copying the four sieve gating fields from FrameworkControl. Since ControlConfig is a Pydantic model that always defines these fields, the hasattr/getattr guards added later were also dead code.

Changes

  • packages/darnit/src/darnit/config/merger.py: Pass when, depends_on, inferred_from, and on_pass from FrameworkControl into EffectiveControl in merge_control. Remove dead hasattr/getattr guards — plain attribute access is correct since ControlConfig always defines these fields.

  • tests/darnit/config/test_merger.py: Add TestSieveGatingMetadataPreservation regression test class that:

    • Builds a FrameworkControl with all four gating fields set
    • Runs it through the full pipeline: merge_controlcontrol_from_effectiveControlSpec
    • Asserts all four fields survive into both EffectiveControl and ControlSpec.metadata
    • Also verifies controls without optional gating fields are unaffected

Type of Change

  • Bug fix (non-breaking change fixing an issue)

Testing

  • Tests pass locally (uv run pytest tests/ -v)
  • Added regression tests for the fixed behaviour
  • Linting passes (uv run ruff check .)

…l loader

Fixes an issue where sieve controls with `when`, `depends_on`, `inferred_from`, and `on_pass` metadata attributes were stripped during config merging. This correctly honors skipping applicability for OSPS-QA-02.01.

Signed-off-by: jaydeep869 <jaydeeppokhariya2106@gmail.com>
@Jaydeep869 Jaydeep869 requested a review from mlieberman85 as a code owner July 7, 2026 14:50
@Marc-cn

Marc-cn commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator

Thanks @Jaydeep869 I verified on main that the effective-config path (used by both the MCP audit tool and darnit audit) strips when/depends_on/inferred_from/on_pass from ALL controls: 0 of 62 baseline controls carry gating metadata despite 53 uses in the TOML, so every when gate and inferred_from auto-pass is currently dead in production audits. With your patch, 28 controls get their gating back. The parallel control_from_framework path already carried these fields, which is why unit tests stayed green. Good find

Two changes needed before merge:

  1. Retitle and rescope: this PR fixes the framework, not OSPS-QA-02.01 false negative #308. The diff contains no changes to discovery, no when clause on OSPS-QA-02.01, and no has_package_manager context key (the string appears nowhere in the repo), so QA-02.01 behaves identically after this merge. I suspect you tested with local TOML/context changes that didn't make it into the commit. Please retitle to something like "fix(config): stop dropping sieve gating metadata on the effective config path" and reword the description; leave OSPS-QA-02.01 false negative #308 open. The good news is the follow-up is small, because the machinery already exists:

    • OSPS-QA-02.01 false negative #308 case 1 (cmd/go.mod): file_exists already supports opt-in depth-limited search since fix(sieve): depth-limited file discovery via opt-in max_depth (closes #221) #259. Adding max_depth = 2 to the QA-02.01 file_exists pass finds cmd/go.mod (I verified). One caveat: vendor is not in _FILE_DISCOVERY_PRUNE_DIRS, so a Go repo with only vendored dependency manifests would false-positive; add vendor to the prune set in the same change.
    • OSPS-QA-02.01 false negative #308 case 2 (no package manager): needs a when clause on QA-02.01 plus a has_package_manager detection in context/auto_detect.py (see detect_has_subprojects for the pattern). Your framework fix here is exactly what makes that possible. You can haveit as your second PR or we can file it as a follow-up issue.
  2. Add a regression test. Build a FrameworkControl with when, depends_on, inferred_from, and on_pass, run it through merge_control and control_from_effective, and assert all four land in ControlSpec.metadata. That test would have caught this regression when it was introduced

Minors:

  • The hasattr(framework_control, ...)/getattr guards are dead code since FrameworkControl always defines these fields; plain attribute access is clearer.
  • Related gap I noticed while testing: _auto_derive_on_pass only runs on the framework-loader path, so auto-derived on_pass (project.yaml context capture) is still dropped on the effective path even with this patch, since openssf-baseline has no explicit on_pass

… regression tests

Remove the hasattr/getattr guards in merge_control — ControlConfig is a
Pydantic model that always declares when, depends_on, inferred_from, and
on_pass as fields, so the guards were dead code. Plain attribute access is
cleaner and more idiomatic.

Add TestSieveGatingMetadataPreservation regression test class that:
- Builds a FrameworkControl with all four gating metadata fields set
- Runs it through the full effective-config pipeline:
  merge_control → control_from_effective → ControlSpec
- Asserts all four fields (when, depends_on, inferred_from, on_pass)
  survive into both EffectiveControl and ControlSpec.metadata
- Also verifies controls without optional gating fields are unaffected

This test would have caught the metadata-stripping regression when it
was originally introduced.

Signed-off-by: jaydeep869 <jaydeeppokhariya2106@gmail.com>
@Jaydeep869

Copy link
Copy Markdown
Contributor Author

Hey @Marc-cn I have pushed the changes you asked can you reveiw the pr again.

@Jaydeep869 Jaydeep869 changed the title fix(sieve): pass when and dependency metadata through EffectiveControl loader fix(config): stop dropping sieve gating metadata on the effective config path Jul 9, 2026
@Marc-cn

Marc-cn commented Jul 9, 2026

Copy link
Copy Markdown
Collaborator

Thanks @Jaydeep869 , all addressed: the regression tests cover both layers with exact-value asserts including the negative case, the dead guards are gone, and the description now matches the diff. Re-verified against current main

@Marc-cn Marc-cn merged commit f446735 into kusari-oss:main Jul 9, 2026
8 checks passed
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