Skip to content

Commit a82a282

Browse files
Sergio SisternesCopilot
andcommitted
Address panel review: diagnostic warning, fail-closed tests, stale docs
- Emit logger.warning when .github-private returns absent on a GitHub host -- signals a likely token-scope gap (GitHub returns 404, not 403, for private repos with insufficient scope). - Add test_403_is_error_not_absent: proves 403 -> cache_miss_fetch_fail. - Add test_github_private_auth_error_does_not_fall_through: proves auth error on .github-private stops the cascade (fail-closed). - Add test_github_private_absent_falls_back_to_github: proves the backwards-compat cascade from .github-private to .github. - Assert candidate name in test_github_com_error_fail_closed. - Update 15+ stale .github references in governance-guide sections 11, 12, 15, 16 and policy-reference quickstart checklist. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
1 parent 876b1ba commit a82a282

5 files changed

Lines changed: 77 additions & 13 deletions

File tree

‎docs/src/content/docs/enterprise/apm-policy.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -204,7 +204,7 @@ Commit and push. On the next `apm install` in any repo in the org, a user who de
204204

205205
### 2. Preview before you commit
206206

207-
Point at a policy explicitly to test it before it lands in `.github-private`:
207+
Point at a policy explicitly to test it before it lands in your policy repo (`.github-private` or `.github`):
208208

209209
```bash
210210
apm policy status --policy-source ./apm-policy.yml

‎docs/src/content/docs/enterprise/governance-guide.md‎

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -393,7 +393,7 @@ graph TD
393393
style P4 fill:#fce4ec,stroke:#c2185b,stroke-width:3px,color:#000
394394
```
395395

396-
**Phase 1 -- ship a warn-mode policy.** Land `apm-policy.yml` in `<org>/.github` with `enforcement: warn`. Nothing breaks anywhere. Every `apm install` in the org now discovers the policy, runs the checks, and logs `[!]` warnings for violations -- but proceeds.
396+
**Phase 1 -- ship a warn-mode policy.** Land `apm-policy.yml` in `<org>/.github-private` (or `<org>/.github`) with `enforcement: warn`. Nothing breaks anywhere. Every `apm install` in the org now discovers the policy, runs the checks, and logs `[!]` warnings for violations -- but proceeds.
397397

398398
**Phase 2 -- wire audit into CI.** Use a shared GitHub Actions template (or composite action) that runs `apm audit --ci --policy org -f sarif` and uploads the SARIF to GitHub Code Scanning. Violations become visible to repo owners as code-scanning alerts. Be honest with stakeholders here: in `warn` mode, audit rewrites violations to `passed=True` so the exit code stays 0. CI does not fail. The visibility is in the SARIF + Code Scanning UI, not in the green/red check. Branch protection cannot enforce yet.
399399

@@ -423,9 +423,9 @@ For richer customization (matrix builds, monorepo splits, vendored policy paths)
423423
`apm audit --ci` in warn mode rewrites violations to `passed=True` (audit.py:589-598), so the audit command exits 0 even on policy violations. Visibility is in the SARIF upload + Code Scanning UI, not in branch-protection status. To gate merges on policy violations, you must run in `block` mode.
424424
:::
425425

426-
**Phase 3 -- triage and clean up.** Repo owners either fix their `apm.yml` to comply with the policy, or they open a PR to `<org>/.github/apm-policy.yml` to add an explicit allow entry with rationale. The PR flow is the policy change-management trail (see section 12).
426+
**Phase 3 -- triage and clean up.** Repo owners either fix their `apm.yml` to comply with the policy, or they open a PR to the org policy repo (`<org>/.github-private/apm-policy.yml` or `<org>/.github/apm-policy.yml`) to add an explicit allow entry with rationale. The PR flow is the policy change-management trail (see section 12).
427427

428-
**Phase 4 -- flip to block.** Once Code Scanning shows the violation backlog is drained, change `enforcement: block` in `<org>/.github/apm-policy.yml`. Stage by team if the org is large: a team can adopt block early by setting `enforcement: block` in its own team-level intermediate policy, leaving the org policy at `warn`. (Tighten-only merge means the team's `block` wins for repos under that team's `extends:` chain.) Use `apm policy status --check` in CI as a pre-flight that explains the effective policy and surfaces what would be blocked, before the gate phase actually blocks it.
428+
**Phase 4 -- flip to block.** Once Code Scanning shows the violation backlog is drained, change `enforcement: block` in the org policy repo. Stage by team if the org is large: a team can adopt block early by setting `enforcement: block` in its own team-level intermediate policy, leaving the org policy at `warn`. (Tighten-only merge means the team's `block` wins for repos under that team's `extends:` chain.) Use `apm policy status --check` in CI as a pre-flight that explains the effective policy and surfaces what would be blocked, before the gate phase actually blocks it.
429429

430430
**Circuit-breaker rollout for large fleets.** For 100+ repos, do not flip block org-wide in one commit. Stage: enable `block` for 10% of repos for 1 week (via team-level extends), monitor SARIF alert volume and on-call pages, expand to 50% for 1 week, then 100%. If SARIF volume spikes or on-call escalations cluster, revert to `warn` at the org level (one commit) while you triage.
431431

@@ -437,13 +437,13 @@ For step-by-step CI YAML and SARIF upload examples beyond the snippet above, see
437437

438438
The org policy file is the trust root. Protecting it is on you, not on APM.
439439

440-
- **CODEOWNERS on `<org>/.github/apm-policy.yml`** -- restrict to a security team. Every change requires their review.
441-
- **Branch protection on `<org>/.github` main** -- required reviewers, no force push, no direct push to main, dismiss stale approvals on new commits.
442-
- **GitHub Ruleset on the org `.github` repo** (recommended) -- requires approval from a specific team for any change to policy files. See [GitHub Rulesets](../integrations/github-rulesets/).
440+
- **CODEOWNERS on `<org>/.github-private/apm-policy.yml`** (or `<org>/.github/apm-policy.yml`) -- restrict to a security team. Every change requires their review.
441+
- **Branch protection on `<org>/.github-private` main** (or `<org>/.github`) -- required reviewers, no force push, no direct push to main, dismiss stale approvals on new commits.
442+
- **GitHub Ruleset on the org `.github-private` repo** (recommended) -- requires approval from a specific team for any change to policy files. See [GitHub Rulesets](../integrations/github-rulesets/).
443443
- **Change history is `git log apm-policy.yml`.** Rationale lives in commit messages and PR descriptions. Make commit-message rationale a CODEOWNERS-checked review item.
444444
- **Policy change cooling period** (recommended) -- every change to `apm-policy.yml` requires a PR with rationale and a 24-72 hour waiting period before merge. This is a process control, not a code control, but it is the single most important thing you can add.
445445

446-
**Separation of duties for SOX / SOD-sensitive environments.** CODEOWNERS for `apm-policy.yml` should require approvals from a team distinct from the team authoring the change. Configure GitHub Rulesets on the `<org>/.github` repo to require reviewers from `@org/policy-approvers`, where that team is disjoint from `@org/policy-authors`. The same author cannot self-approve, and the approval team has no commit rights to the policy file directly.
446+
**Separation of duties for SOX / SOD-sensitive environments.** CODEOWNERS for `apm-policy.yml` should require approvals from a team distinct from the team authoring the change. Configure GitHub Rulesets on the `<org>/.github-private` (or `<org>/.github`) repo to require reviewers from `@org/policy-approvers`, where that team is disjoint from `@org/policy-authors`. The same author cannot self-approve, and the approval team has no commit rights to the policy file directly.
447447

448448
**Lint for bypass flags in CI workflows.** Add a pre-merge check that fails any PR introducing a policy-bypass flag without an explicit security review label:
449449

@@ -541,7 +541,7 @@ graph TD
541541
style HubYaml fill:#fce4ec,stroke:#c2185b,stroke-width:1px,color:#000
542542
```
543543

544-
**Minimal policy (1-10 repos)** -- one file at `<org>/.github/apm-policy.yml`:
544+
**Minimal policy (1-10 repos)** -- one file at `<org>/.github-private/apm-policy.yml` (or `<org>/.github/apm-policy.yml`):
545545

546546
```yaml
547547
name: "Starter policy"
@@ -577,14 +577,14 @@ compilation:
577577
**Hub + org + repo (100+ repos)** -- enterprise hub with broad allows, org extending and tightening, repos pinning the hash:
578578

579579
```yaml
580-
# enterprise-hub-org/.github/apm-policy.yml
580+
# enterprise-hub-org/.github-private/apm-policy.yml
581581
name: "Enterprise baseline"
582582
version: "1.0.0"
583583
enforcement: warn
584584
dependencies:
585585
allow: ["microsoft/*", "contoso/*", "partner-corp/*"]
586586
587-
# contoso/.github/apm-policy.yml
587+
# contoso/.github-private/apm-policy.yml
588588
extends: "enterprise-hub-org"
589589
enforcement: block
590590
dependencies:
@@ -601,7 +601,7 @@ policy:
601601
## 16. Where to next
602602

603603
:::tip[Starting a pilot?]
604-
**15-minute path:** copy `templates/apm-policy-starter.yml` to `<your-org>/.github/apm-policy.yml`, wire the CI YAML from section 11 Phase 2, ship in warn mode. Flip to block once SARIF is clean.
604+
**15-minute path:** copy `templates/apm-policy-starter.yml` to `<your-org>/.github-private/apm-policy.yml` (or `<your-org>/.github/apm-policy.yml`), wire the CI YAML from section 11 Phase 2, ship in warn mode. Flip to block once SARIF is clean.
605605
:::
606606

607607
- [`apm-policy.yml`](./apm-policy/) -- the file's mental model.

‎docs/src/content/docs/enterprise/policy-reference.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -943,7 +943,7 @@ All violation messages above flow through `InstallLogger.policy_violation`; unde
943943

944944
Checklist to publish a policy:
945945

946-
1. Create `<org>/.github/apm-policy.yml` in the org's `.github` repository.
946+
1. Create `<org>/.github-private/apm-policy.yml` (or `<org>/.github/apm-policy.yml`) in the org's policy repository.
947947
2. Start from the [Standard org policy](#standard-org-policy) example above and trim it to the minimum that reflects your governance posture.
948948
3. Set `enforcement: warn` first. Let CI surface diagnostics across consuming repos for one cycle without breaking installs.
949949
4. When the warn-cycle is clean, switch to `enforcement: block`. Communicate the change in your org's CHANGELOG/announcements channel — `apm install` will start failing for any non-compliant repo.

‎src/apm_cli/policy/discovery.py‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -700,6 +700,17 @@ def _auto_discover(
700700
candidate_repo,
701701
host,
702702
)
703+
# Warn when .github-private is absent on a GitHub host -- GitHub
704+
# returns 404 (not 403) when the token lacks private-repo scope,
705+
# so an absent result may mask a token-permission problem.
706+
if candidate_repo == ".github-private" and _is_github_host(host):
707+
logger.warning(
708+
"Policy repo %s/%s not found (or token lacks private-repo "
709+
"read scope). If your org publishes policy in .github-private, "
710+
"ensure the token has Contents:read permission on that repo.",
711+
org,
712+
candidate_repo,
713+
)
703714
continue
704715

705716
# Any other outcome (found, error, malformed, etc.) -> return immediately

‎tests/unit/policy/test_discovery.py‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -749,9 +749,62 @@ def test_github_com_error_fail_closed(self, mock_extract, mock_fetch):
749749
result = _auto_discover(Path(tmpdir), no_cache=True)
750750
# Only one call -- error stops the cascade
751751
self.assertEqual(mock_fetch.call_count, 1)
752+
self.assertEqual(mock_fetch.call_args_list[0][0][0], "contoso/.github-private")
752753
self.assertFalse(result.found)
753754
self.assertIn("401", result.error)
754755

756+
@patch("apm_cli.policy.discovery._fetch_from_repo")
757+
@patch("apm_cli.policy.discovery._extract_org_from_git_remote")
758+
def test_403_is_error_not_absent(self, mock_extract, mock_fetch):
759+
"""HTTP 403 -> fail-closed (cache_miss_fetch_fail), not absent."""
760+
mock_extract.return_value = ("contoso", "github.com")
761+
mock_fetch.return_value = PolicyFetchResult(
762+
error="403: Access denied to contoso/.github-private",
763+
outcome="cache_miss_fetch_fail",
764+
)
765+
766+
with tempfile.TemporaryDirectory() as tmpdir:
767+
result = _auto_discover(Path(tmpdir), no_cache=True)
768+
self.assertEqual(mock_fetch.call_count, 1)
769+
self.assertEqual(result.outcome, "cache_miss_fetch_fail")
770+
self.assertNotEqual(result.outcome, "absent")
771+
self.assertFalse(result.found)
772+
773+
@patch("apm_cli.policy.discovery._fetch_from_repo")
774+
@patch("apm_cli.policy.discovery._extract_org_from_git_remote")
775+
def test_github_private_auth_error_does_not_fall_through(self, mock_extract, mock_fetch):
776+
""".github-private auth error -> fail-closed, .github NOT tried."""
777+
mock_extract.return_value = ("contoso", "github.com")
778+
mock_fetch.side_effect = [
779+
PolicyFetchResult(error="403: Access denied", outcome="cache_miss_fetch_fail"),
780+
PolicyFetchResult(policy=ApmPolicy(), outcome="found"),
781+
]
782+
783+
with tempfile.TemporaryDirectory() as tmpdir:
784+
result = _auto_discover(Path(tmpdir), no_cache=True)
785+
# Only .github-private tried -- error stops cascade
786+
self.assertEqual(mock_fetch.call_count, 1)
787+
self.assertFalse(result.found)
788+
789+
@patch("apm_cli.policy.discovery._fetch_from_repo")
790+
@patch("apm_cli.policy.discovery._extract_org_from_git_remote")
791+
def test_github_private_absent_falls_back_to_github(self, mock_extract, mock_fetch):
792+
""".github-private absent (404) -> cascade to .github."""
793+
mock_extract.return_value = ("contoso", "github.com")
794+
mock_fetch.side_effect = [
795+
PolicyFetchResult(outcome="absent"), # .github-private 404
796+
PolicyFetchResult(
797+
policy=ApmPolicy(), source="org:contoso/.github", outcome="found"
798+
), # .github found
799+
]
800+
801+
with tempfile.TemporaryDirectory() as tmpdir:
802+
result = _auto_discover(Path(tmpdir), no_cache=True)
803+
self.assertEqual(mock_fetch.call_count, 2)
804+
self.assertEqual(mock_fetch.call_args_list[0][0][0], "contoso/.github-private")
805+
self.assertEqual(mock_fetch.call_args_list[1][0][0], "contoso/.github")
806+
self.assertTrue(result.found)
807+
755808
@patch("apm_cli.policy.discovery._fetch_from_repo")
756809
@patch("apm_cli.policy.discovery._extract_org_from_git_remote")
757810
def test_ghe_repo_ref_includes_host(self, mock_extract, mock_fetch):

0 commit comments

Comments
 (0)