diff --git a/.github/rulesets/main-protection.json b/.github/rulesets/main-protection.json index 8132af8..8f94e45 100644 --- a/.github/rulesets/main-protection.json +++ b/.github/rulesets/main-protection.json @@ -32,6 +32,6 @@ } ], "bypass_actors": [ - {"actor_id": 5, "actor_type": "RepositoryRole", "bypass_mode": "always"} + {"actor_id": 15368, "actor_type": "Integration", "bypass_mode": "pull_request"} ] } diff --git a/.github/workflows/news-sync.yml b/.github/workflows/news-sync.yml index 332e986..d283a91 100644 --- a/.github/workflows/news-sync.yml +++ b/.github/workflows/news-sync.yml @@ -7,6 +7,11 @@ on: permissions: contents: write + pull-requests: write + +concurrency: + group: news-sync + cancel-in-progress: false jobs: sync-news: @@ -29,11 +34,29 @@ jobs: YOUTUBE_API_KEY: ${{ secrets.YOUTUBE_API_KEY }} run: ./sap-devs news fetch-index --output content/packs/base/news-episodes.json - - name: Commit if changed + - name: Open or update PR if changed + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} run: | git config user.name "github-actions[bot]" git config user.email "github-actions[bot]@users.noreply.github.com" - git diff --quiet content/packs/base/news-episodes.json 2>/dev/null || \ - (git add content/packs/base/news-episodes.json && \ - git commit -m "chore: update news episode index" && \ - git push) + if git diff --quiet content/packs/base/news-episodes.json 2>/dev/null; then + echo "No changes." + exit 0 + fi + BRANCH="bot/news-sync-update" + git checkout -B "$BRANCH" + git add content/packs/base/news-episodes.json + git commit -m "chore: update news episode index" + git push -f origin "$BRANCH" + EXISTING=$(gh pr list --head "$BRANCH" --json number -q '.[0].number // empty' 2>/dev/null || echo "") + if [ -z "$EXISTING" ]; then + gh pr create \ + --title "chore: update news episode index" \ + --body "Automated update of \`content/packs/base/news-episodes.json\`." \ + --base main --head "$BRANCH" + else + echo "PR #$EXISTING already open; new commit pushed to $BRANCH" + fi + gh pr merge --auto --squash "$BRANCH" || \ + echo "::warning::auto-merge not armed; check 'Allow auto-merge' is enabled in repo Settings" diff --git a/CLAUDE.md b/CLAUDE.md index a293a30..aeab330 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -230,7 +230,7 @@ gh release edit vX.Y.Z --draft=false --latest --notes "release notes here" 3. `sign-windows.yml` (workflow_run on Tray success, env: `signing`) → **pauses for required-reviewer approval** (OSPO gate), then Authenticode-signs Windows `.exe` binaries via SignPath.io and publishes the release (best-effort) 4. Manual: `gh release edit --draft=false` only needed if signing is skipped — otherwise the signing job publishes automatically after approval -**OSPO compliance:** `main` is protected by ruleset (`main-protection`: requires PR + CI `test` check, blocks force-push and deletion, admins can bypass). Workflows that touch secrets or publish artifacts run in named environments: `release`, `signing` (required reviewer = repo admin), `news-sync`. SignPath secrets and `YOUTUBE_API_KEY` should be scoped to their respective environments rather than the org/repo level. Ruleset spec: [.github/rulesets/main-protection.json](.github/rulesets/main-protection.json). +**OSPO compliance:** `main` is protected by ruleset (`main-protection`: requires PR + CI `test` check, blocks force-push and deletion). Admins **cannot** bypass — every change to `main` goes through a PR with one approving review and a passing `test` check. The `github-actions[bot]` integration has a narrow `pull_request`-mode bypass so automated PRs (notably `news-sync`) can self-merge once their `test` check passes; it cannot push directly to `main`. Workflows that touch secrets or publish artifacts run in named environments: `release`, `signing` (required reviewer = repo admin), `news-sync`. SignPath secrets and `YOUTUBE_API_KEY` should be scoped to their respective environments rather than the org/repo level. Ruleset spec: [.github/rulesets/main-protection.json](.github/rulesets/main-protection.json). **Artifacts per release:** CLI binaries (linux/amd64, linux/arm64, darwin/amd64, darwin/arm64, windows/amd64) + tray binaries (linux/amd64, darwin/arm64, windows/amd64) + checksums + tray-checksums + Scoop manifest + Homebrew cask. diff --git a/docs/ospo-admin-demotion-draft.md b/docs/ospo-admin-demotion-draft.md new file mode 100644 index 0000000..4021829 --- /dev/null +++ b/docs/ospo-admin-demotion-draft.md @@ -0,0 +1,69 @@ +# OSPO Control 7 — Admin Demotion Draft + +**Status:** Draft — pending project owner action. **Nothing is executed by the OSPO hardening PR.** + +OSPO Hardening Control 7 ("Access Restrictions") flagged 25 admins on this repository, well above the least-privilege threshold of 3–5. This document classifies the roster and proposes per-account actions. + +## Bucket 1 — Project leads (keep admin) + +| Login | Rationale | +|---|---| +| `qmacro` | Project lead | +| `jung-thomas` | Project owner | +| `ajmaradiaga` | Active maintainer (Developer Advocacy) | + +## Bucket 2 — Org-mandated (cannot demote at repo level) + +These admins are inherited from SAP organization-level membership or are platform automation. Removing them from the repo collaborator list does not actually remove their admin powers; that requires an OSPO ticket or org-admin action. + +- `SAP-OSPO-ADMIN` +- `sap-ospo-bot` +- `SebastianWolf-SAP` +- `nicoschoenteich` +- `christianneu` +- `dellagustin-sap` + +**Action:** verify with OSPO. If the inherited grant is intentional, leave alone. If a subset can be pruned via OSPO ticket, file one. + +## Bucket 3 — Inherited / unclear (propose demote to `maintain` or `write`) + +These accounts have admin but no obvious active stake in this repo. Recommended action: demote each to `maintain` (still allows triaging issues, accepting PRs, managing labels) or `write` (just code contribution). Confirm per-account before demoting; demotion notifies the user. + +| Login | Suggested role after demotion | +|---|---| +| `akula86` | maintain | +| `ihrigb` | maintain | +| `vipinvkmenon` | maintain | +| `Sygyzmundovych` | maintain | +| `KevinRiedelsheimer` | maintain | +| `rbrainey` | maintain | +| `thecodester` | maintain | +| `ajinkyapatil8190` | maintain | +| `Shegox` | maintain | +| `rich-heilman` | maintain | +| `noravth` | maintain | +| `sheenamk` | maintain | +| `PoojaGidaveer` | maintain | +| `btbernard` | maintain | +| `ajaysoreng` | maintain | +| `neelamegams` | maintain | + +## Demotion command (per account) + +```bash +gh api -X PUT repos/SAP-samples/sap-devs-cli/collaborators/ \ + -f permission=maintain +``` + +Expected response: `204 No Content`. The user receives an email notification. + +## Expected end state + +| Bucket | Count | +|---|---| +| Admin (project leads) | 3 | +| Admin (org-mandated, awaiting OSPO) | 6 | +| Demoted to `maintain` | 16 | +| **Net admin count** | **9** (or fewer once OSPO bucket is pruned) | + +This brings the repo close to OSPO Control 7's least-privilege threshold (3–5) without disrupting active maintainers. diff --git a/docs/superpowers/plans/2026-06-01-ospo-hardening.md b/docs/superpowers/plans/2026-06-01-ospo-hardening.md new file mode 100644 index 0000000..2157fd9 --- /dev/null +++ b/docs/superpowers/plans/2026-06-01-ospo-hardening.md @@ -0,0 +1,601 @@ +# OSPO Hardening Updates Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Move OSPO Hardening Controls 5 (branch protection) from "Action Required" to "Pass" by removing admin bypass on `main`, and refactor the news-sync workflow to a PR-based flow so it keeps working under the new restrictions. + +**Architecture:** Three coordinated changes: (1) ruleset JSON swap (admin-bypass → narrow github-actions[bot] PR-merge bypass), (2) `news-sync.yml` refactor (direct push → PR + auto-merge), (3) CLAUDE.md update + admin demotion draft document. Spec at [docs/superpowers/specs/2026-06-01-ospo-hardening-design.md](../specs/2026-06-01-ospo-hardening-design.md). + +**Tech Stack:** GitHub rulesets JSON, GitHub Actions YAML, `gh` CLI, bash. No application code touched, no Go changes. + +--- + +## File Structure + +| File | Action | Responsibility | +|---|---|---| +| `.github/rulesets/main-protection.json` | Modify | Source-of-truth ruleset; `bypass_actors` swap | +| `.github/workflows/news-sync.yml` | Modify | Schedule + fetch + open-PR-with-auto-merge (replaces direct push) | +| `docs/superpowers/specs/2026-06-01-ospo-hardening-design.md` | Already exists | Spec, no further edits | +| `CLAUDE.md` | Modify | Update "OSPO compliance" paragraph in the Release section to reflect new posture | +| `docs/ospo-admin-demotion-draft.md` | Create | The 16-account demotion draft from Change 3 of the spec, for the project owner to act on | + +The ruleset and workflow files are tightly coupled — neither works correctly without the other — so they're staged in deployment order (workflow first, ruleset second) per the spec's "Deployment ordering" section. + +**Verification note:** The plan does not include unit tests because nothing in the change is application code. The spec defines an empirical testing plan; that's reproduced as Task 8 and must be run after merge + ruleset import. Pre-merge verification is limited to syntactic validation: `jq` for the ruleset JSON (Task 1) and `yq` + `shellcheck` for the workflow YAML and embedded shell (Tasks 2–4). GitHub's REST API exposes no dry-run/check sub-resource on rulesets, so semantic validation of `actor_id` / `actor_type` / `bypass_mode` happens server-side at import time (Task 8); a 422 there fails harmlessly without changing the active ruleset. + +--- + +## Task 1: Update ruleset JSON + +**Files:** +- Modify: `.github/rulesets/main-protection.json:34-36` (`bypass_actors` array) + +- [ ] **Step 1: Inspect current ruleset state** + +Run: `cat .github/rulesets/main-protection.json` + +Expected: file ends with the existing `bypass_actors` array containing `{"actor_id": 5, "actor_type": "RepositoryRole", "bypass_mode": "always"}`. + +- [ ] **Step 2: Apply the bypass-actor swap** + +Replace the contents of `.github/rulesets/main-protection.json` with: + +```json +{ + "name": "main-protection", + "target": "branch", + "enforcement": "active", + "conditions": { + "ref_name": { + "include": ["refs/heads/main"], + "exclude": [] + } + }, + "rules": [ + {"type": "deletion"}, + {"type": "non_fast_forward"}, + { + "type": "pull_request", + "parameters": { + "required_approving_review_count": 1, + "dismiss_stale_reviews_on_push": false, + "require_code_owner_review": false, + "require_last_push_approval": false, + "required_review_thread_resolution": false + } + }, + { + "type": "required_status_checks", + "parameters": { + "strict_required_status_checks_policy": false, + "required_status_checks": [ + {"context": "test"} + ] + } + } + ], + "bypass_actors": [ + {"actor_id": 15368, "actor_type": "Integration", "bypass_mode": "pull_request"} + ] +} +``` + +The only substantive diff vs the previous version is the single `bypass_actors` entry: `RepositoryRole 5 / always` → `Integration 15368 / pull_request`. All other rules are unchanged. + +- [ ] **Step 3: Validate JSON syntax** + +```bash +cat .github/rulesets/main-protection.json | jq . > /dev/null && echo "JSON OK" +``` + +Expected: `JSON OK`. Exit 0. + +Note: there is no public GitHub REST endpoint for *dry-run* validation of a ruleset payload (the `/repos/{owner}/{repo}/rulesets/{id}` endpoints are GET / PUT / DELETE only — no `/check` sub-resource). Semantic validation of fields like `actor_id`, `actor_type`, `bypass_mode` happens server-side at import time (Task 8, Step 2). A typo there returns a 422 with field-level errors and the import fails harmlessly without changing the active ruleset. + +- [ ] **Step 4: Commit** + +```bash +git add .github/rulesets/main-protection.json +git commit -m "chore(ospo): swap admin bypass for narrow github-actions[bot] bypass + +Removes RepositoryRole 5 (Admin) bypass with mode 'always' and replaces +it with Integration 15368 (github-actions[bot]) bypass scoped to +'pull_request' merges only. + +Effect: humans (including admins) can no longer push directly to main +or merge PRs that fail the rules. The bot can self-merge its own PRs +(e.g. news-sync) without an approver, but the required 'test' status +check still gates the merge. + +Addresses OSPO Hardening Control 5. +Spec: docs/superpowers/specs/2026-06-01-ospo-hardening-design.md" +``` + +--- + +## Task 2: Add `pull-requests: write` permission and concurrency to news-sync workflow + +**Files:** +- Modify: `.github/workflows/news-sync.yml:8-9` (top-level `permissions:` block) +- Modify: `.github/workflows/news-sync.yml` (add new top-level `concurrency:` block) + +This task only adds the new top-level YAML keys. The actual step replacement happens in Task 3 to keep diffs reviewable. + +- [ ] **Step 1: Read the current workflow** + +Run: `cat .github/workflows/news-sync.yml` + +Expected: file shows `permissions: contents: write` and no `concurrency:` block. + +- [ ] **Step 2: Update the `permissions:` block** + +In `.github/workflows/news-sync.yml`, replace: + +```yaml +permissions: + contents: write +``` + +with: + +```yaml +permissions: + contents: write + pull-requests: write +``` + +- [ ] **Step 3: Add a top-level `concurrency:` block** + +Insert immediately after the `permissions:` block (before `jobs:`): + +```yaml +concurrency: + group: news-sync + cancel-in-progress: false +``` + +`cancel-in-progress: false` ensures a manual `workflow_dispatch` run started while the cron run is in flight will *queue* rather than abort it — preventing a force-push race on `bot/news-sync-update`. + +- [ ] **Step 4: Validate YAML syntax** + +Run: `yq '.' .github/workflows/news-sync.yml > /dev/null && echo OK` + +Expected: `OK`. Exit code 0. + +- [ ] **Step 5: Commit** + +```bash +git add .github/workflows/news-sync.yml +git commit -m "chore(news-sync): add pull-requests:write permission and concurrency group + +Prepares news-sync for PR-based flow (Task 3). + +- pull-requests: write needed for 'gh pr create' and 'gh pr merge' +- concurrency: news-sync serializes overlapping cron + manual runs to + prevent force-push races on bot/news-sync-update" +``` + +--- + +## Task 3: Replace direct push with PR-based flow in news-sync + +**Files:** +- Modify: `.github/workflows/news-sync.yml:32-39` (the existing "Commit if changed" step) + +- [ ] **Step 1: Identify the step to replace** + +Run: `grep -n "Commit if changed" .github/workflows/news-sync.yml` + +Expected: matches the step with `run: |` block doing `git commit && git push`. + +- [ ] **Step 2: Replace the "Commit if changed" step** + +Replace the entire step (currently lines ~32–39) with: + +```yaml + - name: Open or update PR if changed + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + run: | + git config user.name "github-actions[bot]" + git config user.email "github-actions[bot]@users.noreply.github.com" + if git diff --quiet content/packs/base/news-episodes.json 2>/dev/null; then + echo "No changes." + exit 0 + fi + BRANCH="bot/news-sync-update" + git checkout -B "$BRANCH" + git add content/packs/base/news-episodes.json + git commit -m "chore: update news episode index" + git push -f origin "$BRANCH" + EXISTING=$(gh pr list --head "$BRANCH" --json number -q '.[0].number // empty' 2>/dev/null || echo "") + if [ -z "$EXISTING" ]; then + gh pr create \ + --title "chore: update news episode index" \ + --body "Automated update of \`content/packs/base/news-episodes.json\`." \ + --base main --head "$BRANCH" + else + echo "PR #$EXISTING already open; new commit pushed to $BRANCH" + fi + gh pr merge --auto --squash "$BRANCH" || \ + echo "::warning::auto-merge not armed; check 'Allow auto-merge' is enabled in repo Settings" +``` + +Key design points (cross-reference the spec's "Change 2" section): + +- `gh pr merge` accepts a branch name directly — no fragile output-parsing of `gh pr create`. +- Calling `gh pr merge --auto` repeatedly is idempotent; if auto-merge is already armed, gh prints a notice and exits 0. +- The required `test` status check fires via `ci.yml`'s `push` trigger on `bot/news-sync-update`, satisfying the rule by check-run name. + +- [ ] **Step 3: Validate the full YAML** + +Run: `yq '.jobs."sync-news".steps' .github/workflows/news-sync.yml` + +Expected: structured output listing all steps including the new `Open or update PR if changed` step. No parse errors. + +- [ ] **Step 4: Lint with shellcheck (the embedded shell script)** + +Extract the run block into a temp file and shellcheck it: + +```bash +yq '.jobs."sync-news".steps[] | select(.name == "Open or update PR if changed") | .run' \ + .github/workflows/news-sync.yml > /tmp/news-sync-step.sh +shellcheck -s bash /tmp/news-sync-step.sh +``` + +Expected: no errors. (Warnings about `${BRANCH}` vs `$BRANCH` in shellcheck SC2086 are acceptable — the variable is not user-controlled.) + +- [ ] **Step 5: Commit** + +```bash +git add .github/workflows/news-sync.yml +git commit -m "feat(news-sync): open auto-merging PR instead of pushing to main + +The previous flow ran 'git commit && git push' directly to main. After +the OSPO hardening ruleset takes effect (sister commit), direct pushes +to main are blocked even for the bot. This step instead: + + 1. Pushes to a stable bot branch (bot/news-sync-update, force-push) + 2. Opens a PR via 'gh pr create' if one isn't already open + 3. Arms 'gh pr merge --auto --squash' which fires when the test check + passes + +The bot's narrow ruleset bypass (Integration 15368 / pull_request mode) +allows the auto-merge to proceed without a human approver. + +Spec: docs/superpowers/specs/2026-06-01-ospo-hardening-design.md" +``` + +--- + +## Task 4: Static validation of the workflow refactor + +The spec says "Verification is empirical (the ruleset only takes effect server-side after import)" — so the meaningful verification of the new news-sync flow happens **post-merge** (Task 8). Pre-merge, the most we can soundly do is static validation: parse the YAML, shellcheck the embedded script, and confirm the workflow structure matches expectations. Running `workflow_dispatch` against the feature branch is **not safe** — `Allow auto-merge` is not yet enabled (it's a post-merge operational step), so `gh pr merge --auto` would error out and either short-circuit on `git diff --quiet` (validating nothing) or open a real PR against `main` from a feature branch run. + +- [ ] **Step 1: Verify the full workflow YAML parses** + +```bash +yq '.' .github/workflows/news-sync.yml > /dev/null && echo "YAML OK" +``` + +Expected: `YAML OK`. Exit 0. + +- [ ] **Step 2: Confirm the new step is wired correctly** + +```bash +yq -r '.jobs."sync-news".steps[] | select(.name == "Open or update PR if changed") | .name' \ + .github/workflows/news-sync.yml +``` + +Expected: prints `Open or update PR if changed`. If it prints nothing, the step name in Task 3 was applied incorrectly. + +- [ ] **Step 3: Confirm `permissions:` and `concurrency:` are wired** + +```bash +yq -r '.permissions["pull-requests"], .concurrency.group' .github/workflows/news-sync.yml +``` + +Expected output (two lines): + +```text +write +news-sync +``` + +- [ ] **Step 4: Shellcheck the embedded script** + +```bash +yq -r '.jobs."sync-news".steps[] | select(.name == "Open or update PR if changed") | .run' \ + .github/workflows/news-sync.yml > /tmp/news-sync-step.sh +shellcheck -s bash /tmp/news-sync-step.sh +``` + +Expected: no errors. SC2086 / SC2155 warnings about variable expansion are acceptable (variables are not user-controlled). + +- [ ] **Step 5: No commit needed** — Task 4 is verification only. + +**Live verification of the new flow happens post-merge in Task 8.** + +--- + +## Task 5: Update CLAUDE.md OSPO compliance paragraph + +**Files:** +- Modify: `CLAUDE.md` (the "OSPO compliance" paragraph in the Release section — locate via `grep`, line numbers shift between commits) + +- [ ] **Step 1: Locate the current paragraph** + +Run: `grep -n "OSPO compliance" CLAUDE.md` + +Expected: matches the line containing the text `**OSPO compliance:** \`main\` is protected by ruleset...admins can bypass...`. Note the resulting line number — Step 2 modifies that single line. + +- [ ] **Step 2: Replace the paragraph** + +Replace the line currently reading: + +```markdown +**OSPO compliance:** `main` is protected by ruleset (`main-protection`: requires PR + CI `test` check, blocks force-push and deletion, admins can bypass). Workflows that touch secrets or publish artifacts run in named environments: `release`, `signing` (required reviewer = repo admin), `news-sync`. SignPath secrets and `YOUTUBE_API_KEY` should be scoped to their respective environments rather than the org/repo level. Ruleset spec: [.github/rulesets/main-protection.json](.github/rulesets/main-protection.json). +``` + +with: + +```markdown +**OSPO compliance:** `main` is protected by ruleset (`main-protection`: requires PR + CI `test` check, blocks force-push and deletion). Admins **cannot** bypass — every change to `main` goes through a PR with one approving review and a passing `test` check. The `github-actions[bot]` integration has a narrow `pull_request`-mode bypass so automated PRs (notably `news-sync`) can self-merge once their `test` check passes; it cannot push directly to `main`. Workflows that touch secrets or publish artifacts run in named environments: `release`, `signing` (required reviewer = repo admin), `news-sync`. SignPath secrets and `YOUTUBE_API_KEY` should be scoped to their respective environments rather than the org/repo level. Ruleset spec: [.github/rulesets/main-protection.json](.github/rulesets/main-protection.json). +``` + +- [ ] **Step 3: Verify the change** + +Run: `grep -A1 "OSPO compliance" CLAUDE.md | head -3` + +Expected: output contains "Admins **cannot** bypass". + +- [ ] **Step 4: Commit** + +```bash +git add CLAUDE.md +git commit -m "docs: reflect new OSPO posture in CLAUDE.md + +Admins no longer bypass the main ruleset; only github-actions[bot] has +a narrow PR-merge bypass for automation." +``` + +--- + +## Task 6: Create admin demotion draft document + +**Files:** +- Create: `docs/ospo-admin-demotion-draft.md` + +This is the operational handoff for Change 3 of the spec — a document the project owner can act on without re-deriving the classification. + +- [ ] **Step 1: Create the file** + +Write this content to `docs/ospo-admin-demotion-draft.md`: + +```markdown +# OSPO Control 7 — Admin Demotion Draft + +**Status:** Draft — pending project owner action. **Nothing is executed by the OSPO hardening PR.** + +OSPO Hardening Control 7 ("Access Restrictions") flagged 25 admins on this repository, well above the least-privilege threshold of 3–5. This document classifies the roster and proposes per-account actions. + +## Bucket 1 — Project leads (keep admin) + +| Login | Rationale | +|---|---| +| `qmacro` | Project lead | +| `jung-thomas` | Project owner | +| `ajmaradiaga` | Active maintainer (Developer Advocacy) | + +## Bucket 2 — Org-mandated (cannot demote at repo level) + +These admins are inherited from SAP organization-level membership or are platform automation. Removing them from the repo collaborator list does not actually remove their admin powers; that requires an OSPO ticket or org-admin action. + +- `SAP-OSPO-ADMIN` +- `sap-ospo-bot` +- `SebastianWolf-SAP` +- `nicoschoenteich` +- `christianneu` +- `dellagustin-sap` + +**Action:** verify with OSPO. If the inherited grant is intentional, leave alone. If a subset can be pruned via OSPO ticket, file one. + +## Bucket 3 — Inherited / unclear (propose demote to `maintain` or `write`) + +These accounts have admin but no obvious active stake in this repo. Recommended action: demote each to `maintain` (still allows triaging issues, accepting PRs, managing labels) or `write` (just code contribution). Confirm per-account before demoting; demotion notifies the user. + +| Login | Suggested role after demotion | +|---|---| +| `akula86` | maintain | +| `ihrigb` | maintain | +| `vipinvkmenon` | maintain | +| `Sygyzmundovych` | maintain | +| `KevinRiedelsheimer` | maintain | +| `rbrainey` | maintain | +| `thecodester` | maintain | +| `ajinkyapatil8190` | maintain | +| `Shegox` | maintain | +| `rich-heilman` | maintain | +| `noravth` | maintain | +| `sheenamk` | maintain | +| `PoojaGidaveer` | maintain | +| `btbernard` | maintain | +| `ajaysoreng` | maintain | +| `neelamegams` | maintain | + +## Demotion command (per account) + +```bash +gh api -X PUT repos/SAP-samples/sap-devs-cli/collaborators/ \ + -f permission=maintain +``` + +Expected response: `204 No Content`. The user receives an email notification. + +## Expected end state + +| Bucket | Count | +|---|---| +| Admin (project leads) | 3 | +| Admin (org-mandated, awaiting OSPO) | 6 | +| Demoted to `maintain` | 16 | +| **Net admin count** | **9** (or fewer once OSPO bucket is pruned) | + +This brings the repo close to OSPO Control 7's least-privilege threshold (3–5) without disrupting active maintainers. +``` + +- [ ] **Step 2: Verify file** + +Run: `wc -l docs/ospo-admin-demotion-draft.md` + +Expected: ~70 lines, exit 0. + +- [ ] **Step 3: Commit** + +```bash +git add docs/ospo-admin-demotion-draft.md +git commit -m "docs: admin demotion draft for OSPO Control 7 + +Classifies all 25 current admins into project leads (3, keep), org-mandated +(6, cannot demote at repo level), and inherited/unclear (16, propose +demote to maintain). + +No demotions are executed by this PR. The project owner reviews and +acts per-account." +``` + +--- + +## Task 7: Open PR and confirm pre-merge state + +- [ ] **Step 1: Push the latest state** + +```bash +git push origin ospo-hardening-2026-06 +``` + +- [ ] **Step 2: Open the PR** + +```bash +gh pr create \ + --title "chore(ospo): close Hardening Controls 5 + 7 findings" \ + --body "$(cat <<'EOF' +## Summary + +Closes the OSPO Hardening Controls 5 ("Action Required") and partially addresses Control 7 ("Warning") findings on this repo. + +## Changes + +1. **Ruleset** — \`.github/rulesets/main-protection.json\`: swap admin (\`RepositoryRole 5 / always\`) bypass for narrow \`github-actions[bot]\` (\`Integration 15368 / pull_request\`) bypass. After this, admins cannot bypass; only the bot can self-merge its own PRs (still gated by the required \`test\` status check). +2. **Workflow** — \`.github/workflows/news-sync.yml\`: refactor direct \`git push origin main\` to a PR-based flow (\`gh pr create\` + \`gh pr merge --auto --squash\`). Adds \`pull-requests: write\` permission and a \`concurrency: news-sync\` block. +3. **Docs** — \`CLAUDE.md\`: update OSPO compliance paragraph. \`docs/ospo-admin-demotion-draft.md\`: new draft document classifying the 25-admin roster for the project owner to act on (no demotions executed in this PR). + +## Spec + +[docs/superpowers/specs/2026-06-01-ospo-hardening-design.md](docs/superpowers/specs/2026-06-01-ospo-hardening-design.md) + +## Deployment ordering (post-merge) + +1. **Merge this PR** under the current admin-bypass ruleset (this is the last admin-bypassable merge). +2. **Enable** Settings → General → Pull Requests → **Allow auto-merge**. +3. **Import** the new \`main-protection\` ruleset via Settings → Rulesets → Import (use the file in \`.github/rulesets/main-protection.json\`). +4. **Verify** by triggering \`news-sync.yml\` manually and confirming a PR opens, auto-merges, and \`main\` updates. + +## Rollback + +\`git revert\` this PR's merge commit and re-import the previous ruleset version. One \`gh api\` call away. + +EOF +)" \ + --base main --head ospo-hardening-2026-06 \ + --label "ospo,security,documentation" +``` + +Expected: PR URL printed. + +- [ ] **Step 3: Confirm CI passes** + +```bash +gh pr checks --watch +``` + +Expected: `test` check passes (no application code changed; failure here would indicate an unrelated CI regression). + +- [ ] **Step 4: No commit needed.** Hand off to human reviewer for approval. + +--- + +## Task 8: Post-merge verification (operational, not pre-merge) + +**This task runs after the PR is merged.** It is the spec's empirical testing plan, reproduced as actionable steps. The maintainer (or the user, post-merge) executes this. + +- [ ] **Step 1: Enable Allow auto-merge** + +Settings → General → Pull Requests → check **Allow auto-merge** → Save. + +Verify with: + +```bash +gh api repos/SAP-samples/sap-devs-cli -q .allow_auto_merge +``` + +Expected: `true`. + +- [ ] **Step 2: Import the new ruleset** + +```bash +RULESET_ID=$(gh api repos/SAP-samples/sap-devs-cli/rulesets \ + -q '.[] | select(.name=="main-protection") | .id') +if [ -z "$RULESET_ID" ]; then + echo "main-protection ruleset not found on server." + echo "Create it via Settings → Rulesets → New ruleset → Import a ruleset" + echo "and select .github/rulesets/main-protection.json, then re-run this step." + exit 1 +fi +gh api -X PUT "repos/SAP-samples/sap-devs-cli/rulesets/$RULESET_ID" \ + --input .github/rulesets/main-protection.json +``` + +Or use the GitHub UI: Settings → Rulesets → main-protection → Edit → Import → select the file. + +- [ ] **Step 3: Verify direct-push to main is blocked** + +From a maintainer clone: + +```bash +git checkout main && git pull +echo "test" >> /tmp/scratch +git commit --allow-empty -m "should be rejected" +git push origin main +``` + +Expected: rejection with a ruleset violation message. Reset with `git reset --hard origin/main`. + +- [ ] **Step 4: Verify a real news-sync run produces a PR** + +```bash +gh workflow run news-sync.yml +sleep 60 +gh pr list --head bot/news-sync-update +``` + +Expected: a PR exists (if there were content changes since the last sync). Watch it auto-merge once `test` passes. + +- [ ] **Step 5: Verify the release pipeline still works (optional, only if a release is due)** + +```bash +gh workflow run release.yml -f version=X.Y.Z # next semver +``` + +Expected: tag pushes successfully; release artifacts upload. Tags are not gated by branch rulesets, so this should be unaffected. + +--- + +## Done criteria + +- [ ] All 8 tasks complete +- [ ] PR merged to main +- [ ] Ruleset imported (Step 2 of Task 8) +- [ ] OSPO Hardening dashboard shows Control 5 = Pass on next refresh +- [ ] (Project owner follow-up) Admin demotion draft acted on per-account diff --git a/docs/superpowers/specs/2026-06-01-ospo-hardening-design.md b/docs/superpowers/specs/2026-06-01-ospo-hardening-design.md new file mode 100644 index 0000000..e637430 --- /dev/null +++ b/docs/superpowers/specs/2026-06-01-ospo-hardening-design.md @@ -0,0 +1,210 @@ +# OSPO Hardening Updates — Design + +**Date:** 2026-06-01 +**Status:** Draft (pending spec review) +**Triggered by:** SAP OSPO Hardening Controls report, Controls 5 + 7 + +## Background + +OSPO's Hardening Controls dashboard flagged two findings on `SAP-samples/sap-devs-cli`: + +| ID | Control | Status | Description | +|----|---------|--------|-------------| +| 5 | Restrict main / release branches with Branch Restrictions | ❌ Action Required | Main/release branches should be protected with branch protection rules or rulesets. | +| 7 | Access Restrictions | ⚠️ Warning | Repository access should follow least privilege with limited admin count. | + +The repo *has* a ruleset on `main` ([.github/rulesets/main-protection.json](../../.github/rulesets/main-protection.json)), but it grants `bypass_mode: "always"` to the Admin role. OSPO's checker treats "admins always bypass" as effectively unprotected — anyone with admin can push directly to `main`, skipping the PR + status-check gates documented in [CLAUDE.md](../../CLAUDE.md) under "OSPO compliance." + +A separate audit shows the repo carries **25 admin collaborators**, well above OSPO's least-privilege threshold of 3–5. The two findings are coupled: while admins can bypass the ruleset, the size of the admin set determines how many people can effectively rewrite `main`. + +## Goals + +- **G1.** Move Control 5 from "Action Required" to "Pass" by removing admin bypass on the `main` ruleset. +- **G2.** Keep the existing automation (release pipeline, scheduled news-sync) working without granting blanket bypass. +- **G3.** Provide the project owner a per-account demotion plan for Control 7 — without executing demotions in this change. +- **G4.** Document the rollback path so any unintended breakage is one revert away. + +## Non-goals + +- Demoting any admin in this PR. The drafted list is for the project owner to act on (via OSPO ticket or org-admin tooling). +- Tightening any rule beyond what Control 5 demands (no `required_signatures`, no `required_linear_history`, no review-thread-resolution requirement). Each adds friction without addressing the flagged finding. +- Touching the release pipeline. It runs as `workflow_dispatch` and pushes a *tag* (not to `main`), which is unaffected by branch rulesets. + +## Design + +### Change 1 — Ruleset bypass scope + +Edit [.github/rulesets/main-protection.json](../../.github/rulesets/main-protection.json): + +```diff + "bypass_actors": [ +- {"actor_id": 5, "actor_type": "RepositoryRole", "bypass_mode": "always"} ++ {"actor_id": 15368, "actor_type": "Integration", "bypass_mode": "pull_request"} + ] +``` + +| Field | Value | Meaning | +|-------|-------|---------| +| `actor_id: 15368` | GitHub Actions integration ID | The well-known, stable ID for the `github-actions[bot]` | +| `actor_type: "Integration"` | GitHub App / integration class | Distinct from `RepositoryRole` (humans by role) | +| `bypass_mode: "pull_request"` | Narrow bypass | Bot can merge a PR that doesn't satisfy rules; **cannot** push directly to `main` | + +After this change: + +- No human, including admins, can `git push origin main`. Every change goes through a PR with ≥1 approver and a passing `test` status check. +- `github-actions[bot]` PRs can self-merge without an approver, but the `test` status check still has to pass (it is a `required_status_checks` rule, not a bypass-eligible one). +- Direct push to `main` from any actor is impossible — the integration's bypass is scoped to PR-merge, not push. + +The ruleset JSON in this repo is a **declarative source of truth**, not auto-applied. The owner (or an org admin) imports it via the GitHub UI ("Import a ruleset") or the API. Updating the file is the engineering deliverable; applying it is an operational step. + +### Change 2 — news-sync workflow refactor + +[.github/workflows/news-sync.yml](../../.github/workflows/news-sync.yml) currently does `git commit && git push` directly to `main` after fetching the news episode index. With Change 1 applied, this fails: the bot's bypass is `"pull_request"` only, not `"always"`. + +**Operational prerequisite:** the repository setting **Settings → General → Pull Requests → Allow auto-merge** must be enabled. `gh pr merge --auto` is a no-op (errors out) if this setting is off. This is a one-time toggle, applied alongside the ruleset import. + +**Workflow-level changes:** + +1. Expand the top-level `permissions:` block so `gh pr create` / `gh pr merge` can call the PRs API: + + ```yaml + permissions: + contents: write + pull-requests: write + ``` + +2. Add a `concurrency:` block to serialize overlapping runs. Without this, a manual `workflow_dispatch` triggered while the cron run is mid-flight will force-push to the same bot branch and invalidate the in-flight auto-merge queue: + + ```yaml + concurrency: + group: news-sync + cancel-in-progress: false + ``` + +3. Replace the final commit-and-push step with a PR-based flow using only the built-in `gh` CLI (no third-party action introduced): + + ```yaml + - name: Open or update PR if changed + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + run: | + git config user.name "github-actions[bot]" + git config user.email "github-actions[bot]@users.noreply.github.com" + if git diff --quiet content/packs/base/news-episodes.json 2>/dev/null; then + echo "No changes." + exit 0 + fi + BRANCH="bot/news-sync-update" + git checkout -B "$BRANCH" + git add content/packs/base/news-episodes.json + git commit -m "chore: update news episode index" + git push -f origin "$BRANCH" + EXISTING=$(gh pr list --head "$BRANCH" --json number -q '.[0].number // empty' 2>/dev/null || echo "") + if [ -z "$EXISTING" ]; then + gh pr create \ + --title "chore: update news episode index" \ + --body "Automated update of \`content/packs/base/news-episodes.json\`." \ + --base main --head "$BRANCH" + else + echo "PR #$EXISTING already open; new commit pushed to $BRANCH" + fi + gh pr merge --auto --squash "$BRANCH" + ``` + + `gh pr merge` accepts the branch name directly, so no fragile output-parsing of `gh pr create`. Calling `gh pr merge --auto` on every run (whether we just opened the PR or refreshed an existing one) is idempotent — if auto-merge is already queued, it's a no-op. + +Behaviour: + +- A stable bot branch (`bot/news-sync-update`) is force-pushed each run. One PR, refreshed in place, until merged or closed. +- `gh pr merge --auto --squash` queues the merge to fire once the `test` check passes. No human approval required (Change 1's narrow bypass). +- The required `test` status check (defined in [`.github/workflows/ci.yml`](../../.github/workflows/ci.yml)) runs on the `git push -f` event because `ci.yml` triggers on both `push` and `pull_request`. The `required_status_checks` rule matches by check-run name, so the push-triggered `test` run satisfies the gate even though `GITHUB_TOKEN`-authored PRs don't fire `pull_request`-triggered runs. **This design depends on `ci.yml` keeping `push` as a trigger** — a future maintainer who removes it must either re-add it or switch news-sync to a PAT/GitHub App token. +- If a PR is already open against the same branch (e.g., test failure left it open), the next run just refreshes the branch and re-arms auto-merge. +- The cron cadence stays at 2x/day. Net user-visible change: the news index updates land via reviewable PR commits instead of opaque direct pushes. + +### Change 3 — Admin demotion draft (Control 7) + +The current admin roster (25 accounts) classified for the project owner: + +| Bucket | Logins | Count | Recommendation | +|--------|--------|-------|----------------| +| **Project leads — keep admin** | `qmacro`, `jung-thomas`, `ajmaradiaga` | 3 | Keep | +| **Org-mandated — cannot demote** | `SAP-OSPO-ADMIN`, `sap-ospo-bot`, `SebastianWolf-SAP`, `nicoschoenteich`, `christianneu`, `dellagustin-sap` | 6 | Verify with OSPO; out of repo-owner reach | +| **Inherited / unclear — propose demote to `maintain`** | `akula86`, `ihrigb`, `vipinvkmenon`, `Sygyzmundovych`, `KevinRiedelsheimer`, `rbrainey`, `thecodester`, `ajinkyapatil8190`, `Shegox`, `rich-heilman`, `noravth`, `sheenamk`, `PoojaGidaveer`, `btbernard`, `ajaysoreng`, `neelamegams` | 16 | Demote unless confirmed active maintainer | + +If the bottom bucket is fully demoted, the admin count drops from 25 to ~9 (3 leads + 6 org-mandated). If the org-mandated set can also be pruned via OSPO, the count approaches the 3–5 threshold OSPO checks for. + +**No demotions execute as part of this change.** The list is for the project owner to confirm per-account and act on via the GitHub UI or `gh api`. + +## Data flow + +```text +┌─────────────────┐ +│ Cron 2x/day │ +└────────┬────────┘ + ▼ +┌─────────────────┐ +│ news-sync │ +│ workflow │ +└────────┬────────┘ + │ git push -f origin bot/news-sync-update (allowed — feature branch) + │ gh pr create --base main + ▼ +┌─────────────────────────────────┐ +│ PR (bot-authored, auto-merge) │ +│ ─ test status check runs │ +│ ─ no human approval needed │ +│ (bypass: Integration 15368) │ +└────────┬────────────────────────┘ + │ checks pass + gh pr merge --auto + ▼ +┌─────────────────┐ +│ main updated │ +└─────────────────┘ +``` + +Human-authored PRs follow the same path but without the bypass — they need 1 approving review and a passing `test` check. + +## Error handling & rollback + +**Deployment ordering matters.** The two repo changes must land in this order: + +1. Merge the workflow refactor (Change 2) to `main`. +2. Enable **Settings → General → Pull Requests → Allow auto-merge**. +3. Import the new ruleset (Change 1) via the GitHub UI or `gh api`. + +If steps are reversed (ruleset imported before the workflow lands), the next scheduled news-sync run fails with a ruleset violation on `git push origin main`. No data loss — just a failed run until the workflow change merges. + +**This PR is the last admin-bypassable merge before the new posture takes effect.** The very PR that introduces these changes cannot itself benefit from the bot's narrow PR-merge bypass (the bypass is only in force after the ruleset is imported). It must merge under the *current* admin-bypass ruleset, which is fine. + +| Scenario | Handling | +|---|---| +| Ruleset breaks an unforeseen workflow | `git revert` the ruleset commit and re-import via the GitHub UI / `gh api`. The old (admin-bypass) state is one revert away. | +| news-sync PR can't auto-merge (test fails on the bot's PR) | PR stays open; next cron run force-pushes to the same branch, refreshing the diff. A human can intervene. No data loss. | +| `bot/news-sync-update` branch grows stale during a long test outage | Force-push semantics keep the branch current; the PR auto-updates. | +| Two news-sync runs overlap | The `concurrency: news-sync` block serializes them — second run waits for the first to finish. | +| GitHub Actions Integration ID changes (~unprecedented) | Update `actor_id` in the ruleset. | +| Release pipeline regression | Unaffected — release pushes a tag, not to `main`. Tags are not gated by branch rulesets. | + +## Testing plan + +Verification is empirical (the ruleset only takes effect server-side after import): + +1. **Direct push blocked** — apply the new ruleset, attempt `git push origin main` from a maintainer clone. Must fail with a ruleset violation. +2. **PR approval enforced** — open a PR with no approvers; the GitHub UI "Merge" button must be greyed out for humans. +3. **Bot auto-merge works** — `gh workflow run news-sync.yml` (or wait for the next cron). Verify: PR opens, `test` check runs, PR auto-merges, `main` reflects the update. +4. **Stale-PR behaviour** — manually fail the `test` check on a bot PR (e.g., temporarily push a broken commit to the same branch); confirm the PR stays open and the next run refreshes it. +5. **Release pipeline unchanged** — `gh workflow run release.yml -f version=` on a test version. Tag pushes successfully; release artifacts upload. + +## Open questions + +None blocking. Items the project owner needs to follow up on after this change merges: + +- File an OSPO ticket to prune the org-mandated admin bucket if least-privilege thresholds need to be met. +- Decide per-account on the 16 "Inherited / unclear" admins listed above. + +## Files touched + +- [.github/rulesets/main-protection.json](../../.github/rulesets/main-protection.json) — bypass actor swap +- [.github/workflows/news-sync.yml](../../.github/workflows/news-sync.yml) — direct push → PR with auto-merge +- [docs/superpowers/specs/2026-06-01-ospo-hardening-design.md](2026-06-01-ospo-hardening-design.md) — this spec (new file) +- [CLAUDE.md](../../CLAUDE.md) — update "OSPO compliance" paragraph to reflect the new posture (admins no longer bypass; bot has narrow PR-merge bypass)