feat(core): zero-config GitHub Actions setup for Maester apps (#1503)#1777
feat(core): zero-config GitHub Actions setup for Maester apps (#1503)#1777kayasax wants to merge 6 commits into
Conversation
Adds -GitHubActions and -SetGitHubSecrets switches to New-MtMaesterApp and Add-MtMaesterAppFederatedCredential so the full GitHub Actions / OIDC setup can be done in one command from a maester-tests clone. * New internal helper Get-MtGitHubRepoFromGit parses git remote (HTTPS/SSH/ssh://) and auto-populates -GitHubOrganization / -GitHubRepository when omitted. * New internal helper Set-MtGitHubActionsSecret pushes AZURE_CLIENT_ID / AZURE_TENANT_ID via gh CLI, with a graceful fallback to the existing manual instructions when gh is missing or unauthenticated. * Backward compatible: existing -GitHubOrganization/-GitHubRepository behaviour is unchanged; manual instruction output is preserved when -SetGitHubSecrets is not used. * Pester tests for the URL parser (HTTPS, SSH, ssh://, .git suffix, non-GitHub, missing remote). * Docs: short PowerShell shortcut tip in website/docs/monitoring/github.md. Refs maester365#1503
Up to standards ✅🟢 Issues
|
PSScriptAnalyzer rule PSUseBOMForUnicodeEncodedFile (enforced by powershell/tests/general/PSScriptAnalyzer.Tests.ps1) requires .ps1 files containing non-ASCII characters to be saved with a BOM. The file uses the green-check character in success messages. Verified locally: PSScriptAnalyzer.Tests.ps1 -> 630/630 passing.
There was a problem hiding this comment.
Pull request overview
Adds zero-config GitHub Actions/OIDC setup to Maester app provisioning by auto-detecting the GitHub repository, creating federated credentials, and optionally setting GitHub Actions secrets through gh.
Changes:
- Adds GitHub repo auto-detection from local
git remote. - Extends
New-MtMaesterAppandAdd-MtMaesterAppFederatedCredentialwith GitHub Actions/secrets automation. - Adds tests for repository auto-detection and updates monitoring documentation with a shortcut.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
website/docs/monitoring/github.md |
Documents the new one-command PowerShell shortcut. |
powershell/tests/functions/Get-MtGitHubRepoFromGit.Tests.ps1 |
Adds Pester coverage for GitHub remote URL parsing. |
powershell/public/core/New-MtMaesterApp.ps1 |
Adds GitHub Actions setup switches and forwards federated credential creation/secrets setup. |
powershell/public/core/Add-MtMaesterAppFederatedCredential.ps1 |
Makes repo parameters optional via auto-detection and adds GitHub secrets setup. |
powershell/internal/Set-MtGitHubActionsSecret.ps1 |
Adds helper for setting GitHub Actions secrets via GitHub CLI. |
powershell/internal/Get-MtGitHubRepoFromGit.ps1 |
Adds helper for parsing GitHub repository details from git remote. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
SamErde
left a comment
There was a problem hiding this comment.
Hi @kayasax! 👋 Thanks so much for working through the automation feedback from #1503. The direction in this PR is spot-on, and I wanted to share some thoughts to help move it toward merge.
Architecture ✅
Your approach of extending New-MtMaesterApp and Add-MtMaesterAppFederatedCredential is exactly right. This keeps the feature integrated with the existing app/federated-credential lifecycle rather than introducing a parallel standalone script. That means:
- Users have one supported entry point for zero-config setup
- Manual, semi-automated, and full-automation flows all use the same trusted cmdlets
- We avoid duplicating app creation, permission grant, and documentation surfaces
Smart pattern: You've also extracted GitHub-specific logic into internal helpers (Get-MtGitHubRepoFromGit, Set-MtGitHubActionsSecret), which sets you up well for reuse. As Connect-Maester and Test-MtConnection expand GitHub support, this same internal-helper pattern will let you avoid duplicating auth logic across cmdlets. Consider that when you're designing those helpers—even if not needed immediately, designing them with composability in mind (e.g., a future Get-MtGitHubCredential helper) will keep the codebase clean and maintainable.
Really well done aligning with the existing design patterns.
Implementation Notes
I'd love to help refine a few areas before merge to make this rock-solid. Here are the merge-blocking items:
1. Secret handling security (high priority)
Set-MtGitHubActionsSecret currently passes secrets via --body flag:
& gh secret set $name --repo $GitHubRepository --body $valueThis puts the secret in the process command line, where it's visible to process inspection, audit logs, and shell history.
The gh CLI supports reading from stdin when --body is omitted. Use this instead:
$value | & gh secret set $name --repo $GitHubRepositoryThe gh documentation confirms: "reads from standard input if not specified". This keeps the secret off the command line and out of logs.
2. GitHub host parsing (security)
The regex in Get-MtGitHubRepoFromGit uses [^/]*github\.com, which would match hosts like notgithub.com or my-github.com. Should be:
^(?:https?://(?:www\.)?github\.com/|git@github\.com:|ssh://git@github\.com/)...Also worth adding tests for hostile/lookalike hosts (e.g., https://attacker.github.com.evil.com/org/repo).
3. Duplicate credential idempotency
The fallback message says "re-run with -SetGitHubSecrets", but if the credential already exists, the cmdlet hits the duplicate-credential check and returns without setting secrets. Either:
- Set secrets when a duplicate credential already matches (best UX), or
- Add a secrets-only path, or
- Update the guidance to clarify the behavior
4. Parameter behavior consistency
-SetGitHubSecrets implicitly enables the GitHub flow even without -GitHubActions, but the help text says they work together. Pick one:
- Allow
-SetGitHubSecretsalone to enable GitHub setup, or - Require
-GitHubActionsexplicitly and update help
Either way, document the choice clearly and add tests for each path.
5. Partial override behavior
Mixing explicit user input with auto-detected values can be surprising. If someone provides only -GitHubRepository but not -GitHubOrganization, should we auto-detect the org? Currently the code allows it, but it's ambiguous.
- Either require both explicit together, or
- Document and test all partial-override combinations
6. Generated command reference docs
The PR doesn't regenerate the .mdx files for these cmdlets, so the new parameters (-GitHubActions, -SetGitHubSecrets) and new examples won't appear in the docs. Run the doc generator and commit the updates.
7. Stale parameter in existing help
Add-MtMaesterAppFederatedCredential documents a -Force parameter that doesn't exist in the code. While that's pre-existing, since you're in this surface, worth fixing it (just remove from help or implement it, whichever makes sense).
Next Steps
I'm happy to pair on any of these! If you'd like, I can help patch these areas directly. Otherwise, let me know which ones you'd prefer to tackle, and I can guide from here.
The vision is solid—just a few safety and consistency tweaks to cross the finish line. 🚀
|
Full disclosure: a good percentage of that review was actually Claude, not Sam Erde. 😅 Take it with a grain of salt and push back if necessary! |
Security and correctness: - Set-MtGitHubActionsSecret: pipe secret values via stdin instead of --body so they never appear on the gh command line (process lists / audit logs). - Get-MtGitHubRepoFromGit: tighten regex to (?:www\.)?github\.com — the previous [^/]*github\.com pattern accepted lookalike hosts such as evilgithub.com, github.com.attacker.com, and my-github.com. - Add-MtMaesterAppFederatedCredential: honor -SetGitHubSecrets when a matching federated credential already exists. Previously the duplicate check returned before the secrets block, so re-runs silently skipped the switch. Consistency / UX: - Both cmdlets now require both -GitHubOrganization and -GitHubRepository (or neither, with auto-detection). Partial overrides were ambiguous. - Clarify in help that -SetGitHubSecrets implicitly enables the GitHub flow on New-MtMaesterApp; -GitHubActions does not need to be specified too. - Remove stale -Force parameter documentation from Add-MtMaesterAppFederatedCredential help (was never implemented). Tests: - Get-MtGitHubRepoFromGit.Tests.ps1: 4 new negative-host cases (evilgithub.com, github.com.attacker.com, my-github.com) plus a www. positive case (11/11 pass). - New Set-MtGitHubActionsSecret.Tests.ps1: gh missing / unauthenticated / success / partial-failure paths (4/4 pass). Docs: - Regenerated website/docs/commands/{New-MtMaesterApp, Add-MtMaesterAppFederatedCredential}.mdx so the new switches and examples are reflected in the command reference. Internal: - Extract the manual GitHub Actions secrets fallback block into a new internal helper Write-MtGitHubSecretsManualInstruction so the duplicate-credential path and the post-create path share one implementation.
|
Thanks @SamErde for the thorough review, really appreciated the details feedback! Security / correctness
Consistency / UX
Docs / tests
Internal
Ready for another look. Happy to iterate further if anything still feels off. |
|
Looks amazing! I've been offline all day and will test it out this weekend. |
- Add-MtMaesterAppFederatedCredential: restore error semantics for duplicate-name conflicts. A duplicate subject (same repo/branch) stays a warning + idempotent re-run, but a duplicate *name* whose subject points at a different repo/branch now writes an Error and returns, so automation can detect that the requested credential was not created. - Write-MtGitHubSecretsManualInstruction: new -AttemptedAutomatic switch swaps the "re-run with -SetGitHubSecrets" tip (misleading when the caller already used that switch and gh failed) for actionable guidance about installing / authenticating gh. Threaded through both call sites in Add-MtMaesterAppFederatedCredential. - Get-MtGitHubRepoFromGit: document the supported ssh:// remote URL form in the cmdlet synopsis - the regex and tests already covered it.
|
Thanks @copilot — three good catches. Pushed
All tests still green (32/32), PSScriptAnalyzer clean. No |
Summary
Adds zero-config GitHub Actions / OIDC setup to the existing
New-MtMaesterAppandAdd-MtMaesterAppFederatedCredentialcmdlets. Inside amaester-testsclone, a user can now do the entire pipeline setup with one command:The cmdlet auto-detects the repo from the local
git remote, creates the app, grants permissions, adds the federated credential, and pushesAZURE_CLIENT_ID/AZURE_TENANT_IDto the repo's Actions secrets via the GitHub CLI.Motivation
Follows up on discussion #1503 — "Automated GitHub Actions setup script — from 17 manual steps to one command". As @SamErde suggested in that thread, this PR fits the improvements into the existing
New-MtMaesterApp/Add-MtMaesterAppFederatedCredentialfunctions rather than adding a separate top-level script.What's new
Add-MtMaesterAppFederatedCredential-GitHubOrganization/-GitHubRepositoryare no longer mandatory — they are auto-detected fromgit remote get-url originwhen omitted. New-SetGitHubSecretsswitch pushes the two secrets viagh.New-MtMaesterApp-GitHubActionsswitch enables the end-to-end flow (was previously triggered only when both-GitHubOrganizationand-GitHubRepositorywere supplied). New-SetGitHubSecretsswitch is forwarded to the FIC cmdlet.Two new internal helpers (not exported):
Get-MtGitHubRepoFromGit— parses HTTPS, scp-style SSH, andssh://GitHub remote URLs; returns$nullon any failure (git missing, not a git repo, non-GitHub remote, unparseable URL).Set-MtGitHubActionsSecret— wrapsgh secret set, validatesghis installed and authenticated first, and returns a boolean so the caller can fall back to printing the manual instructions on any failure.Backward compatibility
100% backward-compatible:
New-MtMaesterApp -GitHubOrganization X -GitHubRepository Ystill creates the FIC exactly as before.Add-MtMaesterAppFederatedCredentialis still printed whenever-SetGitHubSecretsis not specified, or whenghis missing / unauthenticated / fails.Validation
Invoke-ScriptAnalyzer(PSGallery profile, Warning + Error) — 0 issues across all changed files.Get-MtGitHubRepoFromGit.Tests.ps1— 7/7 pass (HTTPS with.git, HTTPS without.git, scp-style SSH,ssh://, non-GitHub remote, empty remote, git not on PATH).Manifest.Tests.ps1— 17/17 pass.Docs
Added a small "PowerShell shortcut" tip box at the top of the Workload Identity Federation section in
website/docs/monitoring/github.mdpointing at the new one-liner. The portal-based steps below it are unchanged.Refs: #1503