From cc8e345ef913b31e054b9f955decd91da4c673cb Mon Sep 17 00:00:00 2001 From: Marcus Burghardt Date: Wed, 20 May 2026 12:10:01 +0200 Subject: [PATCH 1/5] docs: add change proposal for safe-settings adoption Define proposal, design, specs, and tasks for adopting github/safe-settings as a complementary tool to peribolos. safe-settings will manage repository settings (auto-merge, delete-branch-on-merge), branch protection rules, rulesets, and security configurations via GitOps. Peribolos continues to own org membership, team creation, and team-to-repo permissions. Signed-off-by: Marcus Burghardt Assisted-by: OpenCode (claude-opus-4-6) --- .../adopt-safe-settings/.openspec.yaml | 2 + .../changes/adopt-safe-settings/design.md | 270 ++++++++++++++++++ .../changes/adopt-safe-settings/proposal.md | 93 ++++++ .../specs/local-development-workflow/spec.md | 78 +++++ .../specs/maintainer-documentation/spec.md | 96 +++++++ .../specs/org-rulesets-as-code/spec.md | 91 ++++++ .../specs/org-wide-repo-settings/spec.md | 112 ++++++++ .../specs/safe-settings-deployment/spec.md | 127 ++++++++ .../specs/settings-override-policy/spec.md | 56 ++++ .../specs/tool-boundary-enforcement/spec.md | 101 +++++++ openspec/changes/adopt-safe-settings/tasks.md | 57 ++++ 11 files changed, 1083 insertions(+) create mode 100644 openspec/changes/adopt-safe-settings/.openspec.yaml create mode 100644 openspec/changes/adopt-safe-settings/design.md create mode 100644 openspec/changes/adopt-safe-settings/proposal.md create mode 100644 openspec/changes/adopt-safe-settings/specs/local-development-workflow/spec.md create mode 100644 openspec/changes/adopt-safe-settings/specs/maintainer-documentation/spec.md create mode 100644 openspec/changes/adopt-safe-settings/specs/org-rulesets-as-code/spec.md create mode 100644 openspec/changes/adopt-safe-settings/specs/org-wide-repo-settings/spec.md create mode 100644 openspec/changes/adopt-safe-settings/specs/safe-settings-deployment/spec.md create mode 100644 openspec/changes/adopt-safe-settings/specs/settings-override-policy/spec.md create mode 100644 openspec/changes/adopt-safe-settings/specs/tool-boundary-enforcement/spec.md create mode 100644 openspec/changes/adopt-safe-settings/tasks.md diff --git a/openspec/changes/adopt-safe-settings/.openspec.yaml b/openspec/changes/adopt-safe-settings/.openspec.yaml new file mode 100644 index 0000000..8b76914 --- /dev/null +++ b/openspec/changes/adopt-safe-settings/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-05-20 diff --git a/openspec/changes/adopt-safe-settings/design.md b/openspec/changes/adopt-safe-settings/design.md new file mode 100644 index 0000000..eab94fb --- /dev/null +++ b/openspec/changes/adopt-safe-settings/design.md @@ -0,0 +1,270 @@ +## Context + +The complytime GitHub organization uses peribolos for declarative org management +(membership, teams, team-repo permissions) via `peribolos.yaml` in the `.github` +repo. However, repository-level settings (branch protection, rulesets, +auto-merge, security configurations) are managed manually through the GitHub UI. +This creates configuration drift, inconsistent security posture across repos, +and no audit trail for changes. + +A security review of the `.github` repo's access model (conducted during the +`restructure-teams-and-codeowners` change) identified that branch protection +rules and rulesets are critical security controls that should be managed as code, +with the same PR-review-and-apply workflow used for peribolos. + +safe-settings (github/safe-settings, ~870 stars, ISC license) is a GitHub-built +policy-as-code tool that manages repository settings, branch protections, +rulesets, and more from a centralized YAML config. It complements peribolos by +covering the areas peribolos does not manage. + +## Goals / Non-Goals + +**Goals:** +- Manage repository settings (auto-merge, delete-branch-on-merge, merge + strategies, security settings) as code across all complytime repos +- Manage branch protection rules and rulesets as code, replacing UI-managed + rules +- Prevent configuration drift via scheduled GitOps convergence +- Enforce a clear tool boundary between peribolos and safe-settings with + automated validation guardrails +- Enable local testing and dry-run workflows mirroring the peribolos pattern +- Prevent weakening of org-level protections through override validators +- Maintain clean tool boundary: peribolos = people/teams, + safe-settings = repo/branch settings + +**Non-Goals:** +- Replacing peribolos for org membership or team management +- Managing team-to-repo permissions via safe-settings (peribolos owns this) +- Managing repository descriptions or default_branch via safe-settings + (peribolos owns these via its repo definitions) +- Achieving full coverage on day one — start with the most critical settings + (rulesets, auto-merge, delete-branch-on-merge) and expand incrementally + +## Decisions + +### 1. Separate GitHub App for safe-settings + +Register a dedicated GitHub App (`safe-settings-bot` or similar) rather than +extending the existing `complytime-bot` used by peribolos. + +**Rationale:** Peribolos requires `Organization: admin` and `Members: read/write` +— the most privileged GitHub API permissions. safe-settings does NOT need those. +Separate apps isolate blast radius: if one app's private key leaks, the attacker +can only affect that app's scope. Each app gets only the permissions it needs +(principle of least privilege). + +**Permissions for safe-settings app:** +- Repository: Administration (write) — for managing repo settings, branch + protection, rulesets +- Repository: Contents (read) — for reading config files from admin repo +- Repository: Checks (write) — for PR dry-run validation results +- Repository: Pull requests (write) — for PR comments with change previews +- Organization: Administration (read) — for reading org-level rulesets + +**Alternative considered:** Extend complytime-bot. Rejected because it would +give the safe-settings workflow access to org admin permissions it does not need, +and a single key compromise would affect both org membership and repo settings. + +### 2. GitHub Actions-only deployment + +Deploy safe-settings via a GitHub Actions workflow that runs `npm run full-sync` +on three triggers: +- `push` to main — immediate convergence after config changes are merged +- `schedule` daily at 06:00 UTC — drift correction (30 min after peribolos + sync at 05:30 UTC, ensuring membership/teams are applied before repo settings) +- `workflow_dispatch` — manual convergence on demand + +No webhook listener, no hosting infrastructure, no public endpoint. + +**Rationale:** The complytime org has ~12 repos, 3 admins, and ~24 members. +This is a small org where eventual consistency (daily sync) is acceptable. +The `push`-triggered sync provides near-immediate convergence after config +merges. This model matches the existing peribolos operational pattern +(scheduled + push + manual dispatch), requires zero infrastructure, and +eliminates the attack surface of a public webhook endpoint. + +**Alternative considered:** Webhook-driven deployment (Docker/Lambda) for +real-time drift prevention and PR dry-run validation. Rejected because it +adds infrastructure hosting, monitoring, additional secrets (WEBHOOK_SECRET, +CLIENT_ID, CLIENT_SECRET), and operational burden disproportionate to the +org's size. Can be adopted later if the org grows significantly or real-time +enforcement becomes a hard requirement. + +### 3. Config in `.github` repo with CONFIG_PATH override + +Store safe-settings config in the existing `.github` repo under a +`safe-settings/` directory at the repository root, using `ADMIN_REPO=.github` +and `CONFIG_PATH=safe-settings`. + +**Rationale:** Centralizes all org management config in one repo. The `.github` +repo already has admin-only CODEOWNERS, required status checks, required +signatures, and no bypass actors — the same security model applies to +safe-settings config files automatically. + +**Directory structure:** +``` +/ +├── peribolos.yaml # peribolos (existing, at root) +├── safe-settings/ # NEW: safe-settings config +│ ├── settings.yml # org-wide repo defaults +│ ├── deployment-settings.yml # runtime config (restricted repos) +│ ├── suborgs/ +│ │ ├── code-repos.yml # complyctl, providers, policies, etc. +│ │ └── non-code-repos.yml # community, website, demos +│ └── repos/ # per-repo overrides (only if needed) +│ └── .gitkeep +├── .github/ +│ ├── CODEOWNERS +│ └── workflows/ +│ ├── apply_peribolos.yml # existing +│ └── safe_settings_sync.yml # NEW: primary sync workflow +``` + +**Alternative considered:** Dedicated `admin` repo. Rejected because it splits +the org management config across two repos with separate access controls and +review workflows. Keeping everything in `.github` simplifies governance. + +### 4. Suborg groupings: code repos vs non-code repos + +Define two suborgs to apply different policies: +- `code-repos`: complyctl, complytime-providers, complytime-policies, + complyscribe, complytime-collector-components, gemara-content-service, + org-infra — strict branch protection, required reviews, required signatures +- `non-code-repos`: community, complytime-demos, website, complytime — lighter + protection, fewer required checks + +**Rationale:** Code repos need strict security controls (signed commits, +required reviews, status checks). Non-code repos benefit from auto-merge and +simpler review processes. The suborg model allows org-wide defaults with +group-specific overrides without per-repo config files. + +### 5. `.github` repo ruleset remains manually managed + +The `.github` repo currently has a manually-created ruleset named "verifiy" +(sic) managed via the GitHub UI. Since the `.github` repo is excluded from +safe-settings management (decision #7), this ruleset stays manually managed. + +The typo SHALL be corrected from "verifiy" to "verify" via the GitHub UI as a +one-time fix. This is a deliberate exception: the admin repo's own ruleset +should not be subject to circular self-management by a tool it hosts. + +### 6. Override validators for security floor + +Configure `overridevalidators` in `deployment-settings.yml` to prevent suborg +or repo level settings from weakening org-level protections. Specifically: +- Required approving review count cannot be lowered below the org default +- Branch protection cannot be disabled at repo level +- Required signatures cannot be removed + +**Rationale:** Without validators, any repo-level config file could override +org-level branch protection to zero approvers. Validators enforce a security +floor that cannot be weakened by lower-level config. + +### 7. Exclude `.github` and `admin` repos from safe-settings management + +Configure `deployment-settings.yml` to exclude the `.github` repo itself from +safe-settings management. The `.github` repo's settings and rulesets are +managed manually via the GitHub UI. safe-settings should not manage its own +admin repo's settings to avoid circular dependency issues. + +### 8. Tool boundary enforcement with automated guardrails + +Define a clear, enforceable boundary between peribolos and safe-settings: + +**Peribolos owns** (via `peribolos.yaml`): +- Org membership (admins, members) +- Team creation, membership, privacy, and descriptions +- Team-to-repo permission mappings +- Per-repo: `description`, `has_projects`, `default_branch` + +**Safe-settings owns** (via `safe-settings/`): +- Per-repo: `allow_auto_merge`, `delete_branch_on_merge`, `allow_squash_merge`, + `allow_merge_commit`, `allow_rebase_merge`, `allow_update_branch`, + `has_wiki`, `has_issues` +- Security settings (vulnerability alerts, automated fixes) +- Branch protection rules +- Organization-level and repo-level rulesets +- Labels, milestones, autolinks + +**Neither tool manages**: `homepage`, `topics`, `visibility`/`private` +(set at repo creation time, rarely changed). + +**Enforcement:** Go tests in `config/boundary_test.go` validate: +1. All repos listed in suborg files exist in `peribolos.yaml` +2. No repo appears in multiple suborg files +3. Safe-settings config does not set fields owned by peribolos + (`description`, `has_projects`, `default_branch`) +4. Peribolos config does not set fields owned by safe-settings + (`has_wiki`, `allow_auto_merge`, `delete_branch_on_merge`, etc.) + +These tests run on every PR via CI as guardrails against accidental overlap. + +**Rationale:** Both tools can technically manage overlapping repository fields. +Without enforcement, divergent settings cause a flapping loop where each tool +reverts the other's changes. Automated tests prevent this at PR review time. + +### 9. Local development workflow + +Provide Makefile targets for safe-settings that mirror the existing peribolos +local testing pattern: +- `make safe-settings-validate` — yamllint on all safe-settings YAML files +- `make safe-settings-dryrun` — run `npm run full-sync` in dry-run mode against + the live org (requires Node.js and a GitHub token) +- `make ensure-safe-settings` — clone safe-settings and install dependencies + +**Rationale:** Maintainers must be able to validate and test config changes +locally before pushing. The peribolos Makefile targets (`peribolos-dryrun`, +`peribolos-apply`) have proven effective and should be replicated. + +### 10. Maintainer documentation + +Create a `MAINTAINING.md` at the repository root covering both peribolos and +safe-settings workflows: +- Tool boundary and field ownership table +- How to add/remove org members (peribolos) +- How to add a new repo to safe-settings management +- How to change branch protection rules or rulesets +- How to add repo-specific overrides +- How to run local dry-runs for both tools +- How to debug when settings are not applied +- Override validator policies and how to request exceptions + +Link from `README.md` to `MAINTAINING.md`. Keep `README.md` focused on +project overview and prerequisites. + +## Risks / Trade-offs + +**[Eventual consistency]** The GHA-only deployment provides daily convergence, +not real-time enforcement. Manual UI changes to repo settings persist until +the next scheduled sync or a manual `workflow_dispatch` trigger. +-> Mitigation: The `push` trigger on main provides near-immediate convergence +after config merges. Daily scheduled sync catches drift from manual UI changes. +Acceptable for a small org (~12 repos) where admins are disciplined about +config-as-code workflows. + +**[Two tools for org management]** Maintaining peribolos AND safe-settings +increases operational complexity. Two config schemas, two deployment pipelines, +two sets of secrets. +-> Mitigation: Clean boundary (people vs settings) enforced by automated Go +tests and CI guardrails. Both tools are YAML-based GitOps. `MAINTAINING.md` +documents the boundary. The alternative (Terraform) is a single tool but +with significantly higher operational overhead (state management). + +**[safe-settings maturity]** Open issues on the repo. Some edge cases may +require workarounds. +-> Mitigation: Start with well-documented features (rulesets, repo settings). +Avoid less-tested features initially. The project is actively maintained by +GitHub staff. + +**[Node.js dependency]** Introduces a Node.js runtime dependency into an +otherwise Go-based ecosystem. +-> Mitigation: Contained to the GHA runner and local development. Does not +affect application code. Local testing requires `node` and `npm`. + +**[Overlap with peribolos]** Both tools can manage overlapping repository +fields (description, has_projects, has_wiki, merge strategies). Running both +without boundary enforcement causes flapping. +-> Mitigation: Field ownership defined in decision #8. Automated Go tests in +`config/boundary_test.go` validate no overlap exists. CI runs these tests on +every PR as guardrails. `MAINTAINING.md` documents which tool owns which +fields. diff --git a/openspec/changes/adopt-safe-settings/proposal.md b/openspec/changes/adopt-safe-settings/proposal.md new file mode 100644 index 0000000..44bc6e8 --- /dev/null +++ b/openspec/changes/adopt-safe-settings/proposal.md @@ -0,0 +1,93 @@ +## Why + +Peribolos manages org membership, teams, and team-repo permissions effectively, +but it cannot manage repository-level settings such as branch protection rules, +rulesets, auto-merge, auto-delete merged branches, or security configurations. +These settings are currently managed manually through the GitHub UI, leading to +configuration drift, inconsistency across repositories, and no audit trail for +changes. The complytime organization needs a centralized, GitOps-driven solution +to declare and enforce repository settings alongside the existing peribolos-based +org management. + +## What Changes + +- **Adopt github/safe-settings** as the policy-as-code tool for repository and + org-level settings, complementing peribolos (which continues to manage org + membership, team creation, and team membership). +- **Register a dedicated GitHub App** for safe-settings, separate from the + existing complytime-bot used by peribolos, following the principle of least + privilege and blast radius isolation. +- **Deploy safe-settings via GitHub Actions** with push-triggered sync, + daily scheduled drift correction, and manual dispatch — no webhook + infrastructure required. +- **Create centralized config** in the `.github` repo with org-wide defaults, + suborg groupings (code repos vs non-code repos), and per-repo overrides where + needed. +- **Define org-level rulesets** via safe-settings config for code and non-code + repos. The `.github` repo's "verifiy" ruleset remains manually managed + (renamed to "verify" via UI). +- **Define org-wide repository settings** including `allow_auto_merge: true`, + `delete_branch_on_merge: true`, vulnerability alerts, and merge strategy + defaults. +- **Configure override validators** to prevent repo-level or suborg-level + overrides from weakening org-level protections (e.g., lowering required + approver count). +- **Protect safe-settings config** via CODEOWNERS (admin-only approval) and + the existing repository ruleset on the `.github` repo. + +## Capabilities + +### New Capabilities + +- `safe-settings-deployment`: GitHub App registration and GitHub Actions sync + workflow. Covers the operational foundation for running safe-settings. +- `org-wide-repo-settings`: Organization-wide default repository settings + applied to all repos (auto-merge, delete-branch-on-merge, merge strategies, + vulnerability alerts, wiki/projects toggles). Includes suborg-level groupings + for code repos vs non-code repos. +- `org-rulesets-as-code`: Organization-level and repository-level rulesets + managed as YAML config. Covers branch protection rules, required status + checks, required signatures, PR review requirements, and bypass actor + configuration. +- `settings-override-policy`: Custom validators that prevent suborg or repo + level settings from weakening org-level protections. Covers override + validation rules and enforcement behavior. +- `tool-boundary-enforcement`: Automated Go tests and CI guardrails that + validate no field overlap between peribolos and safe-settings configs. + Ensures suborg repo lists are consistent with peribolos.yaml. +- `local-development-workflow`: Makefile targets for local YAML validation + and dry-run testing, mirroring the existing peribolos local testing pattern. +- `maintainer-documentation`: MAINTAINING.md covering both peribolos and + safe-settings workflows, tool boundary, and troubleshooting. + +### Modified Capabilities + +(none -- no existing specs to modify) + +## Impact + +- **New GitHub App**: A `safe-settings-bot` (or similar) GitHub App registered + in the complytime org with repository admin, contents read, checks write, and + pull requests write permissions. Separate from the existing `complytime-bot`. +- **New secrets**: `SAFE_SETTINGS_APP_ID` (repository variable) and + `SAFE_SETTINGS_PRIVATE_KEY` (repository secret) stored in the `.github` + repo. No additional infrastructure secrets required. +- **No new infrastructure**: Runs entirely on GitHub Actions runners. +- **New config files**: `safe-settings/` directory in the `.github` repo with + `settings.yml`, `deployment-settings.yml`, `suborgs/*.yml`, `repos/*.yml`. +- **CODEOWNERS update**: The `.github/CODEOWNERS` may need path-specific rules + to protect `safe-settings/` config files. +- **Existing ruleset fix**: The "verifiy" ruleset on the `.github` repo is + renamed to "verify" via the GitHub UI. It remains manually managed since the + `.github` repo is excluded from safe-settings. +- **No peribolos impact**: Peribolos continues to manage org membership, team + creation/membership, and team-to-repo permissions unchanged. A clear field + ownership boundary is enforced by automated Go tests to prevent overlap. +- **New Go tests**: `config/boundary_test.go` validates cross-tool consistency + (suborg repos exist in peribolos.yaml, no field overlap, no duplicate suborg + membership). Runs on every PR as a CI guardrail. +- **New documentation**: `MAINTAINING.md` documents both tool workflows, field + ownership, and troubleshooting. `README.md` links to it. +- **Node.js dependency**: safe-settings requires Node.js >= 18. This is + contained to the GHA runner and local development. Does not affect the Go + codebase. diff --git a/openspec/changes/adopt-safe-settings/specs/local-development-workflow/spec.md b/openspec/changes/adopt-safe-settings/specs/local-development-workflow/spec.md new file mode 100644 index 0000000..0ff9446 --- /dev/null +++ b/openspec/changes/adopt-safe-settings/specs/local-development-workflow/spec.md @@ -0,0 +1,78 @@ +## ADDED Requirements + +### Requirement: YAML validation for safe-settings config + +A Makefile target `safe-settings-validate` SHALL validate all YAML files +under `safe-settings/` using `yamllint`. The target SHALL use the same +`.yamllint.yml` configuration used for `peribolos.yaml`. + +#### Scenario: YAML validation catches syntax error + +- **GIVEN** a safe-settings YAML file has a syntax error (e.g., bad + indentation) +- **WHEN** `make safe-settings-validate` is run +- **THEN** yamllint reports the error with file path and line number +- **AND** the command exits with a non-zero status + +#### Scenario: YAML validation passes on correct config + +- **GIVEN** all safe-settings YAML files have valid syntax +- **WHEN** `make safe-settings-validate` is run +- **THEN** yamllint reports no errors +- **AND** the command exits with status 0 + +### Requirement: Local dry-run for safe-settings + +A Makefile target `safe-settings-dryrun` SHALL run safe-settings in dry-run +mode against the live GitHub org. This target SHALL: +- Depend on `ensure-safe-settings` (clone and install if not present) +- Use the Probot-native auth pattern (`APP_ID` + `PRIVATE_KEY` env vars) +- Read credentials from a local file (e.g., + `~/.config/safe-settings/env`) or expect them as environment variables +- Run `npm run full-sync` with `NODE_ENV=dry-run` or equivalent +- Display what changes would be applied without actually applying them + +#### Scenario: Dry-run shows pending changes + +- **GIVEN** the local safe-settings config differs from the live GitHub state +- **WHEN** `make safe-settings-dryrun` is run with valid credentials +- **THEN** the output shows what settings would be changed on which repos +- **AND** no actual changes are applied to the GitHub org + +### Requirement: Ensure safe-settings binary/environment + +A Makefile target `ensure-safe-settings` SHALL clone the safe-settings +repository and install its Node.js dependencies if not already present. +The target SHALL: +- Clone `github/safe-settings` at a pinned version/tag to a temporary or + cached directory +- Run `npm install` in the cloned directory +- Be idempotent (skip clone and install if already present) +- Mirror the pattern used by `ensure-peribolos` for peribolos + +#### Scenario: First run clones and installs + +- **GIVEN** safe-settings has not been cloned locally +- **WHEN** `make ensure-safe-settings` is run +- **THEN** the safe-settings repo is cloned at the pinned version +- **AND** `npm install` completes successfully + +#### Scenario: Subsequent runs are no-ops + +- **GIVEN** safe-settings is already cloned and installed +- **WHEN** `make ensure-safe-settings` is run +- **THEN** the target prints a message indicating it is already present +- **AND** does not re-clone or re-install + +### Requirement: Lint target covers safe-settings YAML + +The existing `make lint` target SHALL be extended (or a new combined target +created) to validate both `peribolos.yaml` and `safe-settings/**/*.yml` +files. This ensures all org management YAML is linted consistently. + +#### Scenario: Lint catches error in safe-settings file + +- **GIVEN** a safe-settings YAML file has a lint violation +- **WHEN** `make lint` is run +- **THEN** yamllint reports the error +- **AND** the command exits with a non-zero status diff --git a/openspec/changes/adopt-safe-settings/specs/maintainer-documentation/spec.md b/openspec/changes/adopt-safe-settings/specs/maintainer-documentation/spec.md new file mode 100644 index 0000000..aa152ee --- /dev/null +++ b/openspec/changes/adopt-safe-settings/specs/maintainer-documentation/spec.md @@ -0,0 +1,96 @@ +## ADDED Requirements + +### Requirement: MAINTAINING.md at repository root + +A `MAINTAINING.md` file SHALL be created at the repository root covering +the operational workflows for both peribolos and safe-settings. This file +serves as the single reference for maintainers managing the complytime +GitHub organization. + +#### Scenario: Maintainer finds workflow documentation + +- **GIVEN** a new maintainer needs to understand how org management works +- **WHEN** they read `MAINTAINING.md` +- **THEN** they find clear instructions for both peribolos and safe-settings + workflows, including local testing, CI behavior, and troubleshooting + +### Requirement: Tool boundary documented + +`MAINTAINING.md` SHALL include a table or section defining which tool owns +which configuration areas: +- Peribolos: org membership, teams, team-repo permissions, repo description, + has_projects, default_branch +- Safe-settings: repo settings (auto-merge, delete-branch-on-merge, merge + strategies, wiki, issues), security settings, branch protection, rulesets +- Manually managed: `.github` repo's own ruleset + +The documentation SHALL explain why two tools are used and what happens if +their configurations overlap. + +#### Scenario: Maintainer knows where to make a change + +- **GIVEN** a maintainer wants to change branch protection rules +- **WHEN** they consult the tool boundary section of `MAINTAINING.md` +- **THEN** they find that branch protection is owned by safe-settings +- **AND** they know to edit files under `safe-settings/` + +### Requirement: Common workflows documented + +`MAINTAINING.md` SHALL include step-by-step instructions for these workflows: +- Adding or removing an org member (peribolos) +- Creating a new team or changing team membership (peribolos) +- Adding a new repository to safe-settings management (suborg file edit) +- Changing branch protection rules or rulesets (safe-settings) +- Adding a repo-specific override (when and how to create `repos/.yml`) +- Running local dry-runs for both tools +- Triggering manual sync via `workflow_dispatch` + +#### Scenario: Maintainer adds a new repo to safe-settings + +- **GIVEN** a new repo `new-tool` has been created in the complytime org +- **WHEN** the maintainer follows the documented workflow in `MAINTAINING.md` +- **THEN** they know to: + 1. Add the repo to `peribolos.yaml` (description, default_branch, etc.) + 2. Add the repo to the appropriate suborg file (code-repos or non-code-repos) + 3. Submit a PR and wait for CI validation + 4. Merge to trigger automatic sync + +### Requirement: Override validator policies documented + +`MAINTAINING.md` SHALL document what override validators are configured, +what they prevent (e.g., lowering required approver count), and how to +request an exception if a legitimate use case arises. + +#### Scenario: Maintainer needs a policy exception + +- **GIVEN** a repo needs a temporary exception to the minimum approver count +- **WHEN** the maintainer reads the override validator documentation +- **THEN** they find the process for requesting an exception +- **AND** they understand that exceptions require admin approval + +### Requirement: Troubleshooting section + +`MAINTAINING.md` SHALL include a troubleshooting section covering: +- Settings not applied after merge (check workflow run logs) +- Drift detected but not corrected (trigger manual `workflow_dispatch`) +- Boundary test failures (how to identify and fix field overlap) +- Safe-settings sync errors (common causes and remediation) + +#### Scenario: Maintainer debugs a failed sync + +- **GIVEN** the safe-settings sync workflow failed +- **WHEN** the maintainer consults the troubleshooting section +- **THEN** they find guidance on checking workflow logs, common error + patterns, and how to re-trigger the sync + +### Requirement: README.md links to MAINTAINING.md + +The existing `README.md` SHALL be updated to include a link to +`MAINTAINING.md` for maintainer-specific documentation. `README.md` remains +focused on project overview and prerequisites. + +#### Scenario: README directs maintainers to detailed docs + +- **GIVEN** a user reads `README.md` +- **WHEN** they look for maintainer workflows +- **THEN** they find a link to `MAINTAINING.md` diff --git a/openspec/changes/adopt-safe-settings/specs/org-rulesets-as-code/spec.md b/openspec/changes/adopt-safe-settings/specs/org-rulesets-as-code/spec.md new file mode 100644 index 0000000..ef60e69 --- /dev/null +++ b/openspec/changes/adopt-safe-settings/specs/org-rulesets-as-code/spec.md @@ -0,0 +1,91 @@ +## ADDED Requirements + +### Requirement: Default branch ruleset for code repos + +An org-level ruleset SHALL be defined in `settings.yml` that applies to the +default branch of all code repositories. The ruleset SHALL enforce: + +- `type: deletion` — prevent branch deletion +- `type: non_fast_forward` — prevent force pushes +- `type: required_signatures` — require signed commits +- `type: pull_request` with parameters: + - `dismiss_stale_reviews_on_push: true` + - `require_code_owner_review: true` + - `require_last_push_approval: true` + - `required_approving_review_count: 1` + - `required_review_thread_resolution: true` + +The ruleset SHALL target `~DEFAULT_BRANCH` and SHALL include code repos via +`repository_name` conditions. + +Bypass actors SHALL be limited to `OrganizationAdmin` with `bypass_mode: always`. + +#### Scenario: Code repo default branch protected + +- **GIVEN** the org-level ruleset is defined in safe-settings config +- **WHEN** safe-settings applies the ruleset +- **THEN** all code repos have the default branch protected with signed + commits, required reviews, and no force pushes + +#### Scenario: Non-admin cannot bypass rules + +- **GIVEN** the ruleset bypass_actors only includes OrganizationAdmin +- **WHEN** a non-admin user attempts to force push to the default branch +- **THEN** the push is rejected + +### Requirement: Lighter ruleset for non-code repos + +A separate ruleset SHALL be defined for non-code repositories with lighter +requirements. This ruleset SHALL enforce: + +- `type: deletion` — prevent branch deletion +- `type: non_fast_forward` — prevent force pushes +- `type: pull_request` with parameters: + - `required_approving_review_count: 1` + - `required_review_thread_resolution: true` + +Required signatures and code owner review MAY be omitted for non-code repos +at the admin's discretion. + +#### Scenario: Non-code repo has lighter protection + +- **GIVEN** the non-code repo ruleset is defined +- **WHEN** safe-settings applies the ruleset to `community` +- **THEN** the repo requires a PR with 1 approval but does not require + signed commits or code owner review + +### Requirement: Fix existing "verifiy" ruleset on .github repo + +The manually-created "verifiy" ruleset on the `.github` repo SHALL be renamed +to "verify" via the GitHub UI as a one-time manual fix. This ruleset remains +manually managed because the `.github` repo is excluded from safe-settings +management via `deployment-settings.yml` to avoid circular dependency. + +The `.github` repo's ruleset is a deliberate exception to config-as-code +management. Its configuration (required reviews, stale review dismissal, +last push approval, required status checks, signatures) SHALL be preserved +during the rename. + +#### Scenario: Ruleset renamed without parameter changes + +- **GIVEN** the `.github` repo has a ruleset named "verifiy" +- **WHEN** an admin renames it to "verify" via the GitHub UI +- **THEN** all existing rule parameters remain unchanged +- **AND** the ruleset continues to protect the default branch + +### Requirement: Required status checks configurable per repo + +Rulesets SHALL support per-repo required status checks via the suborg or repo +level config. For example, `complyctl` may require different CI checks than +`complytime-providers`. + +Status checks that are defined outside of safe-settings SHALL use the +`{{EXTERNALLY_DEFINED}}` placeholder to preserve existing checks configured +via the GitHub UI or other tools. + +#### Scenario: Repo-specific status checks preserved + +- **GIVEN** a repo has status checks configured outside of safe-settings +- **AND** the safe-settings config uses `{{EXTERNALLY_DEFINED}}` +- **WHEN** safe-settings applies the ruleset +- **THEN** the existing status checks are preserved and not overwritten diff --git a/openspec/changes/adopt-safe-settings/specs/org-wide-repo-settings/spec.md b/openspec/changes/adopt-safe-settings/specs/org-wide-repo-settings/spec.md new file mode 100644 index 0000000..72c8b8e --- /dev/null +++ b/openspec/changes/adopt-safe-settings/specs/org-wide-repo-settings/spec.md @@ -0,0 +1,112 @@ +## ADDED Requirements + +### Requirement: Auto-merge enabled org-wide + +The org-wide `settings.yml` SHALL set `allow_auto_merge: true` for all +repositories (unless overridden at suborg or repo level). + +#### Scenario: New repo inherits auto-merge setting + +- **GIVEN** the org-wide settings define `allow_auto_merge: true` +- **WHEN** a new repository is created in the complytime org +- **THEN** safe-settings applies `allow_auto_merge: true` to the repo + +### Requirement: Auto-delete merged branches enabled org-wide + +The org-wide `settings.yml` SHALL set `delete_branch_on_merge: true` for all +repositories (unless overridden at suborg or repo level). + +#### Scenario: Merged branch auto-deleted + +- **GIVEN** the org-wide settings define `delete_branch_on_merge: true` +- **WHEN** a pull request is merged in any managed repository +- **THEN** the source branch is automatically deleted + +### Requirement: Merge strategies defined org-wide + +The org-wide `settings.yml` SHALL define the following merge strategy defaults: +- `allow_squash_merge: true` +- `allow_merge_commit: true` +- `allow_rebase_merge: true` +- `allow_update_branch: true` + +#### Scenario: All merge strategies available + +- **GIVEN** the org-wide settings define all merge strategies as enabled +- **WHEN** a repository is managed by safe-settings +- **THEN** all three merge strategies (squash, merge commit, rebase) are + available for pull requests + +### Requirement: Security settings enabled org-wide + +The org-wide `settings.yml` SHALL enable: +- `security.enableVulnerabilityAlerts: true` +- `security.enableAutomatedSecurityFixes: true` + +#### Scenario: Vulnerability alerts enabled on all repos + +- **GIVEN** the org-wide settings enable vulnerability alerts +- **WHEN** safe-settings applies settings to a repository +- **THEN** Dependabot vulnerability alerts are enabled +- **AND** automated security fixes (Dependabot PRs) are enabled + +### Requirement: Wiki disabled org-wide + +The org-wide `settings.yml` SHALL set `has_wiki: false` for all repositories, +since documentation is maintained in dedicated repositories and wiki content +is not version-controlled. + +#### Scenario: Wiki disabled on managed repos + +- **GIVEN** the org-wide settings define `has_wiki: false` +- **WHEN** safe-settings applies settings to a repository +- **THEN** the wiki tab is disabled on that repository + +### Requirement: Suborg grouping for code vs non-code repos + +Two suborg configuration files SHALL be defined: + +1. `code-repos.yml` — applies to repositories containing source code: + complyctl, complytime-providers, complytime-policies, complyscribe, + complytime-collector-components, gemara-content-service, org-infra + +2. `non-code-repos.yml` — applies to repositories without source code: + community, complytime-demos, website, complytime + +Each suborg file SHALL use `suborgrepos` to list the repos in that group. + +#### Scenario: Code repo gets code-specific settings + +- **GIVEN** `code-repos.yml` defines stricter settings for code repos +- **WHEN** safe-settings applies settings to `complyctl` +- **THEN** the code-repo-specific settings are merged on top of org-wide + defaults + +#### Scenario: Non-code repo gets lighter settings + +- **GIVEN** `non-code-repos.yml` defines lighter settings for non-code repos +- **WHEN** safe-settings applies settings to `community` +- **THEN** the non-code-repo-specific settings are merged on top of org-wide + defaults + +### Requirement: Safe-settings SHALL NOT manage peribolos-owned fields + +Safe-settings config files (settings.yml, suborg files, repo override files) +SHALL NOT set the following fields, which are owned by peribolos: +- `description` +- `has_projects` +- `default_branch` + +These fields are managed via `peribolos.yaml` repo definitions. Setting them +in safe-settings config would create a flapping conflict where each tool +reverts the other's changes. + +This boundary is enforced by automated Go tests in +`config/boundary_test.go` (see `tool-boundary-enforcement` spec). + +#### Scenario: Safe-settings config sets a peribolos-owned field + +- **GIVEN** a safe-settings config file sets `description` under `repository` +- **WHEN** the boundary validation tests run +- **THEN** the test fails with an error identifying the field and file +- **AND** the PR cannot merge until the violation is removed diff --git a/openspec/changes/adopt-safe-settings/specs/safe-settings-deployment/spec.md b/openspec/changes/adopt-safe-settings/specs/safe-settings-deployment/spec.md new file mode 100644 index 0000000..1259054 --- /dev/null +++ b/openspec/changes/adopt-safe-settings/specs/safe-settings-deployment/spec.md @@ -0,0 +1,127 @@ +## ADDED Requirements + +### Requirement: Dedicated GitHub App for safe-settings + +A dedicated GitHub App SHALL be registered for safe-settings, separate from the +`complytime-bot` app used by peribolos. The app SHALL be named +`safe-settings-bot` (or similar distinguishable name) and installed on the +complytime organization. + +The app SHALL have the following permissions: +- Repository Administration: write +- Repository Contents: read +- Repository Checks: write +- Repository Pull requests: write +- Organization Administration: read + +The app SHALL NOT have Organization Members, Organization Administration write, +or any other permissions not listed above. + +#### Scenario: App installed with correct permissions + +- **GIVEN** the safe-settings-bot GitHub App is registered +- **WHEN** the app is installed on the complytime org +- **THEN** the app has only the permissions listed above +- **AND** the app does not have Organization Members or Organization + Administration write permissions + +### Requirement: App credentials stored in .github repo + +The safe-settings GitHub App credentials SHALL be stored in the `.github` +repository as follows: +- `SAFE_SETTINGS_APP_ID` — repository variable (not a secret, since the app + ID is not sensitive) +- `SAFE_SETTINGS_PRIVATE_KEY` — repository secret (base64-encoded private key) + +These credentials SHALL be separate from the peribolos bot credentials. +No additional secrets are required (`CLIENT_ID`, `CLIENT_SECRET`, and +`WEBHOOK_SECRET` are not needed for the GHA-only deployment model). + +#### Scenario: Credentials available for workflows + +- **GIVEN** the safe-settings app credentials are stored in the `.github` repo +- **WHEN** a GitHub Actions workflow references `vars.SAFE_SETTINGS_APP_ID` + and `secrets.SAFE_SETTINGS_PRIVATE_KEY` +- **THEN** the workflow can authenticate as the safe-settings app via + Probot-native JWT authentication + +### Requirement: GitHub Actions sync workflow + +A GitHub Actions workflow `safe_settings_sync.yml` SHALL be configured in +the `.github` repo to run `npm run full-sync`. The workflow SHALL be +triggered by: +- `push` to main branch — immediate convergence after config merges +- `schedule` daily at 06:00 UTC — drift correction (30 min after peribolos + sync at 05:30 UTC, ensuring membership/teams are applied before repo + settings) +- `workflow_dispatch` — manual convergence on demand + +The workflow SHALL: +- Check out the `.github` repo (admin repo with config) +- Check out `github/safe-settings` at a pinned version tag +- Run `npm install` and `npm run full-sync` +- Pass environment variables: `APP_ID`, `PRIVATE_KEY`, `GH_ORG=complytime`, + `ADMIN_REPO=.github`, `CONFIG_PATH=safe-settings` +- Use `DEPLOYMENT_CONFIG_FILE` pointing to the workspace-relative path of + `deployment-settings.yml` + +The workflow SHALL use a concurrency group to prevent concurrent sync runs. + +#### Scenario: Push-triggered sync after config merge + +- **GIVEN** a PR modifying safe-settings config is merged to main +- **WHEN** the push event triggers the sync workflow +- **THEN** safe-settings runs `full-sync` and applies the updated config + to all managed repos + +#### Scenario: Scheduled drift correction + +- **GIVEN** a user modified repo settings via the GitHub UI +- **WHEN** the daily scheduled sync runs at 06:00 UTC +- **THEN** safe-settings detects the drift and reverts the settings to match + the declared configuration + +#### Scenario: Manual convergence via workflow_dispatch + +- **GIVEN** a maintainer needs immediate convergence +- **WHEN** they trigger `workflow_dispatch` on the sync workflow +- **THEN** safe-settings runs `full-sync` and applies all settings + +### Requirement: Config directory in .github repo + +safe-settings configuration SHALL be stored in the `.github` repo under a +`safe-settings/` directory at the repository root. The environment variable +`CONFIG_PATH` SHALL be set to `safe-settings` and `ADMIN_REPO` SHALL be set +to `.github`. + +The directory structure SHALL be: +``` +safe-settings/ +├── settings.yml # org-wide defaults +├── deployment-settings.yml # runtime config +├── suborgs/ # suborg-level settings +│ ├── code-repos.yml +│ └── non-code-repos.yml +└── repos/ # per-repo overrides (as needed) +``` + +#### Scenario: Config loaded from correct location + +- **GIVEN** the safe-settings config exists in the `.github` repo under + `safe-settings/` +- **WHEN** safe-settings runs a full sync +- **THEN** it reads org-wide settings from `safe-settings/settings.yml` + and merges them with suborg and repo level overrides + +### Requirement: Deployment settings exclude admin repo + +The `deployment-settings.yml` SHALL configure `restrictedRepos` to exclude +the `.github` repo, the `admin` repo (if it exists), and the +`safe-settings` repo (if it exists) from safe-settings management. The +`.github` repo's settings and rulesets remain manually managed. + +#### Scenario: Admin repo excluded from management + +- **GIVEN** `deployment-settings.yml` excludes `.github` from management +- **WHEN** safe-settings runs a full sync +- **THEN** it does not apply settings to the `.github` repository itself diff --git a/openspec/changes/adopt-safe-settings/specs/settings-override-policy/spec.md b/openspec/changes/adopt-safe-settings/specs/settings-override-policy/spec.md new file mode 100644 index 0000000..7edf8fb --- /dev/null +++ b/openspec/changes/adopt-safe-settings/specs/settings-override-policy/spec.md @@ -0,0 +1,56 @@ +## ADDED Requirements + +### Requirement: Override validators defined in deployment settings + +The `deployment-settings.yml` SHALL define `overridevalidators` that prevent +suborg or repo level settings from weakening org-level protections. + +#### Scenario: Override validator prevents weakening required approvers + +- **GIVEN** the org-level settings define + `required_approving_review_count: 1` +- **AND** an `overridevalidator` for `branches` checks that the override count + is not lower than the base count +- **WHEN** a repo-level config sets `required_approving_review_count: 0` +- **THEN** safe-settings rejects the override and reports a validation error + in the sync workflow logs + +### Requirement: Config validators for collaborator permissions + +The `deployment-settings.yml` SHALL define `configvalidators` that prevent +granting `admin` permission to collaborators at any config level. + +#### Scenario: Admin permission for collaborator rejected + +- **GIVEN** a `configvalidator` for `collaborators` checks that permission + is not `admin` +- **WHEN** a config file grants a collaborator `admin` permission +- **THEN** safe-settings rejects the config and reports a validation error + +### Requirement: Validators produce clear error messages + +All validators (both `configvalidators` and `overridevalidators`) SHALL include +an `error` field with a human-readable message explaining why the validation +failed and what the correct configuration should be. + +#### Scenario: Validation error message in sync output + +- **GIVEN** a config contains an invalid override +- **WHEN** safe-settings runs the full sync +- **THEN** the sync output includes the validator's error message +- **AND** the error message explains which policy was violated + +### Requirement: Validators enforced during full sync + +Validators SHALL run during every full sync (push-triggered, scheduled, or +manual dispatch). If a validator fails during sync, the affected settings +change SHALL NOT be applied and an error SHALL be reported in the workflow +logs. + +#### Scenario: Validator blocks apply on push to main + +- **GIVEN** a config change that violates an override validator was merged + (e.g., the validator was added after the config was already committed) +- **WHEN** safe-settings runs the full sync +- **THEN** the invalid settings are not applied +- **AND** the workflow logs report the validation error diff --git a/openspec/changes/adopt-safe-settings/specs/tool-boundary-enforcement/spec.md b/openspec/changes/adopt-safe-settings/specs/tool-boundary-enforcement/spec.md new file mode 100644 index 0000000..23a13ef --- /dev/null +++ b/openspec/changes/adopt-safe-settings/specs/tool-boundary-enforcement/spec.md @@ -0,0 +1,101 @@ +## ADDED Requirements + +### Requirement: Field ownership boundary defined and enforced + +The following field ownership boundary SHALL be enforced between peribolos and +safe-settings. Neither tool SHALL manage fields owned by the other. + +**Peribolos owns** (via `peribolos.yaml` repo definitions): +- `description` +- `has_projects` +- `default_branch` + +**Safe-settings owns** (via `safe-settings/` config): +- `allow_auto_merge` +- `delete_branch_on_merge` +- `allow_squash_merge` +- `allow_merge_commit` +- `allow_rebase_merge` +- `allow_update_branch` +- `has_wiki` +- `has_issues` +- Security settings (vulnerability alerts, automated fixes) +- Branch protection rules +- Rulesets +- Labels, milestones, autolinks + +**Neither tool manages**: `homepage`, `topics`, `visibility`/`private` +(set at repo creation, rarely changed). + +#### Scenario: Safe-settings config does not set peribolos-owned fields + +- **GIVEN** the safe-settings config files exist under `safe-settings/` +- **WHEN** the boundary validation tests run +- **THEN** no safe-settings config file (settings.yml, suborg files, repo + files) sets `description`, `has_projects`, or `default_branch` under + the `repository` key + +#### Scenario: Peribolos config does not set safe-settings-owned fields + +- **GIVEN** `peribolos.yaml` defines repos with settings +- **WHEN** the boundary validation tests run +- **THEN** no repo definition in `peribolos.yaml` sets `has_wiki`, + `has_issues`, `allow_auto_merge`, `delete_branch_on_merge`, + `allow_squash_merge`, `allow_merge_commit`, `allow_rebase_merge`, or + `allow_update_branch` + +### Requirement: Suborg repos exist in peribolos.yaml + +All repositories listed in safe-settings suborg files (`suborgs/*.yml`) +SHALL exist in the `peribolos.yaml` repos section. This ensures safe-settings +does not reference repos that are not managed by the organization. + +#### Scenario: Suborg references a repo not in peribolos.yaml + +- **GIVEN** a suborg file lists repo `nonexistent-repo` in `suborgrepos` +- **AND** `nonexistent-repo` does not exist in `peribolos.yaml` repos +- **WHEN** the boundary validation tests run +- **THEN** the test fails with an error identifying the unknown repo + +### Requirement: No repo appears in multiple suborg files + +A repository SHALL NOT appear in more than one suborg configuration file. +Each repo belongs to exactly one suborg group (or none, inheriting only +org-wide defaults). + +#### Scenario: Repo listed in two suborg files + +- **GIVEN** `code-repos.yml` lists `complyctl` in `suborgrepos` +- **AND** `non-code-repos.yml` also lists `complyctl` in `suborgrepos` +- **WHEN** the boundary validation tests run +- **THEN** the test fails with an error identifying the duplicate assignment + +### Requirement: Boundary tests in config/boundary_test.go + +All boundary validation logic SHALL be implemented as Go tests in +`config/boundary_test.go`. These tests SHALL: +- Parse `peribolos.yaml` to extract the repo list and per-repo fields +- Parse all YAML files under `safe-settings/` to extract configured fields +- Validate the three boundary rules above (no field overlap, suborg repos + exist, no duplicate suborg membership) + +The tests SHALL be runnable locally via `make test-unit` and SHALL run in +CI on every PR via the existing `validate-peribolos.yml` workflow (or an +equivalent validation workflow). + +#### Scenario: Boundary tests run on PR + +- **GIVEN** a PR modifies `peribolos.yaml` or any file under `safe-settings/` +- **WHEN** the CI validation workflow runs +- **THEN** `config/boundary_test.go` tests execute and report pass/fail +- **AND** the PR cannot merge if boundary tests fail + +#### Scenario: Boundary tests pass with correct config + +- **GIVEN** `peribolos.yaml` defines repos with only `description`, + `has_projects`, and `default_branch` +- **AND** safe-settings config defines only non-overlapping fields +- **AND** all suborg repos exist in `peribolos.yaml` +- **AND** no repo appears in multiple suborgs +- **WHEN** the boundary validation tests run +- **THEN** all tests pass diff --git a/openspec/changes/adopt-safe-settings/tasks.md b/openspec/changes/adopt-safe-settings/tasks.md new file mode 100644 index 0000000..bffa983 --- /dev/null +++ b/openspec/changes/adopt-safe-settings/tasks.md @@ -0,0 +1,57 @@ +## Phase 1: GitHub App Setup + +- [ ] 1.1 Register a new GitHub App (`safe-settings-bot`) in the complytime org with permissions: Repository Administration (write), Contents (read), Checks (write), Pull requests (write), Organization Administration (read) +- [ ] 1.2 Install the app on the complytime org, granting access to all repositories +- [ ] 1.3 Store `SAFE_SETTINGS_APP_ID` as a repository variable and `SAFE_SETTINGS_PRIVATE_KEY` as a repository secret in the `.github` repo (no CLIENT_ID/CLIENT_SECRET needed for GHA-only deployment) + +## Phase 2: Config Structure + +- [ ] 2.1 Create `safe-settings/` directory at the repository root (not under `.github/`) +- [ ] 2.2 Create `safe-settings/deployment-settings.yml` with `restrictedRepos` excluding `.github`, `admin`, and `safe-settings` repos, and define `overridevalidators` and `configvalidators` +- [ ] 2.3 Create `safe-settings/settings.yml` with org-wide defaults: `allow_auto_merge: true`, `delete_branch_on_merge: true`, merge strategies, `has_wiki: false`, security settings (vulnerability alerts, automated fixes) — SHALL NOT include peribolos-owned fields (`description`, `has_projects`, `default_branch`) +- [ ] 2.4 Define org-level rulesets in `safe-settings/settings.yml` for code repos: deletion protection, non-fast-forward, required signatures, PR requirements (dismiss stale reviews, code owner review, last push approval, 1 required approver, thread resolution) +- [ ] 2.5 Create `safe-settings/suborgs/code-repos.yml` with `suborgrepos` listing: complyctl, complytime-providers, complytime-policies, complyscribe, complytime-collector-components, gemara-content-service, org-infra +- [ ] 2.6 Create `safe-settings/suborgs/non-code-repos.yml` with `suborgrepos` listing: community, complytime-demos, website, complytime — lighter ruleset (no required signatures, no code owner review) +- [ ] 2.7 Rename the `.github` repo's "verifiy" ruleset to "verify" via the GitHub UI (one-time manual fix; this ruleset remains manually managed) + +## Phase 3: Deployment (GitHub Actions) + +- [ ] 3.1 Create `safe_settings_sync.yml` workflow in `.github/workflows/` with triggers: `push` to main, daily `schedule` at 06:00 UTC, `workflow_dispatch` +- [ ] 3.2 Configure the workflow to check out both the `.github` repo and `github/safe-settings` at a pinned version tag +- [ ] 3.3 Configure the workflow to run `npm install` and `npm run full-sync` with env vars: `APP_ID`, `PRIVATE_KEY`, `GH_ORG=complytime`, `ADMIN_REPO=.github`, `CONFIG_PATH=safe-settings`, `DEPLOYMENT_CONFIG_FILE` +- [ ] 3.4 Add a concurrency group to prevent concurrent sync runs +- [ ] 3.5 Test workflow: trigger `workflow_dispatch`, verify settings are applied to managed repos + +## Phase 4: Dry-Run Validation + +- [ ] 4.1 Run safe-settings in dry-run mode against all repos to compare declared config vs current GitHub state (via `workflow_dispatch` or local `make safe-settings-dryrun`) +- [ ] 4.2 Review the output — identify any unexpected changes that would be applied +- [ ] 4.3 Adjust config to match desired state (not current state, if current state is wrong) + +## Phase 5: Apply and Verify + +- [ ] 5.1 Merge the safe-settings config to main — verify push-triggered sync applies settings to all managed repos +- [ ] 5.2 Verify org-level rulesets are created and applied to the correct repos +- [ ] 5.3 Verify repo settings (auto-merge, delete-branch-on-merge, wiki disabled) are applied +- [ ] 5.4 Test drift correction: manually change a repo setting via UI, trigger `workflow_dispatch`, verify safe-settings reverts it +- [ ] 5.5 Test override validator: merge a config that lowers required approvers below org default, verify the sync rejects the change + +## Phase 6: Validation and Guardrails + +- [ ] 6.1 Create `config/boundary_test.go` with Go tests that validate: all suborg repos exist in `peribolos.yaml`, no repo in multiple suborgs, no safe-settings config sets peribolos-owned fields, no peribolos config sets safe-settings-owned fields +- [ ] 6.2 Extend `make lint` to cover `safe-settings/**/*.yml` with yamllint (or add `make safe-settings-validate` target) +- [ ] 6.3 Update `validate-peribolos.yml` workflow (or create `validate-safe-settings.yml`) to run boundary tests on every PR touching `peribolos.yaml` or `safe-settings/**` +- [ ] 6.4 Verify boundary tests run and pass in CI before any PR can merge + +## Phase 7: Local Development + +- [ ] 7.1 Add `ensure-safe-settings` Makefile target — clone `github/safe-settings` at a pinned version and run `npm install` (idempotent, mirrors `ensure-peribolos`) +- [ ] 7.2 Add `safe-settings-validate` Makefile target — run yamllint on all `safe-settings/**/*.yml` files +- [ ] 7.3 Add `safe-settings-dryrun` Makefile target — run `npm run full-sync` locally with credentials from `~/.config/safe-settings/env` or environment variables +- [ ] 7.4 Update `make sanity` to include safe-settings YAML validation + +## Phase 8: Documentation + +- [ ] 8.1 Create `MAINTAINING.md` at repo root covering: tool boundary table, common workflows (add member, change rulesets, add repo to safe-settings, add override), local testing instructions, troubleshooting guide, override validator policies +- [ ] 8.2 Update `README.md` to link to `MAINTAINING.md` for maintainer documentation +- [ ] 8.3 Update `.github/CODEOWNERS` if path-specific rules are needed for `safe-settings/` directory From 755079f39f00f9921fe26c89aadf594ef6472772 Mon Sep 17 00:00:00 2001 From: Marcus Burghardt Date: Thu, 21 May 2026 11:55:08 +0200 Subject: [PATCH 2/5] feat: add initial safe-settings configuration Org-wide defaults, rulesets, and deployment config derived from auditing the current GitHub state of all complytime repos. Key decisions: - Code repos ruleset with 1 approver, stale review dismissal, code owner review, last push approval - Non-code repos ruleset with lighter protection (1 approver only) - complyctl override requiring 2 approvers - Override validator prevents lowering approver count below org default - .github repo excluded (manually managed, avoids circular dependency) - complyscribe and gemara-content-service excluded (archived/pending) Signed-off-by: Marcus Burghardt Assisted-by: OpenCode (claude-opus-4-6) --- safe-settings/deployment-settings.yml | 41 ++++++ safe-settings/repos/complyctl.yml | 34 +++++ safe-settings/settings.yml | 169 +++++++++++++++++++++++ safe-settings/suborgs/code-repos.yml | 17 +++ safe-settings/suborgs/non-code-repos.yml | 14 ++ 5 files changed, 275 insertions(+) create mode 100644 safe-settings/deployment-settings.yml create mode 100644 safe-settings/repos/complyctl.yml create mode 100644 safe-settings/settings.yml create mode 100644 safe-settings/suborgs/code-repos.yml create mode 100644 safe-settings/suborgs/non-code-repos.yml diff --git a/safe-settings/deployment-settings.yml b/safe-settings/deployment-settings.yml new file mode 100644 index 0000000..20dcc1a --- /dev/null +++ b/safe-settings/deployment-settings.yml @@ -0,0 +1,41 @@ +# Deployment settings for github/safe-settings. +# Controls which repos are managed and defines validation policies. +# +# This file is read at runtime via the DEPLOYMENT_CONFIG_FILE env var. +# See: https://github.com/github/safe-settings + +# Repositories excluded from safe-settings management. +# The .github repo is excluded to avoid circular dependency (it hosts +# the safe-settings config). Its ruleset is managed manually via the +# GitHub UI. +restrictedRepos: + exclude: + - .github + - admin + - safe-settings + +# Config validators run against each config entry before applying. +# Return true to allow, false to reject. +configvalidators: + - plugin: collaborators + error: | + Admin permission cannot be granted to collaborators via safe-settings. + Use peribolos team membership with admin role instead. + script: | + return baseconfig.permission != 'admin' + +# Override validators run when a suborg or repo config overrides an +# org-level setting. Return true to allow, false to reject. +overridevalidators: + - plugin: branches + error: | + Branch protection required_approving_review_count cannot be lowered + below the org-level default. Contact an org admin to request an + exception. + script: | + const baseCount = baseconfig?.protection?.required_pull_request_reviews?.required_approving_review_count + const overrideCount = overrideconfig?.protection?.required_pull_request_reviews?.required_approving_review_count + if (baseCount && overrideCount) { + return overrideCount >= baseCount + } + return true diff --git a/safe-settings/repos/complyctl.yml b/safe-settings/repos/complyctl.yml new file mode 100644 index 0000000..bec5752 --- /dev/null +++ b/safe-settings/repos/complyctl.yml @@ -0,0 +1,34 @@ +# Repo-level override for complyctl. +# complyctl is the primary CLI tool and requires stricter review rules +# than the org-wide code-repos baseline. +# +# This override is applied on top of the code-repos suborg settings, +# which inherit from org-wide settings.yml. + +# Override the org-level code-repos-default-branch ruleset to require +# 2 approvers (org baseline is 1). This matches the current +# general-rules ruleset on complyctl (id=2569356). +# +# NOTE: safe-settings rulesets defined here are repo-level rulesets, +# separate from the org-level rulesets in settings.yml. The org-level +# ruleset still applies; GitHub evaluates both and the most restrictive +# rule wins. +rulesets: + - name: complyctl-extra-review + target: branch + enforcement: active + + conditions: + ref_name: + include: + - "~DEFAULT_BRANCH" + exclude: [] + + rules: + - type: pull_request + parameters: + required_approving_review_count: 2 + dismiss_stale_reviews_on_push: true + require_code_owner_review: true + require_last_push_approval: true + required_review_thread_resolution: false diff --git a/safe-settings/settings.yml b/safe-settings/settings.yml new file mode 100644 index 0000000..1ec7518 --- /dev/null +++ b/safe-settings/settings.yml @@ -0,0 +1,169 @@ +# Organization-wide repository settings for the complytime GitHub org. +# Applied to all managed repos. Suborg and repo-level overrides can +# extend but not weaken these defaults (enforced by override validators). +# +# IMPORTANT: This file must NOT set fields owned by peribolos: +# description, has_projects, default_branch +# See MAINTAINING.md for the full tool boundary. +# +# MIGRATION NOTE: Existing repo-level rulesets (created manually via the +# GitHub UI) use inconsistent names across repos. Safe-settings creates +# NEW org-level rulesets alongside the existing ones. GitHub evaluates +# all active rulesets and the most restrictive rule wins, so this is +# safe. After verifying the org-level rulesets work correctly, the old +# repo-level rulesets should be deleted manually. +# +# Current repo-level rulesets to clean up after migration: +# complyctl: general-rules (active), verify-1-approver (disabled) +# complytime-collector-components: Default Protection (active) +# complytime-policies: general-rules (disabled) +# complytime-providers: branch-protection (active) +# org-infra: Branch Protection (active) +# community: Branch Protection (active) +# complytime-demos: Branch Protection (active) +# website: general-rules (active) +# +# Excluded from safe-settings management: +# complyscribe: archived +# gemara-content-service: scheduled for archival + +repository: + # Allow auto-merge on pull requests when all requirements are met. + # Currently enabled on 7 repos; this will enable it on the remaining 4 + # (complytime, complytime-demos, website, complytime-policies). + allow_auto_merge: true + + # Automatically delete head branches after pull requests are merged. + # Currently only on complyctl; this enables it org-wide. + delete_branch_on_merge: true + + # Merge strategies — all enabled org-wide, matching current state. + allow_squash_merge: true + allow_merge_commit: true + allow_rebase_merge: true + + # Allow the "Update branch" button on PRs. + # Currently only on complyctl; this enables it org-wide. + allow_update_branch: true + + # Disable wiki org-wide. Documentation lives in dedicated repos. + # Currently enabled on all repos; this is an intentional hardening. + has_wiki: false + + # Dependabot vulnerability alerts and automated security fixes. + # Currently only complyctl and org-infra have dependabot_security_updates + # enabled; this enables it org-wide. + security: + enableVulnerabilityAlerts: true + enableAutomatedSecurityFixes: true + +# Rulesets applied at the org level. +# These create GitHub rulesets visible under Settings > Rules > Rulesets. +# +# NOTE on required_signatures: Not currently enforced on any repo. +# Adding it requires all contributors to configure GPG/SSH signing. +# Omitted from initial config. Add as a follow-up after contributor +# onboarding. +rulesets: + # Ruleset for code repositories — strict branch protection. + # Baseline derived from the complyctl and org-infra rulesets, which + # are the most mature in the org. + - name: code-repos-default-branch + target: branch + enforcement: active + + bypass_actors: + - actor_id: 1 + actor_type: OrganizationAdmin + bypass_mode: always + + conditions: + ref_name: + include: + - "~DEFAULT_BRANCH" + exclude: [] + repository_name: + include: + - complyctl + - complytime-collector-components + - complytime-policies + - complytime-providers + - org-infra + exclude: [] + + rules: + # Prevent deletion of the default branch. + # Currently enforced on all repos that have rulesets. + - type: deletion + + # Prevent force pushes to the default branch. + # Currently enforced on all repos that have rulesets. + - type: non_fast_forward + + # Pull request requirements. + # Baseline: 1 approver, dismiss stale reviews, code owner review, + # last push approval. This matches the org-infra ruleset. + # + # complyctl currently requires 2 approvers — preserved via a + # repo-level override in repos/complyctl.yml. + # + # required_review_thread_resolution is false everywhere today. + # Kept false to match current state. Can tighten later. + - type: pull_request + parameters: + required_approving_review_count: 1 + dismiss_stale_reviews_on_push: true + require_code_owner_review: true + require_last_push_approval: true + required_review_thread_resolution: false + + # Preserve any status checks configured outside of safe-settings + # (e.g., via individual repo workflow configurations). + # Each repo has different CI checks (unit-test, test, test(3.10), + # etc.) — EXTERNALLY_DEFINED keeps existing checks untouched. + - type: required_status_checks + parameters: + strict_required_status_checks_policy: true + required_status_checks: + - context: "{{EXTERNALLY_DEFINED}}" + + # Ruleset for non-code repositories — lighter protection. + # Baseline derived from community, complytime-demos, and website + # rulesets. + - name: non-code-repos-default-branch + target: branch + enforcement: active + + bypass_actors: + - actor_id: 1 + actor_type: OrganizationAdmin + bypass_mode: always + + conditions: + ref_name: + include: + - "~DEFAULT_BRANCH" + exclude: [] + repository_name: + include: + - community + - complytime + - complytime-demos + - website + exclude: [] + + rules: + # Prevent deletion of the default branch. + - type: deletion + + # Prevent force pushes to the default branch. + - type: non_fast_forward + + # Pull request requirements — lighter than code repos. + # 1 approver, no dismiss stale reviews, no code owner review, + # no last push approval. Matches the current complytime-demos + # and community baseline. + - type: pull_request + parameters: + required_approving_review_count: 1 + required_review_thread_resolution: false diff --git a/safe-settings/suborgs/code-repos.yml b/safe-settings/suborgs/code-repos.yml new file mode 100644 index 0000000..689ee9e --- /dev/null +++ b/safe-settings/suborgs/code-repos.yml @@ -0,0 +1,17 @@ +# Suborg: code repositories. +# These repos contain source code and require strict security controls: +# required code owner reviews, required status checks, and stale review +# dismissal. +# +# Org-wide defaults from settings.yml are inherited automatically. +# Only override settings here if code repos need different values +# than the org-wide defaults. +# +# Excluded: complyscribe (archived), gemara-content-service (archival pending) + +suborgrepos: + - complyctl + - complytime-collector-components + - complytime-policies + - complytime-providers + - org-infra diff --git a/safe-settings/suborgs/non-code-repos.yml b/safe-settings/suborgs/non-code-repos.yml new file mode 100644 index 0000000..27e6d75 --- /dev/null +++ b/safe-settings/suborgs/non-code-repos.yml @@ -0,0 +1,14 @@ +# Suborg: non-code repositories. +# These repos hold community content, demos, releases, and the website. +# They use lighter branch protection: no required signatures, no code +# owner review requirement. +# +# Org-wide defaults from settings.yml are inherited automatically. +# Only override settings here if non-code repos need different values +# than the org-wide defaults. + +suborgrepos: + - community + - complytime + - complytime-demos + - website From 35506bd4e3af2e45ccad76de2ee9bbae39fc1551 Mon Sep 17 00:00:00 2001 From: Marcus Burghardt Date: Thu, 21 May 2026 12:03:58 +0200 Subject: [PATCH 3/5] fix: address spec review findings across all artifacts Align specs with implementation decisions: - required_signatures downgraded to SHOULD (deferred to follow-up) - required_review_thread_resolution set to false (matches current state) - Removed archived repos from spec and config repo lists Harden deployment and boundary specs: - Pinned commit SHA, workflow timeout, YAML pre-validation - Suborg/ruleset repo list synchronization requirement - Override validator falsy-0 bypass fix - Phases reordered: boundary tests before apply Signed-off-by: Marcus Burghardt Assisted-by: OpenCode (claude-opus-4-6) --- .../changes/adopt-safe-settings/design.md | 7 ++-- .../specs/local-development-workflow/spec.md | 18 +++++++++- .../specs/org-rulesets-as-code/spec.md | 4 +-- .../specs/org-wide-repo-settings/spec.md | 28 ++++----------- .../specs/safe-settings-deployment/spec.md | 26 +++++++++++++- .../specs/settings-override-policy/spec.md | 12 +++++++ .../specs/tool-boundary-enforcement/spec.md | 36 +++++++++++++++++-- openspec/changes/adopt-safe-settings/tasks.md | 30 ++++++++-------- safe-settings/deployment-settings.yml | 5 ++- safe-settings/settings.yml | 4 +++ 10 files changed, 125 insertions(+), 45 deletions(-) diff --git a/openspec/changes/adopt-safe-settings/design.md b/openspec/changes/adopt-safe-settings/design.md index eab94fb..c2520ad 100644 --- a/openspec/changes/adopt-safe-settings/design.md +++ b/openspec/changes/adopt-safe-settings/design.md @@ -112,7 +112,7 @@ safe-settings config files automatically. │ │ ├── code-repos.yml # complyctl, providers, policies, etc. │ │ └── non-code-repos.yml # community, website, demos │ └── repos/ # per-repo overrides (only if needed) -│ └── .gitkeep +│ └── complyctl.yml # complyctl: 2 required approvers ├── .github/ │ ├── CODEOWNERS │ └── workflows/ @@ -128,8 +128,9 @@ review workflows. Keeping everything in `.github` simplifies governance. Define two suborgs to apply different policies: - `code-repos`: complyctl, complytime-providers, complytime-policies, - complyscribe, complytime-collector-components, gemara-content-service, - org-infra — strict branch protection, required reviews, required signatures + complytime-collector-components, + org-infra — strict branch protection, required reviews (required signatures deferred to follow-up after contributor onboarding). + Excluded: `complyscribe` (archived) and `gemara-content-service` (pending archival). - `non-code-repos`: community, complytime-demos, website, complytime — lighter protection, fewer required checks diff --git a/openspec/changes/adopt-safe-settings/specs/local-development-workflow/spec.md b/openspec/changes/adopt-safe-settings/specs/local-development-workflow/spec.md index 0ff9446..8ef88e5 100644 --- a/openspec/changes/adopt-safe-settings/specs/local-development-workflow/spec.md +++ b/openspec/changes/adopt-safe-settings/specs/local-development-workflow/spec.md @@ -29,7 +29,10 @@ mode against the live GitHub org. This target SHALL: - Use the Probot-native auth pattern (`APP_ID` + `PRIVATE_KEY` env vars) - Read credentials from a local file (e.g., `~/.config/safe-settings/env`) or expect them as environment variables -- Run `npm run full-sync` with `NODE_ENV=dry-run` or equivalent +- Run safe-settings in dry-run mode. The exact dry-run mechanism SHALL be + determined during implementation by consulting the safe-settings + documentation (e.g., `--dry-run` flag, `LOG_LEVEL=debug` for + preview-only output, or equivalent). - Display what changes would be applied without actually applying them #### Scenario: Dry-run shows pending changes @@ -64,6 +67,19 @@ The target SHALL: - **THEN** the target prints a message indicating it is already present - **AND** does not re-clone or re-install +### Requirement: Node.js version validation + +The `ensure-safe-settings` target SHALL validate that the local Node.js +version meets the minimum requirement (>= 18). If the version is below +the minimum, the target SHALL print an error and exit. + +#### Scenario: Node.js version too old + +- **GIVEN** the local Node.js version is 16 +- **WHEN** `make ensure-safe-settings` is run +- **THEN** the target prints an error about the minimum version requirement +- **AND** exits with a non-zero status + ### Requirement: Lint target covers safe-settings YAML The existing `make lint` target SHALL be extended (or a new combined target diff --git a/openspec/changes/adopt-safe-settings/specs/org-rulesets-as-code/spec.md b/openspec/changes/adopt-safe-settings/specs/org-rulesets-as-code/spec.md index ef60e69..9aa3d36 100644 --- a/openspec/changes/adopt-safe-settings/specs/org-rulesets-as-code/spec.md +++ b/openspec/changes/adopt-safe-settings/specs/org-rulesets-as-code/spec.md @@ -7,13 +7,13 @@ default branch of all code repositories. The ruleset SHALL enforce: - `type: deletion` — prevent branch deletion - `type: non_fast_forward` — prevent force pushes -- `type: required_signatures` — require signed commits +- `type: required_signatures` — require signed commits (SHOULD; deferred until contributor GPG/SSH signing onboarding is complete. Omitted from initial deployment.) - `type: pull_request` with parameters: - `dismiss_stale_reviews_on_push: true` - `require_code_owner_review: true` - `require_last_push_approval: true` - `required_approving_review_count: 1` - - `required_review_thread_resolution: true` + - `required_review_thread_resolution: false` (matches current org state; can be tightened as a follow-up) The ruleset SHALL target `~DEFAULT_BRANCH` and SHALL include code repos via `repository_name` conditions. diff --git a/openspec/changes/adopt-safe-settings/specs/org-wide-repo-settings/spec.md b/openspec/changes/adopt-safe-settings/specs/org-wide-repo-settings/spec.md index 72c8b8e..b37e8fc 100644 --- a/openspec/changes/adopt-safe-settings/specs/org-wide-repo-settings/spec.md +++ b/openspec/changes/adopt-safe-settings/specs/org-wide-repo-settings/spec.md @@ -67,8 +67,10 @@ is not version-controlled. Two suborg configuration files SHALL be defined: 1. `code-repos.yml` — applies to repositories containing source code: - complyctl, complytime-providers, complytime-policies, complyscribe, - complytime-collector-components, gemara-content-service, org-infra + complyctl, complytime-providers, complytime-policies, + complytime-collector-components, org-infra + + Excluded: `complyscribe` (archived) and `gemara-content-service` (pending archival). 2. `non-code-repos.yml` — applies to repositories without source code: community, complytime-demos, website, complytime @@ -91,22 +93,6 @@ Each suborg file SHALL use `suborgrepos` to list the repos in that group. ### Requirement: Safe-settings SHALL NOT manage peribolos-owned fields -Safe-settings config files (settings.yml, suborg files, repo override files) -SHALL NOT set the following fields, which are owned by peribolos: -- `description` -- `has_projects` -- `default_branch` - -These fields are managed via `peribolos.yaml` repo definitions. Setting them -in safe-settings config would create a flapping conflict where each tool -reverts the other's changes. - -This boundary is enforced by automated Go tests in -`config/boundary_test.go` (see `tool-boundary-enforcement` spec). - -#### Scenario: Safe-settings config sets a peribolos-owned field - -- **GIVEN** a safe-settings config file sets `description` under `repository` -- **WHEN** the boundary validation tests run -- **THEN** the test fails with an error identifying the field and file -- **AND** the PR cannot merge until the violation is removed +Safe-settings config files SHALL NOT set fields owned by peribolos. See +the `tool-boundary-enforcement` spec for the authoritative field ownership +list and enforcement details. diff --git a/openspec/changes/adopt-safe-settings/specs/safe-settings-deployment/spec.md b/openspec/changes/adopt-safe-settings/specs/safe-settings-deployment/spec.md index 1259054..58ff07d 100644 --- a/openspec/changes/adopt-safe-settings/specs/safe-settings-deployment/spec.md +++ b/openspec/changes/adopt-safe-settings/specs/safe-settings-deployment/spec.md @@ -58,12 +58,18 @@ triggered by: The workflow SHALL: - Check out the `.github` repo (admin repo with config) -- Check out `github/safe-settings` at a pinned version tag +- Check out `github/safe-settings` at a pinned commit SHA (not a mutable tag) to prevent supply chain attacks via tag manipulation. The SHA SHALL be documented in the workflow file. - Run `npm install` and `npm run full-sync` - Pass environment variables: `APP_ID`, `PRIVATE_KEY`, `GH_ORG=complytime`, `ADMIN_REPO=.github`, `CONFIG_PATH=safe-settings` - Use `DEPLOYMENT_CONFIG_FILE` pointing to the workspace-relative path of `deployment-settings.yml` +- Use a concurrency group with `cancel-in-progress: false` to prevent concurrent sync runs from partially applying settings +- Set `timeout-minutes` to 15 to prevent runaway execution +- Include a YAML validation step (yamllint) before running `full-sync` to catch syntax errors before they are applied + +The workflow SHALL accept a `dry-run` boolean input on `workflow_dispatch`, +matching the existing peribolos workflow pattern. The workflow SHALL use a concurrency group to prevent concurrent sync runs. @@ -87,6 +93,24 @@ The workflow SHALL use a concurrency group to prevent concurrent sync runs. - **WHEN** they trigger `workflow_dispatch` on the sync workflow - **THEN** safe-settings runs `full-sync` and applies all settings +#### Scenario: Workflow dispatch with dry-run option + +- **GIVEN** a maintainer wants to preview what changes would be applied +- **WHEN** they trigger `workflow_dispatch` with the `dry-run` input set + to `true` +- **THEN** the workflow runs in preview mode and logs what would change +- **AND** no actual settings are applied + +#### Scenario: Sync workflow failure + +- **GIVEN** the safe-settings sync workflow encounters an error (e.g., + credential expiry, GitHub API outage, invalid YAML) +- **WHEN** the workflow fails +- **THEN** the workflow logs include structured error output +- **AND** no partial settings are applied to some repos while others + are skipped (safe-settings processes each repo independently, so + partial application is possible — document this behavior) + ### Requirement: Config directory in .github repo safe-settings configuration SHALL be stored in the `.github` repo under a diff --git a/openspec/changes/adopt-safe-settings/specs/settings-override-policy/spec.md b/openspec/changes/adopt-safe-settings/specs/settings-override-policy/spec.md index 7edf8fb..88ad3fd 100644 --- a/openspec/changes/adopt-safe-settings/specs/settings-override-policy/spec.md +++ b/openspec/changes/adopt-safe-settings/specs/settings-override-policy/spec.md @@ -27,6 +27,18 @@ granting `admin` permission to collaborators at any config level. - **WHEN** a config file grants a collaborator `admin` permission - **THEN** safe-settings rejects the config and reports a validation error +### Requirement: Valid overrides that strengthen protections are allowed + +Override validators SHALL allow suborg or repo level settings that set +protections equal to or stricter than the org default. + +#### Scenario: Higher approver count is accepted + +- **GIVEN** the org-level default is `required_approving_review_count: 1` +- **AND** an `overridevalidator` for `branches` checks the count +- **WHEN** a repo-level config sets `required_approving_review_count: 2` +- **THEN** the override validator allows the setting + ### Requirement: Validators produce clear error messages All validators (both `configvalidators` and `overridevalidators`) SHALL include diff --git a/openspec/changes/adopt-safe-settings/specs/tool-boundary-enforcement/spec.md b/openspec/changes/adopt-safe-settings/specs/tool-boundary-enforcement/spec.md index 23a13ef..dd38b75 100644 --- a/openspec/changes/adopt-safe-settings/specs/tool-boundary-enforcement/spec.md +++ b/openspec/changes/adopt-safe-settings/specs/tool-boundary-enforcement/spec.md @@ -18,7 +18,7 @@ safe-settings. Neither tool SHALL manage fields owned by the other. - `allow_rebase_merge` - `allow_update_branch` - `has_wiki` -- `has_issues` +- `has_issues` (reserved; not actively managed in initial deployment) - Security settings (vulnerability alerts, automated fixes) - Branch protection rules - Rulesets @@ -74,7 +74,11 @@ org-wide defaults). All boundary validation logic SHALL be implemented as Go tests in `config/boundary_test.go`. These tests SHALL: -- Parse `peribolos.yaml` to extract the repo list and per-repo fields +- Parse `peribolos.yaml` to extract the repo list and per-repo fields. + The peribolos YAML structure is `orgs..repos.` with + per-repo fields at that level. Safe-settings uses a `repository` key in + `settings.yml` and suborg files, and repo-level overrides under + `safe-settings/repos/.yml`. - Parse all YAML files under `safe-settings/` to extract configured fields - Validate the three boundary rules above (no field overlap, suborg repos exist, no duplicate suborg membership) @@ -99,3 +103,31 @@ equivalent validation workflow). - **AND** no repo appears in multiple suborgs - **WHEN** the boundary validation tests run - **THEN** all tests pass + +### Requirement: Suborg repo lists match ruleset repository conditions + +All repositories listed in suborg files (`suborgs/*.yml` via `suborgrepos`) +SHALL match the corresponding `repository_name.include` lists in the +`settings.yml` rulesets. This prevents a repo from getting suborg settings +but missing the corresponding ruleset (or vice versa). + +#### Scenario: Suborg repo missing from ruleset condition + +- **GIVEN** `code-repos.yml` lists `new-repo` in `suborgrepos` +- **AND** `settings.yml` `code-repos-default-branch` ruleset does not + include `new-repo` in `repository_name.include` +- **WHEN** the boundary validation tests run +- **THEN** the test fails identifying the missing repo + +### Requirement: Boundary tests cover repo-level override files + +Boundary validation SHALL parse all YAML files under `safe-settings/repos/` +in addition to `settings.yml` and suborg files. A repo-level override file +SHALL NOT set peribolos-owned fields. + +#### Scenario: Repo override sets peribolos-owned field + +- **GIVEN** `safe-settings/repos/example.yml` sets `description` under + `repository` +- **WHEN** the boundary validation tests run +- **THEN** the test fails identifying the field and file diff --git a/openspec/changes/adopt-safe-settings/tasks.md b/openspec/changes/adopt-safe-settings/tasks.md index bffa983..587fecb 100644 --- a/openspec/changes/adopt-safe-settings/tasks.md +++ b/openspec/changes/adopt-safe-settings/tasks.md @@ -10,14 +10,14 @@ - [ ] 2.2 Create `safe-settings/deployment-settings.yml` with `restrictedRepos` excluding `.github`, `admin`, and `safe-settings` repos, and define `overridevalidators` and `configvalidators` - [ ] 2.3 Create `safe-settings/settings.yml` with org-wide defaults: `allow_auto_merge: true`, `delete_branch_on_merge: true`, merge strategies, `has_wiki: false`, security settings (vulnerability alerts, automated fixes) — SHALL NOT include peribolos-owned fields (`description`, `has_projects`, `default_branch`) - [ ] 2.4 Define org-level rulesets in `safe-settings/settings.yml` for code repos: deletion protection, non-fast-forward, required signatures, PR requirements (dismiss stale reviews, code owner review, last push approval, 1 required approver, thread resolution) -- [ ] 2.5 Create `safe-settings/suborgs/code-repos.yml` with `suborgrepos` listing: complyctl, complytime-providers, complytime-policies, complyscribe, complytime-collector-components, gemara-content-service, org-infra +- [ ] 2.5 Create `safe-settings/suborgs/code-repos.yml` with `suborgrepos` listing: complyctl, complytime-collector-components, complytime-policies, complytime-providers, org-infra - [ ] 2.6 Create `safe-settings/suborgs/non-code-repos.yml` with `suborgrepos` listing: community, complytime-demos, website, complytime — lighter ruleset (no required signatures, no code owner review) - [ ] 2.7 Rename the `.github` repo's "verifiy" ruleset to "verify" via the GitHub UI (one-time manual fix; this ruleset remains manually managed) ## Phase 3: Deployment (GitHub Actions) - [ ] 3.1 Create `safe_settings_sync.yml` workflow in `.github/workflows/` with triggers: `push` to main, daily `schedule` at 06:00 UTC, `workflow_dispatch` -- [ ] 3.2 Configure the workflow to check out both the `.github` repo and `github/safe-settings` at a pinned version tag +- [ ] 3.2 Configure the workflow to check out both the `.github` repo and `github/safe-settings` at a pinned commit SHA (not a mutable tag) - [ ] 3.3 Configure the workflow to run `npm install` and `npm run full-sync` with env vars: `APP_ID`, `PRIVATE_KEY`, `GH_ORG=complytime`, `ADMIN_REPO=.github`, `CONFIG_PATH=safe-settings`, `DEPLOYMENT_CONFIG_FILE` - [ ] 3.4 Add a concurrency group to prevent concurrent sync runs - [ ] 3.5 Test workflow: trigger `workflow_dispatch`, verify settings are applied to managed repos @@ -28,20 +28,21 @@ - [ ] 4.2 Review the output — identify any unexpected changes that would be applied - [ ] 4.3 Adjust config to match desired state (not current state, if current state is wrong) -## Phase 5: Apply and Verify +## Phase 5: Validation and Guardrails -- [ ] 5.1 Merge the safe-settings config to main — verify push-triggered sync applies settings to all managed repos -- [ ] 5.2 Verify org-level rulesets are created and applied to the correct repos -- [ ] 5.3 Verify repo settings (auto-merge, delete-branch-on-merge, wiki disabled) are applied -- [ ] 5.4 Test drift correction: manually change a repo setting via UI, trigger `workflow_dispatch`, verify safe-settings reverts it -- [ ] 5.5 Test override validator: merge a config that lowers required approvers below org default, verify the sync rejects the change +- [ ] 5.1 Create `config/boundary_test.go` with Go tests that validate: all suborg repos exist in `peribolos.yaml`, no repo in multiple suborgs, no safe-settings config sets peribolos-owned fields, no peribolos config sets safe-settings-owned fields +- [ ] 5.2 Extend `make lint` to cover `safe-settings/**/*.yml` with yamllint (or add `make safe-settings-validate` target) +- [ ] 5.3 Update `validate-peribolos.yml` workflow (or create `validate-safe-settings.yml`) to run boundary tests on every PR touching `peribolos.yaml` or `safe-settings/**` +- [ ] 5.4 Verify boundary tests run and pass in CI before any PR can merge -## Phase 6: Validation and Guardrails +## Phase 6: Apply and Verify -- [ ] 6.1 Create `config/boundary_test.go` with Go tests that validate: all suborg repos exist in `peribolos.yaml`, no repo in multiple suborgs, no safe-settings config sets peribolos-owned fields, no peribolos config sets safe-settings-owned fields -- [ ] 6.2 Extend `make lint` to cover `safe-settings/**/*.yml` with yamllint (or add `make safe-settings-validate` target) -- [ ] 6.3 Update `validate-peribolos.yml` workflow (or create `validate-safe-settings.yml`) to run boundary tests on every PR touching `peribolos.yaml` or `safe-settings/**` -- [ ] 6.4 Verify boundary tests run and pass in CI before any PR can merge +- [ ] 6.1 Merge the safe-settings config to main — verify push-triggered sync applies settings to all managed repos +- [ ] 6.2 Verify org-level rulesets are created and applied to the correct repos +- [ ] 6.3 Verify repo settings (auto-merge, delete-branch-on-merge, wiki disabled) are applied +- [ ] 6.4 Test drift correction: manually change a repo setting via UI, trigger `workflow_dispatch`, verify safe-settings reverts it +- [ ] 6.5 Test override validator: merge a config that lowers required approvers below org default, verify the sync rejects the change +- [ ] 6.6 Delete legacy repo-level rulesets (listed in settings.yml migration notes) after verifying org-level rulesets work correctly ## Phase 7: Local Development @@ -54,4 +55,5 @@ - [ ] 8.1 Create `MAINTAINING.md` at repo root covering: tool boundary table, common workflows (add member, change rulesets, add repo to safe-settings, add override), local testing instructions, troubleshooting guide, override validator policies - [ ] 8.2 Update `README.md` to link to `MAINTAINING.md` for maintainer documentation -- [ ] 8.3 Update `.github/CODEOWNERS` if path-specific rules are needed for `safe-settings/` directory +- [ ] 8.3 Update `.github/CODEOWNERS` to add path-specific rules for `safe-settings/` requiring admin approval + diff --git a/safe-settings/deployment-settings.yml b/safe-settings/deployment-settings.yml index 20dcc1a..be32caa 100644 --- a/safe-settings/deployment-settings.yml +++ b/safe-settings/deployment-settings.yml @@ -35,7 +35,10 @@ overridevalidators: script: | const baseCount = baseconfig?.protection?.required_pull_request_reviews?.required_approving_review_count const overrideCount = overrideconfig?.protection?.required_pull_request_reviews?.required_approving_review_count - if (baseCount && overrideCount) { + if (baseCount != null && overrideCount != null) { return overrideCount >= baseCount } + if (baseCount != null && overrideCount == null) { + return false + } return true diff --git a/safe-settings/settings.yml b/safe-settings/settings.yml index 1ec7518..efadb42 100644 --- a/safe-settings/settings.yml +++ b/safe-settings/settings.yml @@ -73,6 +73,8 @@ rulesets: enforcement: active bypass_actors: + # actor_id 1 is the GitHub-reserved ID for OrganizationAdmin. + # See: https://docs.github.com/en/rest/orgs/rules - actor_id: 1 actor_type: OrganizationAdmin bypass_mode: always @@ -135,6 +137,8 @@ rulesets: enforcement: active bypass_actors: + # actor_id 1 is the GitHub-reserved ID for OrganizationAdmin. + # See: https://docs.github.com/en/rest/orgs/rules - actor_id: 1 actor_type: OrganizationAdmin bypass_mode: always From 3a7df00de60b6ca301057085900a44a3e46d75b7 Mon Sep 17 00:00:00 2001 From: Marcus Burghardt Date: Thu, 21 May 2026 16:16:17 +0200 Subject: [PATCH 4/5] feat: implement safe-settings workflow, boundary tests, and docs Workflow (workflow_dispatch only, push/schedule added after validation): - Inputs: dry-run (default true), repos (optional comma-separated filter) - Generates scoped deployment-settings when repos is specified - YAML pre-validation, 15 min timeout, concurrency group Boundary tests (config/boundary_test.go): - Suborg repos exist in peribolos, no duplicate suborg membership - No cross-tool field overlap, suborg/ruleset repo list sync Makefile, docs, and spec updates: - safe-settings-validate target, extended lint, GOTOOLCHAIN=auto - MAINTAINING.md, README.md link, CODEOWNERS path rule - Specs updated for workflow_dispatch-only initial rollout Signed-off-by: Marcus Burghardt Assisted-by: OpenCode (claude-opus-4-6) --- .github/CODEOWNERS | 1 + .github/workflows/safe_settings_sync.yml | 132 ++++++ MAINTAINING.md | 249 +++++++++++ Makefile | 12 +- README.md | 13 +- config/boundary_test.go | 412 ++++++++++++++++++ .../changes/adopt-safe-settings/design.md | 47 +- .../specs/local-development-workflow/spec.md | 89 ++-- .../specs/safe-settings-deployment/spec.md | 92 ++-- openspec/changes/adopt-safe-settings/tasks.md | 69 +-- safe-settings/repos/complyctl.yml | 4 +- safe-settings/settings.yml | 4 +- 12 files changed, 961 insertions(+), 163 deletions(-) create mode 100644 .github/workflows/safe_settings_sync.yml create mode 100644 MAINTAINING.md create mode 100644 config/boundary_test.go diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index edc5ee2..fb8e077 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -1 +1,2 @@ * @jflowers @jpower432 @marcusburghardt +safe-settings/ @jflowers @jpower432 @marcusburghardt diff --git a/.github/workflows/safe_settings_sync.yml b/.github/workflows/safe_settings_sync.yml new file mode 100644 index 0000000..c548560 --- /dev/null +++ b/.github/workflows/safe_settings_sync.yml @@ -0,0 +1,132 @@ +# Workflow to apply safe-settings org configuration. +# Manual dispatch only — add push/schedule triggers after initial validation. +# Uses a dedicated safe-settings-bot GitHub App for authentication. +# +# safe-settings reads config from the admin repo's default branch (main). +# Config must be merged to main before this workflow can apply it. +# +# See MAINTAINING.md for operational documentation. +name: Safe Settings Sync + +"on": + workflow_dispatch: + inputs: + dry-run: + description: 'Preview changes without applying (NOP mode)' + required: false + type: boolean + default: true + repos: + description: >- + Comma-separated list of repos to target (e.g. "complytime-demos"). + Leave empty to apply to all managed repos. + required: false + type: string + default: '' + +concurrency: + group: safe-settings-sync + cancel-in-progress: false + +env: + # Pin to a specific release tag. Update via a deliberate PR. + # TODO: Replace with a commit SHA after initial validation. + SAFE_SETTINGS_VERSION: "2.1.17" + SAFE_SETTINGS_CODE_DIR: ".safe-settings-code" + +jobs: + sync: + if: github.repository_owner == 'complytime' + runs-on: ubuntu-latest + timeout-minutes: 15 + permissions: + contents: read + steps: + - name: Checkout complytime/.github repo + uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + + - name: Validate safe-settings YAML syntax + run: yamllint safe-settings/ + + - name: Generate scoped deployment-settings + if: inputs.repos != '' + env: + TARGET_REPOS: ${{ inputs.repos }} + run: | + echo "Scoping safe-settings to repos: $TARGET_REPOS" + + # Read all repo names from peribolos.yaml + ALL_REPOS=$(yq -r '.orgs[].repos | keys[]' peribolos.yaml) + + # Build the exclude list: all repos EXCEPT the target ones + EXCLUDE_LIST="" + for repo in $ALL_REPOS; do + SKIP=false + IFS=',' read -ra TARGETS <<< "$TARGET_REPOS" + for target in "${TARGETS[@]}"; do + target=$(echo "$target" | xargs) # trim whitespace + if [ "$repo" = "$target" ]; then + SKIP=true + break + fi + done + if [ "$SKIP" = "false" ]; then + EXCLUDE_LIST="${EXCLUDE_LIST} - ${repo}"$'\n' + fi + done + + # Always exclude the admin repo + EXCLUDE_LIST="${EXCLUDE_LIST} - .github"$'\n' + EXCLUDE_LIST="${EXCLUDE_LIST} - admin"$'\n' + EXCLUDE_LIST="${EXCLUDE_LIST} - safe-settings"$'\n' + + # Write scoped deployment-settings.yml (preserving validators + # from the original file) + { + echo "# Auto-generated: scoped to repos: $TARGET_REPOS" + echo "restrictedRepos:" + echo " exclude:" + echo -n "$EXCLUDE_LIST" + # Append validators from original file + echo "" + yq -r 'del(.restrictedRepos) | select(. != null)' \ + safe-settings/deployment-settings.yml + } > /tmp/scoped-deployment-settings.yml + + echo "--- Generated deployment-settings.yml ---" + cat /tmp/scoped-deployment-settings.yml + + - name: Checkout safe-settings code + uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + with: + repository: github/safe-settings + ref: ${{ env.SAFE_SETTINGS_VERSION }} + path: ${{ env.SAFE_SETTINGS_CODE_DIR }} + + - name: Setup Node.js + uses: actions/setup-node@49933ea5288caeca8642195f572a2b2b9a5e172c # v4.4.0 + with: + node-version: '20' + + - name: Install safe-settings dependencies + working-directory: ${{ env.SAFE_SETTINGS_CODE_DIR }} + run: npm install + + - name: Run safe-settings sync + working-directory: ${{ env.SAFE_SETTINGS_CODE_DIR }} + env: + APP_ID: ${{ vars.SAFE_SETTINGS_APP_ID }} + PRIVATE_KEY: ${{ secrets.SAFE_SETTINGS_PRIVATE_KEY }} + GH_ORG: complytime + ADMIN_REPO: .github + CONFIG_PATH: safe-settings + DEPLOYMENT_CONFIG_FILE: >- + ${{ inputs.repos != '' && '/tmp/scoped-deployment-settings.yml' + || format('{0}/safe-settings/deployment-settings.yml', github.workspace) }} + FULL_SYNC_NOP: ${{ inputs.dry-run }} + LOG_LEVEL: ${{ inputs.dry-run && 'debug' || 'info' }} + run: | + if [ "$FULL_SYNC_NOP" = "true" ]; then + echo "=== DRY-RUN MODE: Showing what would change ===" + fi + npm run full-sync diff --git a/MAINTAINING.md b/MAINTAINING.md new file mode 100644 index 0000000..686d2d6 --- /dev/null +++ b/MAINTAINING.md @@ -0,0 +1,249 @@ +# Maintaining the complytime GitHub Organization + +This document covers operational workflows for managing the complytime +GitHub organization using two complementary tools: **peribolos** and +**safe-settings**. + +## Tool Boundary + +| Area | Tool | Config Location | +|------|------|----------------| +| Org membership (admins, members) | peribolos | `peribolos.yaml` | +| Team creation, membership, privacy | peribolos | `peribolos.yaml` | +| Team-to-repo permission mappings | peribolos | `peribolos.yaml` | +| Repo description | peribolos | `peribolos.yaml` | +| Repo has_projects | peribolos | `peribolos.yaml` | +| Repo default_branch | peribolos | `peribolos.yaml` | +| Repo merge strategies | safe-settings | `safe-settings/settings.yml` | +| Repo auto-merge, delete-branch | safe-settings | `safe-settings/settings.yml` | +| Repo has_wiki | safe-settings | `safe-settings/settings.yml` | +| Dependabot alerts and fixes | safe-settings | `safe-settings/settings.yml` | +| Branch protection rules | safe-settings | `safe-settings/settings.yml` | +| Rulesets | safe-settings | `safe-settings/settings.yml` | +| `.github` repo ruleset | **manual** | GitHub UI | + +**Why two tools?** Peribolos manages org-level concerns (who is a member, +what teams exist, what permissions teams have). Safe-settings manages +repo-level concerns (how branches are protected, what merge strategies +are allowed, what security features are enabled). This separation follows +the principle of least privilege for their respective GitHub App +permissions. + +**Boundary enforcement:** Go tests in `config/boundary_test.go` validate +that neither tool manages fields owned by the other. These tests run on +every PR via CI. + +## Common Workflows + +### Add or Remove an Org Member + +1. Edit `peribolos.yaml` — add/remove the username from the `admins` or + `members` list (keep sorted alphabetically). +2. If adding, add to the appropriate team(s) as well. +3. Submit a PR. CI validates the config automatically. +4. After merge, peribolos applies the change (push-triggered or daily + at 05:30 UTC). + +### Create a New Team or Change Team Membership + +1. Edit `peribolos.yaml` — add/modify the team under the `teams` section. +2. Ensure team members are org members (CI validates this). +3. Ensure admins are listed as `maintainers`, not `members` (CI validates). +4. Submit a PR and merge. + +### Add a New Repository to Safe-settings Management + +1. Add the repo to `peribolos.yaml` with `description`, `has_projects`, + and `default_branch` (peribolos-owned fields). +2. Add the repo to the appropriate suborg file: + - `safe-settings/suborgs/code-repos.yml` for code repositories + - `safe-settings/suborgs/non-code-repos.yml` for non-code repositories +3. Add the repo to the matching ruleset `repository_name.include` list + in `safe-settings/settings.yml`. **Both files must be updated** — the + suborg controls settings inheritance, the ruleset controls branch + protection. +4. Submit a PR. CI boundary tests validate consistency. +5. After merge, trigger `workflow_dispatch` on the "Safe Settings Sync" + workflow to apply. + +### Change Branch Protection Rules or Rulesets + +1. Edit `safe-settings/settings.yml` — modify the ruleset under `rulesets`. +2. The `safe-settings: code repos` ruleset applies to code repos. +3. The `safe-settings: non-code repos` ruleset applies to non-code repos. +4. Submit a PR and merge. +5. Trigger `workflow_dispatch` to apply. + +### Add a Repo-Specific Override + +Use repo overrides sparingly. Only create one when a repo needs settings +that differ from its suborg defaults. + +1. Create `safe-settings/repos/.yml`. +2. Set only the fields that differ from the suborg/org defaults. +3. Do NOT set peribolos-owned fields (`description`, `has_projects`, + `default_branch`). +4. Submit a PR. CI boundary tests validate the override. + +See `safe-settings/repos/complyctl.yml` for an example (complyctl requires +2 approvers instead of the org default of 1). + +## Override Validator Policies + +Override validators in `safe-settings/deployment-settings.yml` enforce +a security floor: + +- **Approver count floor**: Suborg or repo configs cannot lower + `required_approving_review_count` below the org default. Setting it + higher is allowed. +- **No admin collaborators**: The `admin` permission cannot be granted + to collaborators via safe-settings. Use peribolos team membership + with admin role instead. + +**Requesting an exception:** If a legitimate use case requires bypassing +a validator, discuss with org admins. Exceptions require modifying the +validator script in `deployment-settings.yml` via a reviewed PR. + +## Local Validation + +### Prerequisites + +- Go (version in `go.mod`) +- `yamllint` (for YAML validation) + +### Commands + +```bash +# Validate all YAML (peribolos + safe-settings) +make lint + +# Run all Go tests (peribolos + boundary) +make test-unit + +# Validate only safe-settings YAML +make safe-settings-validate + +# Full validation: format, vet, lint, tests, diff check +make sanity +``` + +## Applying Safe-settings Changes + +safe-settings reads its config from the `.github` repo's default branch +via the GitHub API. Config changes must be **merged to main** before +safe-settings can apply them. + +### Testing sequence + +1. **Local validation** (before PR): + ```bash + make test-unit # boundary tests + make safe-settings-validate # YAML syntax + ``` + +2. **Submit PR** — CI runs boundary tests and YAML validation. + +3. **Merge PR** — config lands on main. + +4. **Dry-run against a single repo** — go to Actions > "Safe Settings + Sync" > "Run workflow": + - Set `dry-run` to `true` + - Set `repos` to a single repo (e.g., `complytime-demos`) + - Review the workflow output to see what would change + +5. **Apply to a single repo** — same workflow: + - Set `dry-run` to `false` + - Set `repos` to the same repo + - Verify the changes in the GitHub UI + +6. **Apply to all repos** — same workflow: + - Set `dry-run` to `false` + - Leave `repos` empty (applies to all managed repos) + +### Rollback + +If safe-settings applies incorrect settings: +1. `git revert` the config change and push to main +2. Trigger `workflow_dispatch` with `dry-run=false` — safe-settings + reverts to the previous config state +3. Or fix settings manually via the GitHub UI (safe-settings will + re-apply them on the next sync) + +## Triggering Manual Sync + +### Peribolos + +Go to Actions > "Apply Peribolos" > "Run workflow". Set `dry-run` to +`true` for a preview, or `false` to apply. + +### Safe-settings + +Go to Actions > "Safe Settings Sync" > "Run workflow": +- **dry-run**: `true` to preview, `false` to apply (defaults to `true`) +- **repos**: comma-separated list of repos to target (e.g., + `complytime-demos,community`). Leave empty to apply to all managed + repos. + +### Future automation + +After initial validation, the workflow can be extended with: +- `push` trigger on `safe-settings/**` path changes to main +- `schedule` trigger (daily at 06:00 UTC) for drift correction + +These triggers are intentionally disabled during the initial rollout to +ensure full manual control. + +## Troubleshooting + +### Settings not applied after merge + +1. Trigger `workflow_dispatch` manually — safe-settings only runs on + manual dispatch during initial rollout (no push/schedule triggers). +2. Check the "Safe Settings Sync" workflow run in the Actions tab. +3. Look for errors in the workflow logs (credential expiry, API errors). + +### Boundary test failures + +Boundary tests fail when: +- A repo in a suborg file does not exist in `peribolos.yaml` — add it + to peribolos first. +- A repo appears in multiple suborg files — each repo belongs to exactly + one suborg. +- A safe-settings config sets `description`, `has_projects`, or + `default_branch` — these are peribolos-owned fields. +- A suborg repo list does not match the corresponding ruleset + `repository_name.include` — update both files together. + +### safe-settings sync errors + +Common causes: +- **Credential expiry**: The GitHub App private key may need rotation. + Update the `SAFE_SETTINGS_PRIVATE_KEY` secret. +- **API rate limits**: The sync may fail if it hits GitHub API rate + limits. Wait and re-trigger. +- **Invalid YAML**: The workflow validates YAML before applying. Check + the yamllint output in the workflow logs. +- **safe-settings version issue**: If safe-settings behavior changes, + check the pinned version in the workflow file. + +## Excluded Repos + +The following repos are excluded from safe-settings management: + +- `.github` — the admin repo (avoids circular dependency). Its + ruleset ("verify") is managed manually via the GitHub UI. +- `complyscribe` — archived. +- `gemara-content-service` — pending archival. + +These are listed in `safe-settings/deployment-settings.yml` under +`restrictedRepos` and/or excluded from suborg files. + +## Migration Notes + +Existing repo-level rulesets (created manually via the GitHub UI) coexist +with the new org-level rulesets managed by safe-settings. GitHub evaluates +all active rulesets and the most restrictive rule wins. + +After verifying the org-level rulesets work correctly, the old repo-level +rulesets should be deleted via the GitHub UI. The full list is documented +in comments at the top of `safe-settings/settings.yml`. diff --git a/Makefile b/Makefile index e104219..a001e55 100644 --- a/Makefile +++ b/Makefile @@ -1,3 +1,6 @@ +# Ensure Go automatically downloads the toolchain version required by go.mod. +export GOTOOLCHAIN := auto + ##@ Testing test-unit: ## run unit tests with coverage @@ -12,8 +15,9 @@ vet: ## run go vet go vet ./... .PHONY: vet -lint: ## run yamllint on peribolos.yaml +lint: ## run yamllint on peribolos.yaml and safe-settings config yamllint peribolos.yaml + yamllint safe-settings/ .PHONY: lint sanity: vendor format vet lint ## ensure code is ready for commit @@ -94,6 +98,12 @@ peribolos-apply: ensure-peribolos ## apply peribolos config to the live org (DES 2>&1 | jq -r '[.severity, .time, .msg] | join(" | ")' .PHONY: peribolos-apply +##@ Safe-settings (local validation) + +safe-settings-validate: ## validate safe-settings YAML syntax + yamllint safe-settings/ +.PHONY: safe-settings-validate + ##@ CRAP Load Monitoring GAZE_VERSION ?= latest diff --git a/README.md b/README.md index a5a8cf2..861298b 100644 --- a/README.md +++ b/README.md @@ -1,9 +1,18 @@ -This repository will apply peribolos to manage organization complytime. +This repository manages the complytime GitHub organization using two +complementary tools: + +- **[Peribolos](https://docs.prow.k8s.io/docs/components/cli-tools/peribolos/)** + — org membership, teams, and team-repo permissions (`peribolos.yaml`) +- **[safe-settings](https://github.com/github/safe-settings)** — repository + settings, branch protection, rulesets, and security config (`safe-settings/`) + +For maintainer workflows, local testing, and troubleshooting, see +**[MAINTAINING.md](MAINTAINING.md)**. To use Peribolos to manage organization, the base requirement is Go setup. When running Peribolos, it needs permission to access the organization, and repository resources. The Github app installation access token has no -permission for endpoint user when peribolos updates members of organization. +permission for endpoint user when peribolos updates members of organization. The Github app user access token could help. A user access token only has permissions that both the user and the app have. For more details, please see [Generating a user access token for a GitHub App](https://docs.github.com/en/apps/creating-github-apps/authenticating-with-a-github-app/generating-a-user-access-token-for-a-github-app). diff --git a/config/boundary_test.go b/config/boundary_test.go new file mode 100644 index 0000000..7060136 --- /dev/null +++ b/config/boundary_test.go @@ -0,0 +1,412 @@ +// SPDX-License-Identifier: Apache-2.0 + +// Package config provides validation tests for the complytime org configuration. +// +// boundary_test.go validates cross-tool consistency between peribolos.yaml +// and safe-settings config. It ensures: +// - All repos in suborg files exist in peribolos.yaml +// - No repo appears in multiple suborg files +// - Safe-settings config does not set fields owned by peribolos +// - Suborg repo lists match ruleset repository_name conditions +package config + +import ( + "fmt" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/ghodss/yaml" +) + +// safeSettingsDir is the path to the safe-settings config directory. +var safeSettingsDir = "../safe-settings" + +// peribolosOwnedFields are repo-level fields that peribolos manages. +// Safe-settings config must NOT set these fields. +var peribolosOwnedFields = []string{ + "description", + "has_projects", + "default_branch", +} + +// safeSettingsOwnedFields are repo-level fields that safe-settings manages. +// Peribolos config must NOT set these fields. +var safeSettingsOwnedFields = []string{ + "has_wiki", + "has_issues", + "allow_auto_merge", + "delete_branch_on_merge", + "allow_squash_merge", + "allow_merge_commit", + "allow_rebase_merge", + "allow_update_branch", +} + +// suborg represents a parsed suborg configuration file. +type suborg struct { + SuborgRepos []string `json:"suborgrepos"` +} + +// settingsFile represents a parsed safe-settings settings.yml. +type settingsFile struct { + Repository map[string]interface{} `json:"repository"` + Rulesets []ruleset `json:"rulesets"` +} + +// ruleset represents a parsed safe-settings ruleset. +type ruleset struct { + Name string `json:"name"` + Conditions rulesetConditions `json:"conditions"` +} + +// rulesetConditions represents the conditions block of a ruleset. +type rulesetConditions struct { + RepositoryName *repositoryNameCondition `json:"repository_name"` +} + +// repositoryNameCondition represents the repository_name condition. +type repositoryNameCondition struct { + Include []string `json:"include"` +} + +// repoOverride represents a parsed repo-level override file. +type repoOverride struct { + Repository map[string]interface{} `json:"repository"` +} + +// loadSuborgs parses all suborg YAML files from the suborgs directory. +func loadSuborgs(dir string) (map[string][]string, error) { + suborgsDir := filepath.Join(dir, "suborgs") + entries, err := os.ReadDir(suborgsDir) + if err != nil { + if os.IsNotExist(err) { + return nil, nil + } + return nil, fmt.Errorf("reading suborgs directory: %w", err) + } + + result := make(map[string][]string) + for _, entry := range entries { + if entry.IsDir() || !strings.HasSuffix(entry.Name(), ".yml") { + continue + } + + data, err := os.ReadFile(filepath.Join(suborgsDir, entry.Name())) + if err != nil { + return nil, fmt.Errorf("reading suborg file %s: %w", entry.Name(), err) + } + + var s suborg + if err := yaml.Unmarshal(data, &s); err != nil { + return nil, fmt.Errorf("parsing suborg file %s: %w", entry.Name(), err) + } + + result[entry.Name()] = s.SuborgRepos + } + + return result, nil +} + +// loadSettings parses the main settings.yml file. +func loadSettings(dir string) (*settingsFile, error) { + data, err := os.ReadFile(filepath.Join(dir, "settings.yml")) + if err != nil { + return nil, fmt.Errorf("reading settings.yml: %w", err) + } + + var s settingsFile + if err := yaml.Unmarshal(data, &s); err != nil { + return nil, fmt.Errorf("parsing settings.yml: %w", err) + } + + return &s, nil +} + +// loadRepoOverrides parses all repo-level override YAML files. +func loadRepoOverrides(dir string) (map[string]*repoOverride, error) { + reposDir := filepath.Join(dir, "repos") + entries, err := os.ReadDir(reposDir) + if err != nil { + if os.IsNotExist(err) { + return nil, nil + } + return nil, fmt.Errorf("reading repos directory: %w", err) + } + + result := make(map[string]*repoOverride) + for _, entry := range entries { + if entry.IsDir() || !strings.HasSuffix(entry.Name(), ".yml") { + continue + } + + data, err := os.ReadFile(filepath.Join(reposDir, entry.Name())) + if err != nil { + return nil, fmt.Errorf("reading repo override file %s: %w", entry.Name(), err) + } + + var r repoOverride + if err := yaml.Unmarshal(data, &r); err != nil { + return nil, fmt.Errorf("parsing repo override file %s: %w", entry.Name(), err) + } + + result[entry.Name()] = &r + } + + return result, nil +} + +// peribolosRepoNames extracts the repo names from the parsed peribolos +// config. The peribolos YAML structure is orgs..repos.. +func peribolosRepoNames() map[string]bool { + repos := make(map[string]bool) + for _, orgCfg := range cfg.Orgs { + for repoName := range orgCfg.Repos { + repos[repoName] = true + } + } + return repos +} + +// peribolosRepoFields returns the set of fields configured per repo in +// peribolos.yaml. It re-parses the raw YAML to detect field names +// without relying on Go struct field mappings. +func peribolosRepoFields(configFile string) (map[string]map[string]bool, error) { + data, err := os.ReadFile(configFile) + if err != nil { + return nil, fmt.Errorf("reading peribolos config: %w", err) + } + + var raw map[string]interface{} + if err := yaml.Unmarshal(data, &raw); err != nil { + return nil, fmt.Errorf("parsing peribolos config: %w", err) + } + + result := make(map[string]map[string]bool) + + orgs, ok := raw["orgs"].(map[string]interface{}) + if !ok { + return result, nil + } + + for _, orgData := range orgs { + org, ok := orgData.(map[string]interface{}) + if !ok { + continue + } + + repos, ok := org["repos"].(map[string]interface{}) + if !ok { + continue + } + + for repoName, repoData := range repos { + repoFields, ok := repoData.(map[string]interface{}) + if !ok { + continue + } + + fields := make(map[string]bool) + for fieldName := range repoFields { + fields[fieldName] = true + } + result[repoName] = fields + } + } + + return result, nil +} + +func TestBoundary_SuborgReposExistInPeribolos(t *testing.T) { + if _, err := os.Stat(safeSettingsDir); os.IsNotExist(err) { + t.Skip("safe-settings directory not found, skipping boundary tests") + } + + peribolosRepos := peribolosRepoNames() + suborgs, err := loadSuborgs(safeSettingsDir) + if err != nil { + t.Fatalf("failed to load suborgs: %v", err) + } + + for fileName, repos := range suborgs { + for _, repo := range repos { + if !peribolosRepos[repo] { + t.Errorf("suborg file %s references repo %q which does not exist in peribolos.yaml", fileName, repo) + } + } + } +} + +func TestBoundary_NoDuplicateSuborgMembership(t *testing.T) { + if _, err := os.Stat(safeSettingsDir); os.IsNotExist(err) { + t.Skip("safe-settings directory not found, skipping boundary tests") + } + + suborgs, err := loadSuborgs(safeSettingsDir) + if err != nil { + t.Fatalf("failed to load suborgs: %v", err) + } + + repoToSuborg := make(map[string]string) + for fileName, repos := range suborgs { + for _, repo := range repos { + if existing, ok := repoToSuborg[repo]; ok { + t.Errorf("repo %q appears in both %s and %s", repo, existing, fileName) + } + repoToSuborg[repo] = fileName + } + } +} + +func TestBoundary_SafeSettingsNoPeriblosFields(t *testing.T) { + if _, err := os.Stat(safeSettingsDir); os.IsNotExist(err) { + t.Skip("safe-settings directory not found, skipping boundary tests") + } + + // Check settings.yml + settings, err := loadSettings(safeSettingsDir) + if err != nil { + t.Fatalf("failed to load settings: %v", err) + } + + if settings.Repository != nil { + for _, field := range peribolosOwnedFields { + if _, exists := settings.Repository[field]; exists { + t.Errorf("settings.yml sets peribolos-owned field %q under repository", field) + } + } + } + + // Check suborg files (re-parse as raw YAML for field detection) + suborgsDir := filepath.Join(safeSettingsDir, "suborgs") + entries, err := os.ReadDir(suborgsDir) + if err != nil && !os.IsNotExist(err) { + t.Fatalf("failed to read suborgs directory: %v", err) + } + + for _, entry := range entries { + if entry.IsDir() || !strings.HasSuffix(entry.Name(), ".yml") { + continue + } + + data, err := os.ReadFile(filepath.Join(suborgsDir, entry.Name())) + if err != nil { + t.Fatalf("failed to read suborg file %s: %v", entry.Name(), err) + } + + var raw map[string]interface{} + if err := yaml.Unmarshal(data, &raw); err != nil { + t.Fatalf("failed to parse suborg file %s: %v", entry.Name(), err) + } + + if repo, ok := raw["repository"].(map[string]interface{}); ok { + for _, field := range peribolosOwnedFields { + if _, exists := repo[field]; exists { + t.Errorf("suborg file %s sets peribolos-owned field %q under repository", entry.Name(), field) + } + } + } + } + + // Check repo-level override files + overrides, err := loadRepoOverrides(safeSettingsDir) + if err != nil { + t.Fatalf("failed to load repo overrides: %v", err) + } + + for fileName, override := range overrides { + if override.Repository != nil { + for _, field := range peribolosOwnedFields { + if _, exists := override.Repository[field]; exists { + t.Errorf("repo override file %s sets peribolos-owned field %q under repository", fileName, field) + } + } + } + } +} + +func TestBoundary_PeribolosNoSafeSettingsFields(t *testing.T) { + repoFields, err := peribolosRepoFields(*configPath) + if err != nil { + t.Fatalf("failed to parse peribolos repo fields: %v", err) + } + + for repoName, fields := range repoFields { + for _, forbidden := range safeSettingsOwnedFields { + if fields[forbidden] { + t.Errorf("peribolos.yaml repo %q sets safe-settings-owned field %q", repoName, forbidden) + } + } + } +} + +func TestBoundary_SuborgReposMatchRulesetConditions(t *testing.T) { + if _, err := os.Stat(safeSettingsDir); os.IsNotExist(err) { + t.Skip("safe-settings directory not found, skipping boundary tests") + } + + suborgs, err := loadSuborgs(safeSettingsDir) + if err != nil { + t.Fatalf("failed to load suborgs: %v", err) + } + + settings, err := loadSettings(safeSettingsDir) + if err != nil { + t.Fatalf("failed to load settings: %v", err) + } + + // Build a map of suborg name (without extension) to repo set + suborgRepoSets := make(map[string]map[string]bool) + for fileName, repos := range suborgs { + name := strings.TrimSuffix(fileName, ".yml") + repoSet := make(map[string]bool) + for _, r := range repos { + repoSet[r] = true + } + suborgRepoSets[name] = repoSet + } + + // For each ruleset, check if its repository_name.include matches a + // suborg. The naming convention is "safe-settings: " where + // corresponds to a suborg file name (with hyphens replaced + // by spaces). E.g., "safe-settings: code repos" matches "code-repos.yml". + for _, rs := range settings.Rulesets { + if rs.Conditions.RepositoryName == nil { + continue + } + + rulesetRepos := make(map[string]bool) + for _, r := range rs.Conditions.RepositoryName.Include { + rulesetRepos[r] = true + } + + // Normalize the ruleset name for matching: strip the + // "safe-settings: " prefix, then replace spaces with hyphens + // to match suborg file names (e.g., "code repos" -> "code-repos"). + normalized := strings.TrimPrefix(rs.Name, "safe-settings: ") + normalized = strings.ReplaceAll(normalized, " ", "-") + + for suborgName, suborgRepos := range suborgRepoSets { + if normalized != suborgName { + continue + } + + // Check repos in suborg but not in ruleset + for repo := range suborgRepos { + if !rulesetRepos[repo] { + t.Errorf("repo %q is in suborg %s but missing from ruleset %q repository_name.include", + repo, suborgName+".yml", rs.Name) + } + } + + // Check repos in ruleset but not in suborg + for repo := range rulesetRepos { + if !suborgRepos[repo] { + t.Errorf("repo %q is in ruleset %q repository_name.include but missing from suborg %s", + repo, rs.Name, suborgName+".yml") + } + } + } + } +} diff --git a/openspec/changes/adopt-safe-settings/design.md b/openspec/changes/adopt-safe-settings/design.md index c2520ad..b99a875 100644 --- a/openspec/changes/adopt-safe-settings/design.md +++ b/openspec/changes/adopt-safe-settings/design.md @@ -65,23 +65,32 @@ can only affect that app's scope. Each app gets only the permissions it needs give the safe-settings workflow access to org admin permissions it does not need, and a single key compromise would affect both org membership and repo settings. -### 2. GitHub Actions-only deployment +### 2. GitHub Actions deployment — manual dispatch first -Deploy safe-settings via a GitHub Actions workflow that runs `npm run full-sync` -on three triggers: -- `push` to main — immediate convergence after config changes are merged -- `schedule` daily at 06:00 UTC — drift correction (30 min after peribolos - sync at 05:30 UTC, ensuring membership/teams are applied before repo settings) -- `workflow_dispatch` — manual convergence on demand +Deploy safe-settings via a GitHub Actions workflow that runs `npm run full-sync`. +Initial rollout uses only `workflow_dispatch` (manual dispatch) for full +control during validation. Automated triggers are added after confidence is +established: + +**Initial (this change):** +- `workflow_dispatch` only — manual dispatch with `dry-run` (default true) + and `repos` (optional, comma-separated) inputs + +**Future (separate change, after validation):** +- `push` to main on `safe-settings/**` path changes — convergence after merge +- `schedule` daily at 06:00 UTC — drift correction No webhook listener, no hosting infrastructure, no public endpoint. +safe-settings reads config from the admin repo's default branch via the +GitHub API. Config must be merged to main before the workflow can apply it. +The `repos` input allows targeting specific repos for incremental rollout. + **Rationale:** The complytime org has ~12 repos, 3 admins, and ~24 members. -This is a small org where eventual consistency (daily sync) is acceptable. -The `push`-triggered sync provides near-immediate convergence after config -merges. This model matches the existing peribolos operational pattern -(scheduled + push + manual dispatch), requires zero infrastructure, and -eliminates the attack surface of a public webhook endpoint. +Manual dispatch during initial rollout gives admins full control over when +and what is applied. The `repos` input enables incremental validation +(dry-run one repo, apply one repo, then expand). Automated triggers are +added only after the config is validated against all repos. **Alternative considered:** Webhook-driven deployment (Docker/Lambda) for real-time drift prevention and PR dry-run validation. Rejected because it @@ -235,13 +244,13 @@ project overview and prerequisites. ## Risks / Trade-offs -**[Eventual consistency]** The GHA-only deployment provides daily convergence, -not real-time enforcement. Manual UI changes to repo settings persist until -the next scheduled sync or a manual `workflow_dispatch` trigger. --> Mitigation: The `push` trigger on main provides near-immediate convergence -after config merges. Daily scheduled sync catches drift from manual UI changes. -Acceptable for a small org (~12 repos) where admins are disciplined about -config-as-code workflows. +**[Manual dispatch during initial rollout]** The workflow only runs on manual +dispatch during initial rollout. There is no automated convergence until +`push` and `schedule` triggers are added in a follow-up change. +-> Mitigation: After validating safe-settings against all repos via +`workflow_dispatch`, automated triggers are added. The `repos` input allows +incremental rollout (one repo at a time). Acceptable for a small org +(~12 repos) where admins are disciplined about config-as-code workflows. **[Two tools for org management]** Maintaining peribolos AND safe-settings increases operational complexity. Two config schemas, two deployment pipelines, diff --git a/openspec/changes/adopt-safe-settings/specs/local-development-workflow/spec.md b/openspec/changes/adopt-safe-settings/specs/local-development-workflow/spec.md index 8ef88e5..04b921e 100644 --- a/openspec/changes/adopt-safe-settings/specs/local-development-workflow/spec.md +++ b/openspec/changes/adopt-safe-settings/specs/local-development-workflow/spec.md @@ -21,74 +21,43 @@ under `safe-settings/` using `yamllint`. The target SHALL use the same - **THEN** yamllint reports no errors - **AND** the command exits with status 0 -### Requirement: Local dry-run for safe-settings - -A Makefile target `safe-settings-dryrun` SHALL run safe-settings in dry-run -mode against the live GitHub org. This target SHALL: -- Depend on `ensure-safe-settings` (clone and install if not present) -- Use the Probot-native auth pattern (`APP_ID` + `PRIVATE_KEY` env vars) -- Read credentials from a local file (e.g., - `~/.config/safe-settings/env`) or expect them as environment variables -- Run safe-settings in dry-run mode. The exact dry-run mechanism SHALL be - determined during implementation by consulting the safe-settings - documentation (e.g., `--dry-run` flag, `LOG_LEVEL=debug` for - preview-only output, or equivalent). -- Display what changes would be applied without actually applying them - -#### Scenario: Dry-run shows pending changes - -- **GIVEN** the local safe-settings config differs from the live GitHub state -- **WHEN** `make safe-settings-dryrun` is run with valid credentials -- **THEN** the output shows what settings would be changed on which repos -- **AND** no actual changes are applied to the GitHub org - -### Requirement: Ensure safe-settings binary/environment - -A Makefile target `ensure-safe-settings` SHALL clone the safe-settings -repository and install its Node.js dependencies if not already present. -The target SHALL: -- Clone `github/safe-settings` at a pinned version/tag to a temporary or - cached directory -- Run `npm install` in the cloned directory -- Be idempotent (skip clone and install if already present) -- Mirror the pattern used by `ensure-peribolos` for peribolos - -#### Scenario: First run clones and installs +### Requirement: Lint target covers safe-settings YAML -- **GIVEN** safe-settings has not been cloned locally -- **WHEN** `make ensure-safe-settings` is run -- **THEN** the safe-settings repo is cloned at the pinned version -- **AND** `npm install` completes successfully +The existing `make lint` target SHALL be extended to validate both +`peribolos.yaml` and `safe-settings/**/*.yml` files. This ensures all +org management YAML is linted consistently. -#### Scenario: Subsequent runs are no-ops +#### Scenario: Lint catches error in safe-settings file -- **GIVEN** safe-settings is already cloned and installed -- **WHEN** `make ensure-safe-settings` is run -- **THEN** the target prints a message indicating it is already present -- **AND** does not re-clone or re-install +- **GIVEN** a safe-settings YAML file has a lint violation +- **WHEN** `make lint` is run +- **THEN** yamllint reports the error +- **AND** the command exits with a non-zero status -### Requirement: Node.js version validation +### Requirement: Boundary tests validate config locally -The `ensure-safe-settings` target SHALL validate that the local Node.js -version meets the minimum requirement (>= 18). If the version is below -the minimum, the target SHALL print an error and exit. +The Go tests in `config/boundary_test.go` SHALL be runnable locally +via `make test-unit` without any credentials or network access. These +tests parse the local YAML files and validate cross-tool consistency. -#### Scenario: Node.js version too old +#### Scenario: Local boundary validation -- **GIVEN** the local Node.js version is 16 -- **WHEN** `make ensure-safe-settings` is run -- **THEN** the target prints an error about the minimum version requirement -- **AND** exits with a non-zero status +- **GIVEN** a developer modifies safe-settings config +- **WHEN** they run `make test-unit` +- **THEN** boundary tests validate the config against peribolos.yaml +- **AND** report any violations (field overlap, missing repos, duplicates) +- **AND** no GitHub API calls or credentials are required -### Requirement: Lint target covers safe-settings YAML +### Requirement: Full local validation via make sanity -The existing `make lint` target SHALL be extended (or a new combined target -created) to validate both `peribolos.yaml` and `safe-settings/**/*.yml` -files. This ensures all org management YAML is linted consistently. +The `make sanity` target SHALL include safe-settings YAML validation +as part of its checks (via the extended `make lint` target). Running +`make sanity` SHALL validate both peribolos and safe-settings configs. -#### Scenario: Lint catches error in safe-settings file +#### Scenario: Sanity check covers safe-settings -- **GIVEN** a safe-settings YAML file has a lint violation -- **WHEN** `make lint` is run -- **THEN** yamllint reports the error -- **AND** the command exits with a non-zero status +- **GIVEN** a developer wants to verify all configs before committing +- **WHEN** they run `make sanity` +- **THEN** peribolos.yaml and safe-settings YAML are validated +- **AND** boundary tests are run +- **AND** the command exits with zero status if everything is correct diff --git a/openspec/changes/adopt-safe-settings/specs/safe-settings-deployment/spec.md b/openspec/changes/adopt-safe-settings/specs/safe-settings-deployment/spec.md index 58ff07d..c602d4e 100644 --- a/openspec/changes/adopt-safe-settings/specs/safe-settings-deployment/spec.md +++ b/openspec/changes/adopt-safe-settings/specs/safe-settings-deployment/spec.md @@ -48,68 +48,72 @@ No additional secrets are required (`CLIENT_ID`, `CLIENT_SECRET`, and ### Requirement: GitHub Actions sync workflow A GitHub Actions workflow `safe_settings_sync.yml` SHALL be configured in -the `.github` repo to run `npm run full-sync`. The workflow SHALL be -triggered by: -- `push` to main branch — immediate convergence after config merges -- `schedule` daily at 06:00 UTC — drift correction (30 min after peribolos - sync at 05:30 UTC, ensuring membership/teams are applied before repo - settings) -- `workflow_dispatch` — manual convergence on demand +the `.github` repo to run `npm run full-sync`. The workflow SHALL initially +be triggered only by `workflow_dispatch` (manual dispatch) to allow +controlled rollout and validation. Automated triggers (`push` to main, +`schedule`) SHALL be added in a follow-up change after initial validation +is complete. + +safe-settings reads its config from the admin repo's default branch via +the GitHub API. Config changes must be merged to main before the workflow +can apply them. + +The workflow SHALL accept the following inputs on `workflow_dispatch`: +- `dry-run` — boolean, default `true`. When true, safe-settings runs in + NOP mode (logs what would change without applying). Safe by default. +- `repos` — string, optional. Comma-separated list of repos to target + (e.g., `complytime-demos,community`). When empty, applies to all + managed repos. When provided, the workflow dynamically generates a + scoped `deployment-settings.yml` that restricts safe-settings to only + the specified repos. The workflow SHALL: - Check out the `.github` repo (admin repo with config) -- Check out `github/safe-settings` at a pinned commit SHA (not a mutable tag) to prevent supply chain attacks via tag manipulation. The SHA SHALL be documented in the workflow file. +- Validate YAML syntax via yamllint before running `full-sync` +- Check out `github/safe-settings` at a pinned version (TODO: replace + with commit SHA after initial validation) - Run `npm install` and `npm run full-sync` - Pass environment variables: `APP_ID`, `PRIVATE_KEY`, `GH_ORG=complytime`, - `ADMIN_REPO=.github`, `CONFIG_PATH=safe-settings` -- Use `DEPLOYMENT_CONFIG_FILE` pointing to the workspace-relative path of - `deployment-settings.yml` -- Use a concurrency group with `cancel-in-progress: false` to prevent concurrent sync runs from partially applying settings + `ADMIN_REPO=.github`, `CONFIG_PATH=safe-settings`, `DEPLOYMENT_CONFIG_FILE`, + `FULL_SYNC_NOP` +- Use a concurrency group with `cancel-in-progress: false` to prevent + concurrent sync runs from partially applying settings - Set `timeout-minutes` to 15 to prevent runaway execution -- Include a YAML validation step (yamllint) before running `full-sync` to catch syntax errors before they are applied -The workflow SHALL accept a `dry-run` boolean input on `workflow_dispatch`, -matching the existing peribolos workflow pattern. +#### Scenario: Dry-run against a single repo -The workflow SHALL use a concurrency group to prevent concurrent sync runs. - -#### Scenario: Push-triggered sync after config merge - -- **GIVEN** a PR modifying safe-settings config is merged to main -- **WHEN** the push event triggers the sync workflow -- **THEN** safe-settings runs `full-sync` and applies the updated config - to all managed repos - -#### Scenario: Scheduled drift correction - -- **GIVEN** a user modified repo settings via the GitHub UI -- **WHEN** the daily scheduled sync runs at 06:00 UTC -- **THEN** safe-settings detects the drift and reverts the settings to match - the declared configuration +- **GIVEN** a maintainer wants to preview changes for `complytime-demos` +- **WHEN** they trigger `workflow_dispatch` with `dry-run=true` and + `repos=complytime-demos` +- **THEN** the workflow generates a scoped deployment-settings that + restricts safe-settings to only `complytime-demos` +- **AND** safe-settings runs in NOP mode and logs what would change +- **AND** no actual settings are applied -#### Scenario: Manual convergence via workflow_dispatch +#### Scenario: Apply to a single repo -- **GIVEN** a maintainer needs immediate convergence -- **WHEN** they trigger `workflow_dispatch` on the sync workflow -- **THEN** safe-settings runs `full-sync` and applies all settings +- **GIVEN** a maintainer has verified the dry-run output is correct +- **WHEN** they trigger `workflow_dispatch` with `dry-run=false` and + `repos=complytime-demos` +- **THEN** safe-settings applies settings only to `complytime-demos` +- **AND** other managed repos are not affected -#### Scenario: Workflow dispatch with dry-run option +#### Scenario: Apply to all managed repos -- **GIVEN** a maintainer wants to preview what changes would be applied -- **WHEN** they trigger `workflow_dispatch` with the `dry-run` input set - to `true` -- **THEN** the workflow runs in preview mode and logs what would change -- **AND** no actual settings are applied +- **GIVEN** config changes have been merged to main +- **WHEN** a maintainer triggers `workflow_dispatch` with `dry-run=false` + and `repos` left empty +- **THEN** safe-settings runs `full-sync` and applies settings to all + managed repos #### Scenario: Sync workflow failure - **GIVEN** the safe-settings sync workflow encounters an error (e.g., credential expiry, GitHub API outage, invalid YAML) - **WHEN** the workflow fails -- **THEN** the workflow logs include structured error output -- **AND** no partial settings are applied to some repos while others - are skipped (safe-settings processes each repo independently, so - partial application is possible — document this behavior) +- **THEN** the workflow logs include error output +- **AND** safe-settings processes each repo independently, so partial + application is possible — this behavior is documented in MAINTAINING.md ### Requirement: Config directory in .github repo diff --git a/openspec/changes/adopt-safe-settings/tasks.md b/openspec/changes/adopt-safe-settings/tasks.md index 587fecb..4f319af 100644 --- a/openspec/changes/adopt-safe-settings/tasks.md +++ b/openspec/changes/adopt-safe-settings/tasks.md @@ -2,58 +2,61 @@ - [ ] 1.1 Register a new GitHub App (`safe-settings-bot`) in the complytime org with permissions: Repository Administration (write), Contents (read), Checks (write), Pull requests (write), Organization Administration (read) - [ ] 1.2 Install the app on the complytime org, granting access to all repositories -- [ ] 1.3 Store `SAFE_SETTINGS_APP_ID` as a repository variable and `SAFE_SETTINGS_PRIVATE_KEY` as a repository secret in the `.github` repo (no CLIENT_ID/CLIENT_SECRET needed for GHA-only deployment) +- [ ] 1.3 Store `SAFE_SETTINGS_APP_ID` as a repository variable and `SAFE_SETTINGS_PRIVATE_KEY` as a repository secret in the `.github` repo (no CLIENT_ID/CLIENT_SECRET needed) ## Phase 2: Config Structure - [ ] 2.1 Create `safe-settings/` directory at the repository root (not under `.github/`) - [ ] 2.2 Create `safe-settings/deployment-settings.yml` with `restrictedRepos` excluding `.github`, `admin`, and `safe-settings` repos, and define `overridevalidators` and `configvalidators` - [ ] 2.3 Create `safe-settings/settings.yml` with org-wide defaults: `allow_auto_merge: true`, `delete_branch_on_merge: true`, merge strategies, `has_wiki: false`, security settings (vulnerability alerts, automated fixes) — SHALL NOT include peribolos-owned fields (`description`, `has_projects`, `default_branch`) -- [ ] 2.4 Define org-level rulesets in `safe-settings/settings.yml` for code repos: deletion protection, non-fast-forward, required signatures, PR requirements (dismiss stale reviews, code owner review, last push approval, 1 required approver, thread resolution) +- [ ] 2.4 Define org-level rulesets in `safe-settings/settings.yml` for code repos: deletion protection, non-fast-forward, PR requirements (dismiss stale reviews, code owner review, last push approval, 1 required approver) - [ ] 2.5 Create `safe-settings/suborgs/code-repos.yml` with `suborgrepos` listing: complyctl, complytime-collector-components, complytime-policies, complytime-providers, org-infra -- [ ] 2.6 Create `safe-settings/suborgs/non-code-repos.yml` with `suborgrepos` listing: community, complytime-demos, website, complytime — lighter ruleset (no required signatures, no code owner review) +- [ ] 2.6 Create `safe-settings/suborgs/non-code-repos.yml` with `suborgrepos` listing: community, complytime-demos, website, complytime — lighter ruleset (no code owner review) - [ ] 2.7 Rename the `.github` repo's "verifiy" ruleset to "verify" via the GitHub UI (one-time manual fix; this ruleset remains manually managed) ## Phase 3: Deployment (GitHub Actions) -- [ ] 3.1 Create `safe_settings_sync.yml` workflow in `.github/workflows/` with triggers: `push` to main, daily `schedule` at 06:00 UTC, `workflow_dispatch` -- [ ] 3.2 Configure the workflow to check out both the `.github` repo and `github/safe-settings` at a pinned commit SHA (not a mutable tag) -- [ ] 3.3 Configure the workflow to run `npm install` and `npm run full-sync` with env vars: `APP_ID`, `PRIVATE_KEY`, `GH_ORG=complytime`, `ADMIN_REPO=.github`, `CONFIG_PATH=safe-settings`, `DEPLOYMENT_CONFIG_FILE` -- [ ] 3.4 Add a concurrency group to prevent concurrent sync runs -- [ ] 3.5 Test workflow: trigger `workflow_dispatch`, verify settings are applied to managed repos +- [ ] 3.1 Create `safe_settings_sync.yml` workflow in `.github/workflows/` with `workflow_dispatch` trigger only (push/schedule triggers added after validation) +- [ ] 3.2 Add workflow inputs: `dry-run` (boolean, default true), `repos` (string, optional comma-separated repo filter) +- [ ] 3.3 Add workflow step to generate scoped `deployment-settings.yml` when `repos` input is provided (excludes all repos except targets) +- [ ] 3.4 Configure the workflow to check out both the `.github` repo and `github/safe-settings` at a pinned version +- [ ] 3.5 Configure the workflow to run `npm install` and `npm run full-sync` with env vars: `APP_ID`, `PRIVATE_KEY`, `GH_ORG=complytime`, `ADMIN_REPO=.github`, `CONFIG_PATH=safe-settings`, `DEPLOYMENT_CONFIG_FILE`, `FULL_SYNC_NOP` +- [ ] 3.6 Add concurrency group, timeout (15 min), and YAML pre-validation step -## Phase 4: Dry-Run Validation +## Phase 4: Validation and Guardrails -- [ ] 4.1 Run safe-settings in dry-run mode against all repos to compare declared config vs current GitHub state (via `workflow_dispatch` or local `make safe-settings-dryrun`) -- [ ] 4.2 Review the output — identify any unexpected changes that would be applied -- [ ] 4.3 Adjust config to match desired state (not current state, if current state is wrong) +- [ ] 4.1 Create `config/boundary_test.go` with Go tests that validate: all suborg repos exist in `peribolos.yaml`, no repo in multiple suborgs, no safe-settings config sets peribolos-owned fields, no peribolos config sets safe-settings-owned fields, suborg repo lists match ruleset conditions +- [ ] 4.2 Extend `make lint` to cover `safe-settings/**/*.yml` with yamllint (and add `make safe-settings-validate` target) +- [ ] 4.3 Update CI to run boundary tests on every PR touching `peribolos.yaml` or `safe-settings/**` +- [ ] 4.4 Verify boundary tests run and pass in CI before any PR can merge -## Phase 5: Validation and Guardrails +## Phase 5: Merge and Initial Dry-Run -- [ ] 5.1 Create `config/boundary_test.go` with Go tests that validate: all suborg repos exist in `peribolos.yaml`, no repo in multiple suborgs, no safe-settings config sets peribolos-owned fields, no peribolos config sets safe-settings-owned fields -- [ ] 5.2 Extend `make lint` to cover `safe-settings/**/*.yml` with yamllint (or add `make safe-settings-validate` target) -- [ ] 5.3 Update `validate-peribolos.yml` workflow (or create `validate-safe-settings.yml`) to run boundary tests on every PR touching `peribolos.yaml` or `safe-settings/**` -- [ ] 5.4 Verify boundary tests run and pass in CI before any PR can merge +- [ ] 5.1 Merge the safe-settings config PR to main +- [ ] 5.2 Trigger `workflow_dispatch` with `dry-run=true` and `repos=complytime-demos` — review what would change +- [ ] 5.3 Adjust config if the dry-run reveals unexpected changes, re-merge, re-run -## Phase 6: Apply and Verify +## Phase 6: Incremental Apply and Verify -- [ ] 6.1 Merge the safe-settings config to main — verify push-triggered sync applies settings to all managed repos -- [ ] 6.2 Verify org-level rulesets are created and applied to the correct repos -- [ ] 6.3 Verify repo settings (auto-merge, delete-branch-on-merge, wiki disabled) are applied -- [ ] 6.4 Test drift correction: manually change a repo setting via UI, trigger `workflow_dispatch`, verify safe-settings reverts it -- [ ] 6.5 Test override validator: merge a config that lowers required approvers below org default, verify the sync rejects the change -- [ ] 6.6 Delete legacy repo-level rulesets (listed in settings.yml migration notes) after verifying org-level rulesets work correctly +- [ ] 6.1 Trigger `workflow_dispatch` with `dry-run=false` and `repos=complytime-demos` — apply to one repo +- [ ] 6.2 Verify repo settings and rulesets in the GitHub UI for `complytime-demos` +- [ ] 6.3 Repeat for each repo group: expand `repos` to include more repos progressively +- [ ] 6.4 Trigger `workflow_dispatch` with `dry-run=false` and `repos` empty — apply to all managed repos +- [ ] 6.5 Verify org-level rulesets are created and applied to the correct repos +- [ ] 6.6 Verify repo settings (auto-merge, delete-branch-on-merge, wiki disabled) are applied +- [ ] 6.7 Test drift correction: manually change a repo setting via UI, trigger `workflow_dispatch`, verify safe-settings reverts it +- [ ] 6.8 Test override validator: merge a config that lowers required approvers below org default, verify the sync rejects the change +- [ ] 6.9 Delete legacy repo-level rulesets (listed in settings.yml migration notes) after verifying org-level rulesets work correctly -## Phase 7: Local Development +## Phase 7: Documentation -- [ ] 7.1 Add `ensure-safe-settings` Makefile target — clone `github/safe-settings` at a pinned version and run `npm install` (idempotent, mirrors `ensure-peribolos`) -- [ ] 7.2 Add `safe-settings-validate` Makefile target — run yamllint on all `safe-settings/**/*.yml` files -- [ ] 7.3 Add `safe-settings-dryrun` Makefile target — run `npm run full-sync` locally with credentials from `~/.config/safe-settings/env` or environment variables -- [ ] 7.4 Update `make sanity` to include safe-settings YAML validation +- [ ] 7.1 Create `MAINTAINING.md` at repo root covering: tool boundary table, common workflows (add member, change rulesets, add repo to safe-settings, add override), local validation instructions, workflow_dispatch usage, troubleshooting guide, override validator policies +- [ ] 7.2 Update `README.md` to link to `MAINTAINING.md` for maintainer documentation +- [ ] 7.3 Update `.github/CODEOWNERS` to add path-specific rules for `safe-settings/` requiring admin approval -## Phase 8: Documentation +## Phase 8: Enable Automation (future change) -- [ ] 8.1 Create `MAINTAINING.md` at repo root covering: tool boundary table, common workflows (add member, change rulesets, add repo to safe-settings, add override), local testing instructions, troubleshooting guide, override validator policies -- [ ] 8.2 Update `README.md` to link to `MAINTAINING.md` for maintainer documentation -- [ ] 8.3 Update `.github/CODEOWNERS` to add path-specific rules for `safe-settings/` requiring admin approval +- [ ] 8.1 Add `push` trigger on `safe-settings/**` path changes to main +- [ ] 8.2 Add `schedule` trigger (daily at 06:00 UTC, 30 min after peribolos) for drift correction +- [ ] 8.3 Verify automated triggers work correctly diff --git a/safe-settings/repos/complyctl.yml b/safe-settings/repos/complyctl.yml index bec5752..458c4b5 100644 --- a/safe-settings/repos/complyctl.yml +++ b/safe-settings/repos/complyctl.yml @@ -5,7 +5,7 @@ # This override is applied on top of the code-repos suborg settings, # which inherit from org-wide settings.yml. -# Override the org-level code-repos-default-branch ruleset to require +# Supplement the org-level "safe-settings: code repos" ruleset to require # 2 approvers (org baseline is 1). This matches the current # general-rules ruleset on complyctl (id=2569356). # @@ -14,7 +14,7 @@ # ruleset still applies; GitHub evaluates both and the most restrictive # rule wins. rulesets: - - name: complyctl-extra-review + - name: "safe-settings: complyctl stricter review" target: branch enforcement: active diff --git a/safe-settings/settings.yml b/safe-settings/settings.yml index efadb42..b007c5a 100644 --- a/safe-settings/settings.yml +++ b/safe-settings/settings.yml @@ -68,7 +68,7 @@ rulesets: # Ruleset for code repositories — strict branch protection. # Baseline derived from the complyctl and org-infra rulesets, which # are the most mature in the org. - - name: code-repos-default-branch + - name: "safe-settings: code repos" target: branch enforcement: active @@ -132,7 +132,7 @@ rulesets: # Ruleset for non-code repositories — lighter protection. # Baseline derived from community, complytime-demos, and website # rulesets. - - name: non-code-repos-default-branch + - name: "safe-settings: non-code repos" target: branch enforcement: active From 69f185079a8cac10edd14609f24647e17347b398 Mon Sep 17 00:00:00 2001 From: Marcus Burghardt Date: Thu, 28 May 2026 14:07:29 +0200 Subject: [PATCH 5/5] fix(spec): require Organization Administration write for org rulesets Creating org-level rulesets via POST /orgs/{org}/rulesets requires Organization Administration: write, not read. Update design doc, deployment spec, and task list to reflect the correct permission. Addresses PR #114 review feedback from @sonupreetam. Signed-off-by: Marcus Burghardt Assisted-by: OpenCode (claude-opus-4-6) --- openspec/changes/adopt-safe-settings/design.md | 2 +- .../specs/safe-settings-deployment/spec.md | 6 +++--- openspec/changes/adopt-safe-settings/tasks.md | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/openspec/changes/adopt-safe-settings/design.md b/openspec/changes/adopt-safe-settings/design.md index b99a875..67971bf 100644 --- a/openspec/changes/adopt-safe-settings/design.md +++ b/openspec/changes/adopt-safe-settings/design.md @@ -59,7 +59,7 @@ can only affect that app's scope. Each app gets only the permissions it needs - Repository: Contents (read) — for reading config files from admin repo - Repository: Checks (write) — for PR dry-run validation results - Repository: Pull requests (write) — for PR comments with change previews -- Organization: Administration (read) — for reading org-level rulesets +- Organization: Administration (read and write) — for managing org-level rulesets **Alternative considered:** Extend complytime-bot. Rejected because it would give the safe-settings workflow access to org admin permissions it does not need, diff --git a/openspec/changes/adopt-safe-settings/specs/safe-settings-deployment/spec.md b/openspec/changes/adopt-safe-settings/specs/safe-settings-deployment/spec.md index c602d4e..0fc39be 100644 --- a/openspec/changes/adopt-safe-settings/specs/safe-settings-deployment/spec.md +++ b/openspec/changes/adopt-safe-settings/specs/safe-settings-deployment/spec.md @@ -12,10 +12,10 @@ The app SHALL have the following permissions: - Repository Contents: read - Repository Checks: write - Repository Pull requests: write -- Organization Administration: read +- Organization Administration: read and write -The app SHALL NOT have Organization Members, Organization Administration write, -or any other permissions not listed above. +The app SHALL NOT have Organization Members or any other permissions not +listed above. #### Scenario: App installed with correct permissions diff --git a/openspec/changes/adopt-safe-settings/tasks.md b/openspec/changes/adopt-safe-settings/tasks.md index 4f319af..191176f 100644 --- a/openspec/changes/adopt-safe-settings/tasks.md +++ b/openspec/changes/adopt-safe-settings/tasks.md @@ -1,6 +1,6 @@ ## Phase 1: GitHub App Setup -- [ ] 1.1 Register a new GitHub App (`safe-settings-bot`) in the complytime org with permissions: Repository Administration (write), Contents (read), Checks (write), Pull requests (write), Organization Administration (read) +- [ ] 1.1 Register a new GitHub App (`safe-settings-bot`) in the complytime org with permissions: Repository Administration (write), Contents (read), Checks (write), Pull requests (write), Organization Administration (read and write) - [ ] 1.2 Install the app on the complytime org, granting access to all repositories - [ ] 1.3 Store `SAFE_SETTINGS_APP_ID` as a repository variable and `SAFE_SETTINGS_PRIVATE_KEY` as a repository secret in the `.github` repo (no CLIENT_ID/CLIENT_SECRET needed)