Skip to content

Security insights#2760

Open
david-a-wheeler wants to merge 11 commits intomainfrom
security-insights
Open

Security insights#2760
david-a-wheeler wants to merge 11 commits intomainfrom
security-insights

Conversation

@david-a-wheeler
Copy link
Copy Markdown
Collaborator

No description provided.

david-a-wheeler and others added 8 commits April 2, 2026 16:20
…s.yml

Adds a new detective that looks for a project's security-insights.yml
(OSSF Security Insights spec) in the repo root or .github/ directory and
infers badge criteria answers from its fields.

New files:
- app/lib/security_insights_detective.rb: detective (INPUTS: repo_files;
  30 distinct output criteria, both metal and baseline)
- criteria/security_insights_map.yml: 39 YAML-driven mappings covering
  vulnerability reporting, documentation URLs, repo status, security
  tools (SAST/SCA/fuzzing/secret), assessments, and release attestations
- script/update_security_insights_comments.rb: standalone script that
  refreshes # Target: criterion-text comments in the mapping YAML from
  config/locales/en.yml (safe to re-run at any time)
- test/unit/lib/security_insights_detective_test.rb: 36 tests covering
  all condition types, edge cases, and security properties

Security: untrusted YAML is loaded with safe_load (permitted_classes: []),
aliases: false (prevents alias/anchor bombs), and a 50 KB size cap.
Confidence levels are 1-3 (never override user input).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Signed-off-by: David A. Wheeler <dwheeler@dwheeler.com>
Fix a correctness bug and adjust two confidences after careful review:

Bug fix: repository.security.assessments.self is *required* in the SI spec,
so checking "present" would always fire for any valid SI file.  Changed both
affected entries to repository.security.assessments.self.evidence (the
optional URL) which actually signals a self-assessment was performed.

Confidence corrections:
- reports-accepted=true → osps_vm_03_01 reduced from 2 to 1: accepting
  reports doesn't confirm a *private* channel exists (could be a public
  issue tracker), which is what osps_vm_03_01 specifically requires.

Commented-out mapping:
- security-policy → osps_do_02_01 removed: a security/CVD policy document
  is distinct from a general defect-reporting guide (osps_do_02_01), so
  the mapping was misleading.  Explanation comment left in place.

Add clarifying notes near several other entries to explain asymmetries and
edge cases for future reviewers.  Add two new tests covering the self.evidence
fix and the confidence-1 osps_vm_03_01 case.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Signed-off-by: David A. Wheeler <dwheeler@dwheeler.com>
When a security-insights.yml field includes a comment, append
'Comment says: "...".' to the generated justification text so
reviewers can see the project's own documentation verbatim.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Signed-off-by: David A. Wheeler <dwheeler@dwheeler.com>
Add five new SI-field-to-criterion mappings that were not previously covered:
- repository.documentation.code-of-conduct → code_of_conduct (Met, conf 2)
- repository.security.tools[type=DAST]    → dynamic_analysis (Met, conf 2)
- repository.security.tools[type=pentest] → security_review   (Met, conf 2)
- repository.security.tools[type=pentest] → osps_sa_03_01     (Met, conf 2)
- project.security.security-contacts      → osps_vm_02_01     (Met, conf 2)

Target: blocks populated by script/update_security_insights_comments.rb.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Signed-off-by: David A. Wheeler <dwheeler@dwheeler.com>
- Add MAX_SI_COMMENT_SIZE (500 chars) truncation for untrusted SI comment
  strings before they are included in justification text; prevents an
  adversarial file from injecting an arbitrarily long string (bounded only
  by the 50 KB file cap) into the stored criterion justification.
- Expand SECURITY: header to document early-stop DoS protection, no-regex
  guarantee, and comment truncation.
- Extract raw_si_comment and truncate_si_comment helpers for readability.
- Add test verifying that oversized comments are truncated.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Signed-off-by: David A. Wheeler <dwheeler@dwheeler.com>
… skip

- Raise MAX_SI_COMMENT_SIZE 500 → 2048 to allow substantive comments.
- Expand SECURITY: header in detective with per-threat-category rationale
  (DoS/early-stop, YAML injection, ReDoS, type confusion, comment injection,
  SQL/XSS) so reviewers can verify each threat is addressed.
- Skip mappings with confidence: 0, allowing the map file to document
  understood situations we deliberately do not act on (clearer than just
  commenting out the entry).
- Document confidence: 0 convention in map file header.
- Mark reports-accepted → vulnerability_report_response as confidence: 0
  with rationale (boolean flag cannot imply a ≤14-day SLA).
- Add test verifying confidence-0 entries produce no proposals.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Signed-off-by: David A. Wheeler <dwheeler@dwheeler.com>
Confidence adjustments (all with rationale comments):
- reports-accepted=true → osps_vm_03_01: 1→0 (says nothing about privacy)
- review-policy → osps_qa_07_01: 1→0 (policy URL ≠ VCS enforcement)
- SAST-in-CI → static_analysis_fixed: 1→0 (tools can be configured to ignore)
- SCA (not in CI) → osps_vm_05_03: 1→0 (criterion requires CI)
- SCA-in-CI → osps_vm_05_03: stays 2, rationale comment added
- secret tool → no_leaked_credentials: 1→2 (active scanning is strong signal)
- third-party assessment → security_review: 3→2 (5-year window uncertain)
- self.evidence → security_review: 2→1 (consistent; self < third-party)

Also add test for unknown si_condition returning false (fail-safe else
branch, previously an untested production code statement).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Signed-off-by: David A. Wheeler <dwheeler@dwheeler.com>
The new detective checks each test project repo for security-insights.yml
in the standard candidate paths; all return 404 (none of the fixture repos
have an SI file).  VCR recorded these interactions during the integration
test run on 2026-04-02.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Signed-off-by: David A. Wheeler <dwheeler@dwheeler.com>
david-a-wheeler and others added 3 commits April 2, 2026 23:35
Signed-off-by: David A. Wheeler <dwheeler@dwheeler.com>
… conditions

- Add Security Insights v1.0.0 mappings (project-lifecycle.stage,
  vulnerability-reporting.accepts-vulnerability-reports, security-contacts)
  with symmetric Met+Unmet rules and [v1.0.0] markers throughout
  This doesn't do a *lot*. Version 1.0.0 doesn't have a lot of the
  information we need, and old files may be out of date.
  Still, getting a little data when we can may be helpful.
- Rename si_condition "true"/"false" to "is_true"/"is_false" to avoid
  confusion with YAML boolean literals. The quoted strings were a footgun
- Add KNOWN_CONDITIONS constant; validate all MAPPINGS entries against it
  at load time. This way, a typo in the YAML raises an exception immediately
  at boot rather than silently producing zero proposals
- condition_met? now raises ArgumentError for unknown conditions instead of
  returning false; update test to assert_raises

Co-Authored-by: David A. Wheeler <dwheeler@dwheeler.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Signed-off-by: David A. Wheeler <dwheeler@dwheeler.com>
Signed-off-by: David A. Wheeler <dwheeler@dwheeler.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