Skip to content

docs(security): reconcile branch-protection pages with the main-branch - #2759

Open
Jamie Kim (jkim323) wants to merge 3 commits into
mainfrom
docs/2755-branch-protection-ruleset-drift
Open

docs(security): reconcile branch-protection pages with the main-branch#2759
Jamie Kim (jkim323) wants to merge 3 commits into
mainfrom
docs/2755-branch-protection-ruleset-drift

Conversation

@jkim323

@jkim323 Jamie Kim (jkim323) commented Aug 23, 2026

Copy link
Copy Markdown
Collaborator

Pull Request

Description

Both branch-protection documentation pages described classic branch protection, which no longer governs main. They now describe the repository ruleset main-branch-protection (id 9453100, source_type: Repository, enforcement: active), verified against the live GitHub API.

The most consequential correction is a security claim. Both pages stated that stale approvals are dismissed when new commits are pushed. They are not: dismiss_stale_reviews_on_push is false. That control was assessed and accepted as a gap in #2461, so the pages were publicly contradicting the triage record. Both pages now document the gap explicitly and name require_last_push_approval as the compensating control.

The pages were also wrong in the opposite direction, understating the repository's posture:

Claim Was Now
Required reviewers 1 2
Required status checks 8 listed, including two that are not required The 6 live context strings
OpenSSF Scorecard Branch-Protection "~8/10", remediated by adding a second reviewer 9, with stale-review dismissal as the sole remaining deduction
Administration path Settings, Branches, Branch protection rules Settings, Rules, Rulesets, main-branch-protection
Rulesets migration Proposed as a future consideration Already complete

Validate Dependency Pinning and npm Security Audit were listed as required and are not. They are now named as non-required alongside Markdown Link Check.

A third page carried the same claim from the other direction. docs/security/dependency-pinning.md described the branch-protection page as covering "required status checks including dependency pinning", which this PR directly contradicts. That line now points at the branch-protection page without restating any gate status, so the branch-protection page stays the single place that answers what gates a merge. The wording was chosen so it remains accurate whichever way #2760 resolves.

Five controls had no documentation at all and are now covered: required_review_thread_resolution, require_extra_approval_for_unattributed_changes, squash-only merges, non_fast_forward and deletion protection, and the code_quality rule at errors severity.

Each branch-protection page now carries the gh api commands that reproduce every documented value, so a future reader can re-verify the pages instead of trusting them.

No repository setting, workflow, ruleset, or runtime behavior is changed by this PR.

Sourcing note

The require_extra_approval_for_unattributed_changes description is cited to GitHub's available rules for rulesets. The rule is narrower than its parameter name suggests: it applies to pull requests Copilot opens under its own app identity, not to unattributed authorship generally. Both pages state that GitHub enables it by default and lists it as public preview, so its presence is not presented as a deliberate configuration choice.

Related Issue(s)

Fixes #2755

Related: #2461 (the accepted stale-review-dismissal gap that these pages contradicted)

Related: #2760 (open question on whether PR Validation Success should be a required status check; raised from this work, and the reason the dependency-pinning cross-reference avoids asserting gate status)

Type of Change

Select all that apply:

Code & Documentation:

  • Bug fix (non-breaking change fixing an issue)
  • New feature (non-breaking change adding functionality)
  • Breaking change (fix or feature causing existing functionality to change)
  • Documentation update

Infrastructure & Configuration:

  • GitHub Actions workflow
  • Linting configuration (markdown, PowerShell, etc.)
  • Security configuration
  • DevContainer configuration
  • Dependency update

AI Artifacts:

  • Reviewed contribution with hve-builder and addressed all actionable findings
  • Copilot instructions (.github/instructions/*.instructions.md)
  • Copilot prompt (.github/prompts/*.prompt.md)
  • Copilot agent (.github/agents/*.agent.md)
  • Copilot skill (.github/skills/*/SKILL.md)
  • Copilot hook (.github/hooks/*/*.json)
  • Eval spec added/updated for changed AI artifacts (evals/)

Other:

  • Script/automation (.ps1, .sh, .py)
  • Other (please describe):

Sample Prompts (for AI Artifact Contributions)

Testing

Documentation-only change. No manual testing was performed beyond reading the rendered pages.

Source verification

Every documented value was read back from the live API rather than taken from the previous page text or the issue body:

  • gh api repos/microsoft/hve-core/rules/branches/main for all five active rules and every pull_request parameter
  • gh api repos/microsoft/hve-core/rulesets/9453100 for ruleset identity, enforcement, bypass actors, and conditions
  • gh api repos/microsoft/hve-core/branches/main to confirm classic protection is inert
  • GitHub's available-rules documentation for the one behavioral description the API does not expose

Automated validation

Command Scope Result
npx markdownlint-cli2 All three changed pages Pass, 0 issues
npm run lint:tables Repository Pass
pwsh ./scripts/linting/Validate-MarkdownFrontmatter.ps1 -WarningsAsErrors -EnableSchemaValidation Repository, 1144 files Pass, 0 errors
npx cspell All three changed pages Pass, 0 issues
pwsh ./scripts/linting/Markdown-Link-Check.ps1 -Path <file> All three changed pages Pass, all links resolved

Two findings were surfaced and fixed during validation: lint:tables required a format:tables pass after the tables were rewritten, and markdownlint reported MD013 line-length violations on two new paragraphs, which were wrapped. The dependency-pinning page passed all five checks on the first run.

Checklist

Required Checks

  • Documentation is updated (if applicable)
  • Files follow existing naming conventions
  • Changes are backwards compatible (if applicable)
  • Tests added for new functionality (if applicable) (N/A: documentation-only change with no test files)

AI Artifact Contributions

  • Used hve-builder review mode to review contribution
  • Addressed all actionable findings from the hve-builder review
  • Verified contribution follows common standards and type-specific requirements

Required Local Checks

The following local-safe validation commands must pass before merging:

  • Local validation aggregate: npm run validate:local (not yet run)
  • Documentation validation (if docs changed): npm run validate:docs (not yet run; this aggregate covers Docusaurus lint, labels, typecheck, and component tests, none of which exercise the Markdown checks that gate these files)
  • Spell checking: npm run spell-check (equivalent npx cspell run passed scoped to all three changed files; the repository-wide script has not been run)
  • Link validation: npm run lint:md-links (equivalent per-file script run passed for all three changed files; the repository-wide script has not been run)

Security Considerations

  • This PR does not contain any sensitive or NDA information
  • Any new dependencies have been reviewed for security issues (N/A: no dependency changes)
  • Security-related scripts follow the principle of least privilege (N/A: no security scripts modified)

All configuration values reproduced in these pages are already public: they are readable by anyone with repository access through the documented gh api commands, and the same values were published in the #2461 triage comment.

This PR documents a security control that is not enforced. That is deliberate. The alternative, silently deleting the incorrect claim, would leave readers with the same false impression the pages already created, and would not match the accepted-risk record in #2461.

Additional Notes

Deliberately out of scope

  • Several Effect and Purpose descriptions on the contributing page summarize rule behavior without citation. Only the rule whose scope is easy to misread is cited here. A broader sourcing pass is tracked separately.

Maintenance triggers

  • If the ruleset is changed to enable dismiss_stale_reviews_on_push, both pages and [Security][high] Branch-Protection #2461 need a follow-up update.
  • If [CI] Clarify whether PR Validation Success should be a required status check #2760 resolves by adding PR Validation Success to the required set, the required-status-check section of the contributing page needs updating. The dependency-pinning cross-reference was deliberately worded to need no change in that case.
  • The Copilot approval rule is in public preview. If GitHub renames the section or moves the feature out of preview, the cited anchor and the surrounding wording need re-checking. A link check can only prove the URL resolves, not that the content still says what these pages claim.

Heading changes

The contributing page's "Configuration Reference" subsections were replaced and "Future Considerations" became "Change Management". No internal link in the repository targets an anchor on either page, so nothing breaks, but external deep links to those anchors would not survive.

…h-protection ruleset

- describe the ruleset as the governing mechanism and drop classic UI steps
- document stale-review dismissal as unenforced with its compensating control
- correct review counts, required checks, and the Scorecard score to 9
- add the previously undocumented merge, history, and quality controls

🔐 - Generated by Copilot
@jkim323 Jamie Kim (jkim323) changed the title docs(security): reconcile branch-protection pages with the main-branc… docs(security): reconcile branch-protection pages with the main-branch Aug 23, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Eval Execution

⚠️ No eval summary was produced.

@jkim323 Jamie Kim (jkim323) self-assigned this Aug 23, 2026
@codecov-commenter

Codecov Comments Bot (codecov-commenter) commented Aug 23, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.96%. Comparing base (5d81d09) to head (142ca1b).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #2759      +/-   ##
==========================================
- Coverage   83.16%   82.96%   -0.20%     
==========================================
  Files         180      181       +1     
  Lines       32168    32698     +530     
  Branches       25       25              
==========================================
+ Hits        26751    27127     +376     
- Misses       5414     5568     +154     
  Partials        3        3              
Flag Coverage Δ
docusaurus 89.92% <ø> (ø)
pester 83.45% <ø> (-0.73%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.
see 10 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@jkim323
Jamie Kim (jkim323) marked this pull request as ready for review August 24, 2026 02:08
@jkim323
Jamie Kim (jkim323) requested a review from a team as a code owner August 24, 2026 02:08
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.

[Docs] Branch protection docs contradict the live main-branch-protection ruleset

2 participants