ci: harden GitHub Actions token and input handling - #2008
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe pull request hardens GitHub Actions checkout and token handling, passes release values through environment variables, removes tag documentation automation, and groups generated tag documentation by suite. ChangesWorkflow maintenance
Tag documentation
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 18 |
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
There was a problem hiding this comment.
Pull request overview
This PR hardens GitHub Actions token usage and shell input handling by reducing default GITHUB_TOKEN privileges, limiting credential persistence on checkouts, and deferring higher-privilege GitHub App token minting until just before PR creation. It also removes legacy, nonfunctional tag-documentation automation.
Changes:
- Reduce default workflow/job token privileges and disable persisted checkout credentials in read-only jobs.
- Mint GitHub App tokens only immediately before PR creation, with narrowly scoped
contents: write/pull-requests: write. - Avoid direct
${{ }}interpolation in shell commands by passing values via step-local environment variables; remove obsolete tag doc workflow/script and update action pins.
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| docs/proposals/repo-restructure.md | Removes obsolete proposal reference to the deleted tag-doc generator. |
| build/Update-TagsDocumentation.ps1 | Removes legacy tag documentation generator script. |
| .github/workflows/update-tag-documentation.yml | Removes legacy workflow that attempted to generate/commit tag docs. |
| .github/workflows/update-role-definitions.yaml | Uses read-only checkout token; mints App token only for PR creation; disables persisted creds. |
| .github/workflows/update-public-suffix-list.yaml | Reduces default job token to contents: read; mints App token only for PR creation; disables persisted creds. |
| .github/workflows/update-module-docs.yaml | Uses read-only checkout token; mints App token only for PR creation; disables persisted creds. |
| .github/workflows/build-docs.yaml | Uses read-only checkout token; mints App token only for PR creation; disables persisted creds. |
| .github/workflows/publish-versioned-docs.yml | Passes input version to Node via step env var (safer shell argument handling). |
| .github/workflows/publish-tests.yaml | Disables persisted checkout creds; passes release tag via env vars; updates cpina action pin. |
| .github/workflows/publish-module.yaml | Passes module version via env vars into PowerShell/Node steps. |
| .github/workflows/publish-module-preview.yaml | Passes module version via env vars into PowerShell/Node steps. |
| .github/workflows/publish-module-manualversionupdate.yaml | Passes module version via env vars into PowerShell/Node steps. |
| .github/workflows/opengrep.yml | Disables persisted checkout creds in scan job. |
| .github/workflows/maester-action-smoke-test.yaml | Aligns checkout pin to actions/checkout v7.0.1 SHA. |
| .github/workflows/dependency-review.yaml | Disables persisted checkout creds for dependency review job. |
| .github/workflows/codeql.yml | Disables persisted checkout creds (but currently introduces invalid YAML indentation in steps). |
| .github/workflows/build-website.yaml | Disables persisted checkout creds across website build jobs. |
| .github/workflows/build-maester-report-template.yaml | Disables persisted checkout creds in report-template build job. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
I love the intent and the goal of this PR! The one thing I want to check: the newer test auto-documentation workflow doesn't completely replace what the tag documentation workflow does (as far as I have seen). Let's see if we can find a way to get feature parity before the tag documentation is removed. As an aside, I think the last website reorganization resulted in some valuable pages being lost from the navigation structure and I'd like to see some of those recovered as well. |
I was under the impression that I could be wrong on both points, but that's how it seemed to me. It would be good if someone else could verify or disprove :) |
I didn't think that workflow was even a year old? |
You're right - it was introduced in commit d2ddee1 in December 2025. What I meant was that based on the run history since it was implemented, the workflow doesn't appear to have produced a commit. I phrased that poorly. |
|
I removed the cpina gh action pin change in this commit since I'm removing the third party dependency completely and using the Maester Bot to perform the task. See #2068 |
|
@SamErde The tag-documentation feature-parity concern is now addressed in 5284e66. We kept one source of truth: The obsolete workflow and PowerShell script remain removed, so two generators cannot drift or overwrite the same page. Validation:
The broader navigation-recovery aside is not changed by this focused fix and can be handled separately. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@website/scripts/generate-test-docs.mjs`:
- Around line 514-519: Update the CIS matcher in tagGroups so its L1 and L2
alternatives are anchored to match only standalone tags, preventing values such
as CISA.L1, ORCA.L2, or embedded substrings from being classified as CIS. Add
regression cases covering these overlapping-prefix tags and verify they remain
in their correct sections.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 067499ed-173a-400d-a8ef-8226b1eb60e3
📒 Files selected for processing (2)
website/docs/tests/tags/readme.mdwebsite/scripts/generate-test-docs.mjs
There was a problem hiding this comment.
🧹 Nitpick comments (1)
website/scripts/tag-groups.test.mjs (1)
5-20: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd regression cases for the
MS.andMaesterbranches.The test covers the
CISAprefix but not theMS.alternative. It does not cover theMaestergroup at all. Add representative cases so these mappings remain protected.Suggested cases
["CISA.L1", "CISA"], + ["MS.Azure.Baseline", "CISA"], ["EIDSCA.L1", "EIDSCA"], ["ORCA.L2", "ORCA"], + ["MT.1001", "Maester"], + ["Maester", "Maester"],🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@website/scripts/tag-groups.test.mjs` around lines 5 - 20, Add regression cases to the cases array in the standalone CIS-level classification test for representative tags using the MS. branch and the Maester group, asserting each maps to its expected group through tagGroupFor. Keep the existing overlap and ungrouped cases unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@website/scripts/tag-groups.test.mjs`:
- Around line 5-20: Add regression cases to the cases array in the standalone
CIS-level classification test for representative tags using the MS. branch and
the Maester group, asserting each maps to its expected group through
tagGroupFor. Keep the existing overlap and ungrouped cases unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 0bdfd14f-7c50-46d1-aaa1-cce591f2ae2b
📒 Files selected for processing (3)
website/scripts/generate-test-docs.mjswebsite/scripts/tag-groups.mjswebsite/scripts/tag-groups.test.mjs
🚧 Files skipped from review as they are similar to previous changes (1)
- website/scripts/generate-test-docs.mjs
📑 Description
Hardens GitHub Actions token and input handling while preserving active automation behavior:
GITHUB_TOKENinupdate-public-suffix-listtocontents: read.contents: writeandpull-requests: write.update-tag-documentationworkflow and its sole-purposebuild/Update-TagsDocumentation.ps1generator. GitHub records 116 runs from January 19 through July 25, including 73 on non-main branches. Those runs performed checkout withcontents: write, installed Pester, discovered tests, and wrotewebsite/docs/tests/tags/readme.md; however, after the generator moved to that path, the commit step continued checkingwebsite/docs/tags.md, loggedNo changes to commit, and discarded the generated change. Repository history contains no matchingchore: update tags documentationbot commit. Since May,website/scripts/generate-test-docs.mjshas owned the current linked tag index and is enforced by the website checks and prebuild. The current generator now also preserves the legacy tag-usage guidance and groups the inventory into CIS, CISA, EIDSCA, ORCA, Maester, and Ungrouped sections. Removing the legacy workflow and script therefore eliminates nonfunctional automation without removing its useful documentation behavior.The persisted checkout credentials in publish jobs remain an accepted residual because
stefanzweifel/git-auto-commit-actionrequires them for its later push. Existing active triggers, jobs, matrices, artifacts, branches, tags, inputs, and release fallback behavior remain unchanged. The regenerated tag index is the only intended generated-output change.Supporting guidance:
actions/create-github-app-tokenpermissionspeter-evans/create-pull-requesttoken guidanceactions/checkoutcredential persistencestefanzweifel/git-auto-commit-actioncheckout requirement✅ Checks
/powershell/tests/pester.ps1locally.Local validation completed:
Update-TagsDocumentationreferences remain.npm --prefix website run generate-test-docsgenerated 408 test-documentation pages, includingwebsite/docs/tests/tags/readme.md, andnpm --prefix website run buildcompleted successfully. A subsequent generated-doc comparison reports only the unrelated, time-sensitive contributor snapshot as stale; the tag page is current.git diff --check upstream/mainpassed.Common.Tests.ps1for more than 15 minutes. The local run was stopped and is not reported as passing; the required Ubuntu, Windows, and macOSbuild-validationjobs are the authoritative full-suite result.Earlier PR checks for commit
e951cbc3(before the tag-index parity update):npm run check-test-docsand failed only because the pre-existing generated contributor snapshotwebsite/src/data/contributors.jsonis stale.build-validationjobs each ran 10,224 tests and reported the same nine failures: the Global Secure Access functions added in Add 9 Global Secure Access checks (MT.1187–MT.1195) [Preview] #1992 do not satisfy the repository-wideWrite-Verbosepolicy test. This PR does not change those functions, and an unrelated PR run immediately before this one failed on the same checks, so no unrelated GSA implementation was added to this focused workflow-hardening change.ℹ️ Additional Information
No repository variables, secrets, environments, generated command documentation, module output, or runtime dependencies are added or changed. The generated tag index is the only generated test documentation changed. No prior issue is required for this focused maintenance change.
Summary by CodeRabbit
Security
Maintenance
Documentation
Removed