feat(baseline): add Best Practices Badge automation-proposal URL output format#319
feat(baseline): add Best Practices Badge automation-proposal URL output format#319Jaydeep869 wants to merge 5 commits into
Conversation
…ut format Closes darnitdevorg#283 Adds a new 'badge' output format to darnit that generates an OpenSSF Best Practices Badge automation-proposal URL from OSPS Baseline audit results. Maintainers can follow the URL to pre-fill their badge entry and earn a badge for meeting the OSPS Baseline criteria. ## What changed - New module: darnit_baseline/formatters/badge.py - control_id_to_key(): OSPS-AC-01.01 -> osps_ac_01_01 - status_to_badge_status(): PASS->Met | FAIL->Unmet | WARN->? | NA->N/A - generate_badge_url(): builds full automation-proposal URL with _status and _justification params per control; truncates long justifications at 500 chars; warns when project URL is missing - formatters/__init__.py: export the three new public symbols - tools.py (audit_openssf_baseline MCP tool): - Add output_format='badge' branch - Add optional project_url param (auto-detected from owner/repo if absent) - cli.py (darnit audit): - Add 'badge' to -o/--output choices - Add --project-url flag ## Tests 30 new unit tests covering: - All control ID transform cases - All 6 status-mapping cases (PASS/FAIL/WARN/NA/N/A/unknown) - URL structure (as=edit, url=, _status, _justification params) - Edge cases: empty results, no project URL, long justifications, missing id/status fields, empty/whitespace-only details - Package-level re-export sanity check Co-authored-by: david-a-wheeler <wheeler@dwheeler.com> Signed-off-by: jaydeep869 <jaydeeppokhariya2106@gmail.com>
Remove unused pytest import and move inline import block to module level to satisfy ruff I001 import sort order rules flagged in CI. Signed-off-by: jaydeep869 <jaydeeppokhariya2106@gmail.com>
|
Tested end-to-end @Jaydeep869 the formatter works, the git-remote auto-detect works, and the pure functions + tests are solid. Three changes needed before merge:
Thanks! |
Three changes based on review comments on PR darnitdevorg#319: Remove badge output from darnit core CLI. The darnit package must not depend on darnit-baseline, and badge output only makes sense for the openssf-baseline framework. The feature is available via the MCP tool audit_openssf_baseline(output_format="badge") which lives in the right package. Removes the -o badge choice, --project-url flag, and the badge branch from cmd_audit in cli.py. Drop justifications for inconclusive and N/A results. Only Met (PASS) and Unmet (FAIL) results include a justification in the URL. WARN and NA statuses map to ? / N/A with no justification, preventing error messages or command-not-found text from surfacing in a real badge submission. Cap total URL at 6 KB. All _status params are always included. Justifications are appended in result order until the running byte total exceeds the budget. This keeps the URL within common server and proxy limits even on large repos. Tests updated and extended to cover all three cases (34 tests, all passing). Signed-off-by: jaydeep869 <jaydeeppokhariya2106@gmail.com>
|
Hey @Marc-cn done, can you review it again. |
mlieberman85
left a comment
mlieberman85
left a comment
There was a problem hiding this comment.
Overall this looks decent — the badge formatter itself is clean, well-tested, and does what #283 asks for. One architectural issue to sort out plus a handful of smaller items.
Dependency direction.
packages/darnit/src/darnit/cli.py:207-211 imports from darnit_baseline.formatters.badge import generate_badge_url. That's the framework package (darnit) reaching into one of its plugin modules (darnit_baseline). Once that import is there, darnit no longer stands alone — it depends on the plugin it's supposed to host, and the "framework loads plugins" arrow gets a return arrow pointing the other way.
This is called out explicitly in docs/getting-started/framework-development.md under "Separation Rules" (around line 46 today): the framework must not import implementation packages, and cross-boundary communication should go through the ComplianceImplementation protocol or entry points. The badge output format and --project-url flag are baseline-specific concepts (OSPS control IDs, Best Practices Badge site), so they don't naturally belong in the generic framework CLI anyway.
Two ways to fix this that I'd be happy with:
- Add a badge-output primitive in core
darnitthat downstream modules call into. The framework exposes a generic "audit-results-to-URL-form" mechanism (or something similar with generic keys/values), and the baseline module supplies the OSPS-specific mapping and the Best Practices Badge base URL. Cleaner long-term shape and unblocks future badge-adjacent formats, but a bigger PR. - Keep the badge feature purely in
darnit-baselinefor now and abstract later. Drop thepackages/darnit/src/darnit/cli.pychanges; keep the badge logic and theoutput_format=\"badge\"branch in the baseline MCP tool (audit_openssf_baseline) where it already lives. Users invoke it via the MCP tool; CLI access can come back later once we have the right protocol. Smallest change, ships the feature, defers the abstraction.
I'd lean toward the second — it lands the value from #283 without committing to an abstraction shape we haven't designed yet. But I'm open to either.
ASCII-only in file content.
packages/darnit-baseline/src/darnit_baseline/formatters/badge.py uses `...` (horizontal ellipsis) for truncation and a warning emoji in the header. Please replace with plain ASCII (`...` and something like `WARNING:` respectively). Keeps rendering consistent across terminals and diffs clean.
URL length ceiling.
OSPS Baseline has ~62 controls. At a 500-char per-control justification cap plus keys plus URL-encoding overhead, a fully populated URL can easily land in the 30-50K character range. Practical browser URL limits are around 8K-32K depending on browser and server; the badge site's own tolerance isn't documented in #283. Two concrete asks:
- Reduce `_MAX_JUSTIFICATION_LEN` to something like 200. The badge form expects short human-readable justifications, not evidence dumps.
- Add a test asserting the generated URL length stays under a fixed ceiling (say 8000 chars) for a 62-control audit with typical justifications. Turns "URL might be too long" into a hard signal.
Test-file import cleanup.
`tests/darnit_baseline/formatters/test_badge_formatter.py:11-25` imports the same four symbols twice — once from `darnit_baseline.formatters` with `_`-prefixed aliases, once from `darnit_baseline.formatters.badge` with real names. The aliased copies are only used in the last test that verifies the re-export. Simpler: import once from `darnit_baseline.formatters` with real names and let every test use those references — if the re-export breaks, all tests break, which is the correct signal.
Assertion sloppiness in encoding tests.
`test_url_with_warn_result` and `test_url_with_na_result` accept either the URL-encoded (`%3F`, `%2F`) or unencoded form of the status value. Since `urlencode(..., quote_via=quote)` always percent-encodes reserved characters, please assert the specific encoded form — otherwise a regression that ships raw `?` or `/` in the URL query would slip past.
Nits (take or leave):
- `--project-url` accepts any string; a bad value silently produces a broken badge URL. A cheap check that it starts with `http://` or `https://` at argparse time would help.
- `re.sub(r"[-.]", "_", control_id).lower()` — two `str.replace()` calls would be equivalent and clearer for two literal characters. Minor.
Nice work overall. Once the dependency direction is sorted plus the ASCII fix and URL-length cap, this should be in good shape.
ASCII-only content. Replace the ellipsis character used for truncation with '...' and the warning emoji with 'WARNING:' in badge.py. Keeps terminal rendering and diffs clean across all environments. Reduce justification cap to 200 chars. Down from 500. The badge form expects short human-readable notes, not evidence dumps. Combined with the 6 KB URL budget this keeps URLs well within practical limits. Replace re.sub with str.replace. Two str.replace calls are equivalent and clearer for two literal characters; the re import is no longer needed. Test import consolidation. Tests now import once from darnit_baseline.formatters with real names. A re-export breakage will fail every test in the file, which is the correct signal. Specific percent-encoded assertions. test_warn_result_has_no_justification and test_na_result_has_no_justification now assert %3F and N%2FA respectively, since urlencode(quote_via=quote) always encodes reserved characters. A regression shipping raw '?' or '/' would no longer slip past. 62-control URL ceiling test. test_url_under_8000_chars_for_62_controls builds a realistic 62-control audit and asserts the URL stays under 8000 characters, turning 'URL might be too long' into a hard signal. Signed-off-by: jaydeep869 <jaydeeppokhariya2106@gmail.com>
|
Thanks for the follow-up commit — most of the review is addressed cleanly (cli.py no longer imports darnit_baseline, One remaining ASCII issue:
Once those are ASCII, this is good to merge. |
Replace three U+2014 em-dashes with plain ASCII double-hyphens (--) to keep rendering consistent across all terminals and diffs clean: - Line 50 comment: inconclusive -- cannot assert Met or Unmet - Line 181 comment: Budget exhausted -- skip remaining justifications - Line 193 header string: OpenSSF Best Practices Badge -- Automation Proposal Requested by mlieberman85 in final review round. Signed-off-by: jaydeep869 <jaydeeppokhariya2106@gmail.com>
|
hey @mlieberman85 @Marc-cn i have updated the pr as you told can you review it again. |
3 participants
Closes #283
This adds a
badgeoutput format todarnit auditand theaudit_openssf_baselineMCP tool. It generates a ready-to-follow OpenSSF Best Practices Badge automation-proposal URL pre-filled with the audit results.Usage
CLI
MCP tool
How it works
Control ID transform:
OSPS-AC-01.01becomesosps_ac_01_01(lowercase, hyphens and dots replaced with underscores).Status mapping follows Marc's proposal from the issue:
The
url=parameter is taken from--project-urlif given, otherwise auto-detected from the git remote ashttps://github.com/{owner}/{repo}. A warning is shown when it cannot be determined.Justifications (the
detailsfield) are included per control and capped at 500 characters to keep URLs practical.Changes
packages/darnit-baseline/src/darnit_baseline/formatters/badge.py— new module with three pure functions:control_id_to_key,status_to_badge_status,generate_badge_urlpackages/darnit-baseline/src/darnit_baseline/formatters/__init__.py— export the new symbolspackages/darnit-baseline/src/darnit_baseline/tools.py— addoutput_format="badge"branch and optionalproject_urlparameter toaudit_openssf_baselinepackages/darnit/src/darnit/cli.py— addbadgeto-o/--outputchoices and add--project-urlflagtests/darnit_baseline/formatters/test_badge_formatter.py— 30 unit tests covering all status mappings, key transforms, URL structure, and edge casesAll 30 new tests pass. No existing tests broken by this change.
Signed-off-by: jaydeep869 jaydeeppokhariya2106@gmail.com