feat: add GitHub REST connection infrastructure (CIS GH PR 1 of 2)#1756
feat: add GitHub REST connection infrastructure (CIS GH PR 1 of 2)#1756thetechgy wants to merge 24 commits into
Conversation
Introduces the foundation layer for Maester CIS GitHub Enterprise Cloud
benchmark tests. CIS controls consuming this infrastructure arrive in a
follow-up PR; this PR intentionally ships connection-only to allow
maintainer review of the integration pattern before controls are added.
New public function:
- Connect-MtGitHub: validates a PAT via GET /user and GET /orgs/{org},
stores session state in $__MtSession, supports MAESTER_GITHUB_TOKEN
and GH_TOKEN env vars, and is configurable for GHE.com EMU deployments
via -ApiBaseUri.
New internal functions:
- Invoke-MtGitHubRequest: PS 5.1-compatible (Invoke-WebRequest) cached,
paginating GET wrapper with rate-limit detection.
- Get-MtGitHubResponseHeaderValue: case-insensitive header extraction
compatible with both PS 5.1 WebHeaderCollection and PS 7
HttpResponseHeaders.
- Get-MtGitHubErrorStatusCode: safely extracts HTTP status codes
from exception responses cross-platform.
- Get-MtGitHubErrorMessage: extracts GitHub API error body for skip
and error detail output.
Changes to existing files:
- Maester.psm1: adds GitHubCache = @{} to $__MtSession initializer.
- Clear-ModuleVariable.ps1: resets GitHubConnection, GitHubAuthHeader,
and GitHubCache on each Invoke-Maester run.
- Test-MtConnection.ps1: adds 'GitHub' service region using cached
$__MtSession.GitHubConnection state (no auto-detect; requires
explicit Connect-MtGitHub call).
- Add-MtTestResultDetail.ps1: adds 'NotConnectedGitHub' to ValidateSet.
- Get-MtSkippedReason.ps1: adds 'NotConnectedGitHub' skip message.
- Maester.psd1: exports Connect-MtGitHub.
- tests/maester-config.json: adds GitHubOrganization, GitHubApiBaseUri,
and GitHubApiVersion to GlobalSettings.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Covers all branching paths in the cross-version header extraction helper: null input, IDictionary path (PS 5.1 WebHeaderCollection — exact case, different case, array value, missing header), GetValues path (PS 7 HttpResponseHeaders — success and throw), and TryGetValues path (PS 7 — true and false return). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Clear-ModuleVariable nulled GitHubConnection and GitHubAuthHeader on every Invoke-Maester call, discarding any Connect-MtGitHub session established beforehand. Only GitHubCache needs to be reset per run, consistent with how $__MtSession.Connections (MS Graph) is handled. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Connect-MtGitHub relied on Get-MtMaesterConfigGlobalSetting for the Organization, ApiBaseUri, and ApiVersion fallbacks, but MaesterConfig is only populated inside Invoke-Maester — after Clear-ModuleVariable has already run. Calling Connect-MtGitHub before Invoke-Maester (the documented flow) silently dropped all three config fallbacks. Add a lazy-load block that calls Get-MtMaesterConfig once when MaesterConfig is null and any config-backed parameter is omitted, covering all three values rather than Organization alone. Also fix a latent ValidatePattern bug: assigning $null from a config lookup directly to $ApiBaseUri or $ApiVersion (both declared with [ValidatePattern]) throws ValidationMetadataException inside the function body. Use local intermediate variables for the lookups and only update the parameter variable when the result is non-empty. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Clear-ModuleVariable: regression guard verifying GitHubConnection and GitHubAuthHeader survive Invoke-Maester's run-start reset while GitHubCache is cleared (Count -eq 0) and Test-MtConnection GitHub still returns true. Connect-MtGitHub: covers NotConfigured, NoToken, TokenInvalid, and OrgAccessFailed (403 and 404) failure modes; successful connection with probe URI and X-GitHub-Api-Version header assertions; config resolution from pre-loaded MaesterConfig without calling Get-MtMaesterConfig; and lazy-load paths for all three config-backed parameters (Organization, ApiBaseUri, ApiVersion). Invoke-MtGitHubRequest: covers connection guard (null and false), cache hit (web request called once for two identical calls), cache store (key verified as "$version|$absUri"), -DisableCache bypass without overwriting an existing entry, single-page and paginated (Link rel=next) responses, verbose rate-limit warning at remaining=0, primary rate-limit throw (403/429), and secondary rate-limit throw. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Add GitHub to Test-MtConnection comment-based help: valid service list, behavior note (explicit Connect-MtGitHub required, treated as required in -Service All), updated example descriptions, new -Service GitHub example - Improve Get-MtGitHubErrorMessage to extract the JSON .message field from GitHub REST error bodies so failures show clean strings instead of raw JSON - Add Test-MtConnection.Tests.ps1 with 7 GitHub-focused unit tests covering null/sentinel/disconnected/connected states and -Details/-Service All paths Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
GitHub was unconditionally entered when -Service All was used, stamping a NotCalled sentinel and setting ConnectionState = $false whenever Connect-MtGitHub had not been called. This silently broke existing users relying on -Service All for Microsoft services only. GitHub is now skipped under All unless a real connection attempt has been made (GitHubConnection is non-null and FailureReason is not 'NotCalled'). Explicit -Service GitHub remains strict. Update docstrings and examples to match the new opt-in semantics. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The previous -Service All -Details test hit real cmdlets (Get-AzContext, Get-MgContext, Get-MtExo, Get-CsTenant, Get-ADOPSConnection), taking 60+ seconds and emitting Azure token-cache warnings. Replace with four mocked tests using Mock -ModuleName Maester: - -Service GitHub -Details: details-object shape when GitHub connected - Regression test: all MS services mocked connected + GitHub null → AllConnected must be $true (proves the fix, not just property absence) - NotCalled sentinel: GitHub skipped even when sentinel exists from a prior -Service GitHub probe - GitHub connected: property populated when Connect-MtGitHub was called Also reset AzureDevOpsConnection in BeforeEach/AfterEach to prevent state leak between tests. Suite now completes in ~7s. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…st perf
- Get-MtSession: redact GitHubAuthHeader Authorization on output (case-insensitive,
fail-closed for non-dictionary shapes); live session unchanged so internal callers
still work. Prevents PAT disclosure in troubleshooting dumps.
- Add Disconnect-MtGitHub to clear GitHub session state; export from manifest.
- Disconnect-Maester (and Disconnect-MtMaester alias) now also clears GitHub state;
Disconnect-MtGraph alias keeps its narrow Graph-only semantic.
- Connect-MtGitHub: add /orgs/{org}/memberships/{user} probe to verify org role.
New fields Role, RoleState, RoleVerified, RoleVerificationFailureReason
distinguish "known non-admin" from "could not check". Warns on member/pending/
probe-failure/malformed-body without blocking connection.
- Test-MtConnection.Tests.ps1: stub MS service cmdlets in BeforeAll when absent
(cleaned up in AfterAll), bypassing Pester's slow command-resolution path.
Cuts the -Service All regression test from ~63s to ~180ms.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Satisfies the repo's PSUseBOMForUnicodeEncodedFile analyzer rule (enforced by powershell/tests/pester.ps1) on every .ps1 this branch touches that contains non-ASCII content. Without the BOM, build-validation.yaml fails. BOM was prepended via a byte-preserving rewrite (File.ReadAllBytes + Buffer.BlockCopy + File.WriteAllBytes) so file content after the BOM is byte-identical to the prior version. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…nect
Connect-MtGitHub: promote /orgs/{org}/memberships/{user} from advisory to
blocking. /orgs/{org} returns public metadata even for tokens with no
relationship to the org, so it is not a real access proof. 4xx, malformed
body, or missing state/role on the membership probe now sets
FailureReason = 'OrgMembershipFailed' and aborts before storing auth
headers; valid non-admin / pending states still connect with a warning.
Connect-MtGitHub: revalidate the resolved ApiBaseUri after the
param→config→default fallback. [ValidatePattern('^https://')] only fires
on the bound parameter, so a config-sourced http:// URI could otherwise
reach Invoke-WebRequest with a Bearer header. Resolution now uses a local
variable, then enforces an absolute https:// URI via [uri]::TryCreate +
case-sensitive scheme check, failing with FailureReason = 'InvalidApiBaseUri'.
Disconnect-Maester: normalize $MyInvocation.InvocationName by splitting on
the literal '\\' qualifier (PowerShell uses '\\' on all OSes; Split-Path
-Leaf would no-op on Linux). 'Maester\\Disconnect-Maester' now routes to
the GitHub-clearing branch alongside the bare and Disconnect-MtMaester
forms; Disconnect-MtGraph keeps its narrow Graph-only semantic.
Tests: move the five membership-failure cases into a new OrgMembershipFailed
context with Connected=$false assertions; add an InvalidApiBaseUri context
covering http:// config, non-URI config, and trailing-slash trim on a valid
parameter; add a Maester\\Disconnect-Maester clearing test. Failure-path
tests also assert GitHubAuthHeader stays \$null to lock in the security
property that no Bearer header is left behind on a failed connection.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…-MtGitHub
Adds a fourth non-blocking probe to GET /orgs/{org}/actions/permissions
to verify the token can reach an org-administration endpoint. Failure
records AdministrationPermissionVerified = $false and warns about both
classic (admin:org) and fine-grained (Administration: read) permission
models, but does not flip Connected — the session remains usable for
controls that do not require org administration access. Captures the
x-accepted-github-permissions response header on failures.
Validates the resolved ApiVersion locally so a malformed config value
no longer reaches /user as X-GitHub-Api-Version and gets misreported
as TokenInvalid. Removes the parameter [ValidatePattern] so invalid
-ApiVersion input also flows through the resolved-value check, which
ensures session-clearing logic runs first. /user catch now maps 410
and 400 with API/version wording (including GitHub's documented "Not
a supported version" message) to InvalidApiVersion instead of
TokenInvalid.
Updates synopsis to describe the session as organization-scoped (not
enterprise-admin) and lists required token permissions for both PAT
types.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
- Remove [ValidatePattern] on -ApiBaseUri so invalid parameter values flow through the existing in-body URI check after session state is cleared, instead of throwing before the cleanup runs. - Split /user error classification: 401 stays TokenInvalid, $null status code (DNS/TLS/connect failure) becomes ApiBaseUriFailed, and 5xx becomes a new ApiUnavailable reason. Host messages no longer describe transport or service failures as token validation failures. - Refresh stale comments that referenced the removed [ValidatePattern]. - Add regression tests for invalid -ApiBaseUri parameter values, /user transport failures (no Response), and /user 5xx responses. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Adds a GitHub ListItem to the Maester.Connections list view so Test-MtConnection -Service GitHub -Details visibly renders connection details. The formatter only displays safe metadata (Connected, Organization, TokenLogin, ApiBaseUri, Role, RoleState, AdministrationPermissionVerified) and never touches the auth header, Authorization, or token values. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Compute the invocation-name check up front, then wrap the existing Graph/Azure/EXO/Teams disconnect calls in try/finally so Disconnect-MtGitHub runs even when an earlier service-disconnect throws. Original exceptions still propagate. Disconnect-MtGraph alias keeps its Graph-only semantic and does not clear GitHub. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
A malicious or buggy upstream that injects a foreign URL into the Link header would otherwise receive the Authorization header on a cross-origin request. Validate each next URL against the configured ApiBaseUri (scheme, host, port, and base path prefix so GHE-style https://host/api/v3 bases work) and throw before issuing the request when the origin does not match. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Connect-MtGitHub bootstrap probes used raw Invoke-WebRequest catches that
did not detect GitHub primary/secondary rate-limit responses. A valid
token receiving HTTP 403/429 with rate-limit headers could be reported
as TokenInvalid, OrgAccessFailed, OrgMembershipFailed, or as a misleading
admin-permission warning.
Extract rate-limit detection into Get-MtGitHubRateLimitMessage so both
Invoke-MtGitHubRequest and Connect-MtGitHub share one implementation.
The helper uses [int]::TryParse / [long]::TryParse so a malformed
x-ratelimit-remaining or x-ratelimit-reset header cannot mask the
underlying error with a parse exception.
Blocking probes (/user, /orgs/{org}, /orgs/{org}/memberships/{user})
short-circuit on rate limit with FailureReason = 'RateLimited' and do
not store the auth header. The non-blocking admin probe keeps the
session connected but records a rate-limit-specific failure reason and
warning instead of implying missing permissions.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
ConvertFrom-Json '[]' yields $null because it enumerates the empty array, so AddRange(@($body)) was contributing one $null item per empty page. A list endpoint with no results would surface as a single-$null collection. Short-circuit the empty-array case before parsing and filter $null while merging pages so empty intermediate pages also don't pollute the result. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Pending membership (invitation not yet accepted) was previously treated as a successful connection with a warning. Org-scoped controls would silently fail or report misleading data because the user can't actually act on the org until membership is active. Treat any non-active membership state as a connection failure with FailureReason = 'OrgMembershipPending'. Auth header is not retained because the session-clear at the top of Connect-MtGitHub already nulled it before the failure return. Active non-admin roles still connect with a limited-coverage warning, unchanged. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
GitHub returns 403/429 for secondary rate limits, but does not always include a retry-after header (older abuse-detection responses, or response chains where a proxy strips the header). Fall back to matching "secondary rate limit" / "abuse detection" wording in the response body so callers see a rate-limit message instead of a generic permission failure. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…tion Three related Connect-MtGitHub changes from the latest review pass: - Restrict ApiBaseUri to api.github.com and api.<subdomain>.ghe.com, with no path, query, fragment, or non-default port. Rejected URIs short-circuit before any web request runs, so the PAT cannot be sent to an attacker-supplied or misconfigured host. - Apply the existing /user transport/5xx classification to the /orgs and /memberships probes too: a transport failure now reports ApiBaseUriFailed and a 5xx reports ApiUnavailable instead of being conflated with org-access or membership permission failures. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Surrounding whitespace from a config file or shell-quoted parameter would otherwise be URL-encoded into probe paths and silently 404, or fail the ApiVersion format check despite a valid date. Add UTF-8 BOM to the test file so ScriptAnalyzer no longer flags PSUseBOMForUnicodeEncodedFile. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
A malformed x-ratelimit-remaining (proxy-rewritten value) or an out-of-range x-ratelimit-reset epoch would raise a parse exception that masked an otherwise successful GitHub response. Use TryParse for both headers and wrap FromUnixTimeSeconds in try/catch, falling back to 'unknown' when the reset epoch is outside the [-62135596800, 253402300799] range accepted by .NET. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Whitespace-padded ApiBaseUri values from -ApiBaseUri or config-supplied GitHubApiBaseUri were only trailing-slash trimmed, so they passed URI validation but URL-encoded into probe paths and silently 404'd. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Up to standards ✅🟢 Issues
|
|
Thanks, @thetechgy! I'm a bit swamped right now, but look forward to testing this! |
There was a problem hiding this comment.
Pull request overview
This PR introduces the GitHub REST API connection infrastructure that will be consumed by upcoming CIS GitHub Enterprise Cloud Benchmark tests in PR 2. It establishes a session-based GitHub API client with PAT authentication, validation probes, response caching, pagination, rate-limit detection, and integration with existing Maester connection/session management.
Changes:
- Adds
Connect-MtGitHub/Disconnect-MtGitHubpublic cmdlets plus several internal helpers (Invoke-MtGitHubRequest, error/header/rate-limit utilities) with strict URI allowlisting and detailed failure classification. - Extends
Test-MtConnection,Disconnect-Maester,Add-MtTestResultDetail,Get-MtSession, andClear-ModuleVariableto be GitHub-aware (with PAT redaction and lifecycle handling). - Adds three GitHub-related
GlobalSettingskeys tomaester-config.jsonand exports the new public functions in the manifest/format file.
Reviewed changes
Copilot reviewed 26 out of 26 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| tests/maester-config.json | Adds GitHubOrganization/ApiBaseUri/ApiVersion global settings |
| powershell/Maester.psd1 | Exports Connect-MtGitHub and Disconnect-MtGitHub |
| powershell/Maester.psm1 | Adds GitHubCache to $__MtSession |
| powershell/Maester.Format.ps1xml | Adds GitHub block to Maester.Connections list view |
| powershell/public/core/Connect-MtGitHub.ps1 | New: PAT-based GitHub connection with four probes |
| powershell/public/core/Disconnect-MtGitHub.ps1 | New: Clears GitHub session keys idempotently |
| powershell/public/core/Test-MtConnection.ps1 | Adds GitHub service region with NotCalled sentinel |
| powershell/public/core/Get-MtSession.ps1 | Redacts GitHubAuthHeader Authorization on output |
| powershell/public/Disconnect-Maester.ps1 | Routes GitHub cleanup via try/finally for non-Graph aliases |
| powershell/public/Add-MtTestResultDetail.ps1 | Adds NotConnectedGitHub skipped reason |
| powershell/internal/Invoke-MtGitHubRequest.ps1 | New: GET wrapper with cache, pagination, rate-limit detection |
| powershell/internal/Get-MtGitHubErrorStatusCode.ps1 | New: Extracts HTTP status from ErrorRecord |
| powershell/internal/Get-MtGitHubErrorMessage.ps1 | New: Extracts message from JSON ErrorDetails or Exception |
| powershell/internal/Get-MtGitHubResponseHeaderValue.ps1 | New: Cross-version case-insensitive header lookup |
| powershell/internal/Get-MtGitHubRateLimitMessage.ps1 | New: Classifies 403/429 rate-limit responses |
| powershell/internal/Get-MtSkippedReason.ps1 | Adds NotConnectedGitHub explanation |
| powershell/internal/Clear-ModuleVariable.ps1 | Resets GitHubCache only; preserves connection across runs |
| powershell/tests/functions/*.Tests.ps1 (8 files) | Pester coverage for all new code paths |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
📑 Description
Adds the connection layer needed to run GitHub Enterprise Cloud security checks against the GitHub REST API. This is the first of two PRs that bring the CIS GitHub Enterprise Cloud Benchmark v1.2.0 (org-level controls) into Maester. No CIS tests ship in this PR — the consuming
Test-MtCisGh*helpers, their.mdcompanions, Pester tests, andTestSettingsentries land in PR 2 so each PR stays focused on a single type of change.What this PR introduces:
Connect-MtGitHub(public) — establishes a session againstapi.github.comor a GHE.com EMU host, with token resolution via-Token(SecureString),MAESTER_GITHUB_TOKEN, orGH_TOKEN. Validates the PAT withGET /userand probes org reachability withGET /orgs/{org}plus a membership check. Connection success means the token works and the org is reachable; per-control field visibility is validated by each helper in PR 2 so a partially-scoped token still produces useful skip messages instead of a hard failure at connect time.Disconnect-MtGitHub(public) and updatedDisconnect-Maester— clearsGitHubConnection,GitHubAuthHeader, andGitHubCacheon disconnect (and on reconnect, to prevent cross-org cache bleed).Invoke-MtGitHubRequest(internal) — authenticated GET wrapper with per-session response caching, opt-in-Paginate(Link: rel="next"follow withper_page=100), and rate-limit detection on both success and error responses. Built onInvoke-WebRequest+ConvertFrom-Jsonfor PowerShell 5.1 compatibility (Invoke-RestMethod -ResponseHeadersVariableis 7+ only).Get-MtGitHubErrorStatusCode,Get-MtGitHubErrorMessage,Get-MtGitHubResponseHeaderValue(cross-versionWebHeaderCollectionvs. PS7Dictionary<string,string[]>access with case-insensitive lookup), andGet-MtGitHubRateLimitMessage(covers primary rate limits, secondary/abuse-detection responses including the body-wording fallback when GitHub omitsretry-after).Test-MtConnection— adds aGitHubservice region.-Service Alldoes not auto-include GitHub, so existing users runningConnect-Maesterwithout a GitHub PAT see no behavior change.Add-MtTestResultDetailandGet-MtSkippedReason— newNotConnectedGitHubskip reason. Reserved strictly for the truly-not-connected case; all other failures (no token, invalid token, org access denied, missing required field on a 200 response, 403/429 at call time) useCustomwith a specific, GitHub-API-message-preserving reason so the report tells you what to fix.tests/maester-config.json— three newGlobalSettingskeys (GitHubOrganization,GitHubApiBaseUri,GitHubApiVersion).GitHubEnterpriseSlugis intentionally not added until enterprise-level endpoints are implemented in a later phase.Maester.psd1— exportsConnect-MtGitHubandDisconnect-MtGitHub. Internal helpers andInvoke-MtGitHubRequestare not exported; the visibility of the request wrapper can be revisited in PR 2 if maintainers prefer it public.A few design points worth flagging for review:
ApiBaseUriallowlist. Onlyapi.github.comandapi.<subdomain>.ghe.comare accepted, with no path/query/fragment and only the default port. Rejected URIs short-circuit before any web request runs, so a misconfigured or attacker-supplied host can't receive the PAT./user,/orgs/{org}, and the membership probe each distinguish transport failures (ApiBaseUriFailed), 5xx (ApiUnavailable), token problems (TokenInvalid), org access (OrgAccessFailed), and rate limits, so the resultingFailureReasondrives a useful skip message in PR 2's helpers instead of collapsing to a single "couldn't connect" string.$__MtSession.GitHubAuthHeaderonly; the connection object on$__MtSession.GitHubConnectionexposes login, plan, org, ApiBaseUri, and ApiVersion but not the secret.Get-MtSessionandTest-MtConnection -Detailsredact accordingly.two_factor_requirement_enabledreflects the GitHub org setting only. This will be documented on the correspondingTest-MtCisGhOrgTwoFactorRequired.mdcompanion when PR 2 lands.✅ Checks
/powershell/tests/pester.ps1locally.ℹ️ Additional Information
Test-MtConnection -Service Allstill completes without a GitHub connection.Connect-Maesteris unchanged — GitHub requires explicitConnect-MtGitHub, since Maester-style GitHub testing is org-scoped and there is no sensible auto-detection path for a PAT.Connect-MtGitHub(token resolution, URI allowlist, all probe error classifications, membership states, rate-limit detection, redaction),Disconnect-MtGitHub,Disconnect-Maester(GitHub teardown viatry/finally),Invoke-MtGitHubRequest(caching, pagination, rate-limit headers on success and error paths, empty-array pages, cross-originLinkrejection),Get-MtGitHubResponseHeaderValue,Get-MtGitHubRateLimitMessage,Test-MtConnection,Get-MtSession, andClear-ModuleVariable.How to Contribute
🏗️ Read our full contributing guide for the Maester project.
🧪 We also have additional instructions and a checklist for creating tests.
Join us at the Maester repository discussions or Entra Discord for more help and conversations!
While you wait for a review, why not spread some Maester love on social media? Thank you! 💖