From f2a24be330d621cfc9ca65a2d9d3921b1ae098f4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Va=C5=A1ko?= Date: Fri, 7 Aug 2026 09:31:34 +0200 Subject: [PATCH 1/3] feat(skill): add kbagent-promotion-pipeline skill for source->dest project promotion Adds a from-scratch generator (plugins/kbagent/skills/kbagent-promotion-pipeline/) for a GitHub Actions pipeline that promotes Keboola configs between two distinct projects (e.g. dev -> prod), for the "one repo covers the whole org" pattern. Mechanic (kbagent-native, no git-branching or GH-Environment-per-branch magic needed): a pull workflow fetches the SOURCE project into a shared directory and opens one PR against main; a validate workflow runs `sync push --dry-run` against the DESTINATION project on that PR, showing exactly what would change there; a push workflow (environment-gated) ships it once the PR merges. Every step uses KBAGENT_PROJECT_FROM_ENV=1 + --project __env__, so no token ever touches disk. A single repo can host several independent pipelines via a JSON config (one entry per source/dest pair). Generator is stdlib-only, mirrors the kbagent-cicd-migration skill's structure; verified by generating both single- and multi-pipeline configs and inline sanity assertions on the rendered YAML. --- .../kbagent-promotion-pipeline/SKILL.md | 157 +++++++ .../references/env-injection.md | 37 ++ .../references/secrets-setup.md | 59 +++ .../scripts/generate_promotion_pipeline.py | 394 ++++++++++++++++++ 4 files changed, 647 insertions(+) create mode 100644 plugins/kbagent/skills/kbagent-promotion-pipeline/SKILL.md create mode 100644 plugins/kbagent/skills/kbagent-promotion-pipeline/references/env-injection.md create mode 100644 plugins/kbagent/skills/kbagent-promotion-pipeline/references/secrets-setup.md create mode 100644 plugins/kbagent/skills/kbagent-promotion-pipeline/scripts/generate_promotion_pipeline.py diff --git a/plugins/kbagent/skills/kbagent-promotion-pipeline/SKILL.md b/plugins/kbagent/skills/kbagent-promotion-pipeline/SKILL.md new file mode 100644 index 00000000..655ad4ab --- /dev/null +++ b/plugins/kbagent/skills/kbagent-promotion-pipeline/SKILL.md @@ -0,0 +1,157 @@ +--- +name: kbagent-promotion-pipeline +description: > + Use when setting up a from-scratch GitHub Actions pipeline that promotes + Keboola configurations from a SOURCE project (e.g. dev) to a DESTINATION + project (e.g. prod) using kbagent sync -- one GitHub repo covering the + whole org, main branch as the reviewable source of truth. Covers: PR-based + promotion (pull from source opens a PR, merging pushes to destination), + cross-project diff before merge, multi-pipeline repos (several independent + source/destination pairs in one repo), and GitHub secrets/environment + setup. Triggers: promote config between projects, dev to prod pipeline, + source project destination project, propagate changes between Keboola + projects, kbagent promotion workflow, cross-project sync GitHub Actions, + set up project promotion CI/CD. +--- + +# kbagent Source -> Destination Promotion Pipeline + +Generates a **from-scratch** GitHub Actions setup (not a migration -- see +[kbagent-cicd-migration](../kbagent-cicd-migration/SKILL.md) for porting an +existing `kbc` repo) that promotes Keboola configuration changes from a named +**source project** to a named **destination project**, with a human-reviewed +PR gate in between. + +## The mechanic + +kbagent's `sync` targets one registered project alias per invocation +(`--project ALIAS`) -- it has no "this git branch is bound to that project" +magic the way some kbc-era setups do. This skill builds the promotion loop +directly out of that primitive, using one shared directory per pipeline and +two Storage API tokens (source, destination): + +1. **Pull** (`kbagent-promote-pull.yml`, manual + optional schedule) runs + `sync pull --project __env__ --directory --force` against the + **source** project's token, for every configured pipeline, then opens (or + updates) **one PR** against `main` with the combined diff via + [`peter-evans/create-pull-request`](https://github.com/peter-evans/create-pull-request). +2. **Validate** (`kbagent-promote-validate.yml`, on the PR) runs + `sync push --dry-run --project __env__ --directory ` against the + **destination** project's token, for every pipeline touched by the PR -- + this is the cross-project diff: *if this PR merges, here is exactly what + changes in the destination project.* Read this before approving. +3. **Push** (`kbagent-promote-push.yml`, on push to `main`) runs + `sync push --project __env__ --directory ` against the + **destination** project's token, gated by the `prod` GitHub Environment + (add required reviewers there for manual approval even though the trigger + is an automatic push-on-merge, not `workflow_dispatch`). + +`main` therefore always represents "the last thing approved and pushed to +every destination project" -- the reviewable source of truth the whole repo +is built around. A promotion is: pull opens a PR -> validate shows the +destination-side diff -> a human approves and merges -> push ships it. + +Every step uses `KBAGENT_PROJECT_FROM_ENV=1` + the reserved `--project __env__` +alias (kbagent's headless/CI auth model) -- no token is ever written to +`config.json` or committed to the repo. See +[references/env-injection.md](references/env-injection.md) if you need the +background on why this exists. + +## One repo, multiple independent pipelines + +A single repo can host several unrelated promotion pipelines (e.g. one per +data source, or one per business unit) -- each is a +`{name, directory, source_stack_url, dest_stack_url}` tuple, all generated +into the same three workflow files as extra per-pipeline steps. Use `--config +pipelines.json` (a JSON list of these tuples) for more than one; the +single-pipeline CLI flags (`--name`/`--directory`/`--source-stack-url`/ +`--dest-stack-url`) are a shortcut for exactly one. + +## How to run this -- ask the customer, don't auto-pilot + +Same discipline as every other skill that touches a customer's live +Keboola projects and their CI/CD: **stop and ask** before you: +- Pick the version pin (Step 2) -- prod vs. scratch lane changes the answer. +- Run `--write` (Step 3) -- show the dry-run inventory first. +- Perform the one-time bootstrap (Step 4) against a real destination + project -- confirm which project is genuinely production before seeding + `main` from it. +- Set up secrets/environments (Step 5) -- these are the customer's + credentials, not yours to generate blindly. + +## Workflow + +### Step 1 -- Gather the pipeline definition(s) +For each pipeline: a name, the directory to sync, and the source + destination +projects' stack URLs (usually the same stack, different project ids -- the +project id itself comes from the token, not a CLI flag). Ask for a config +file up front if there's more than one pipeline; it's much easier to review +as a single JSON list than to re-run the generator repeatedly. + +### Step 2 -- Pick a version pin (decide before generating) +Same guidance as the migration skill: `--version X.Y.Z` (PyPI) pinned for a +prod lane, unpinned only for a scratch/experiment repo. + +### Step 3 -- Generate the workflows (dry-run first) +```bash +# Inspect what would be generated: +python /scripts/generate_promotion_pipeline.py /path/to/repo \ + --name SALESFORCE --directory salesforce \ + --source-stack-url connection.keboola.com \ + --dest-stack-url connection.keboola.com + +# Then, once reviewed, write the files: +python /scripts/generate_promotion_pipeline.py /path/to/repo --write \ + --config pipelines.json --version X.Y.Z --schedule "0 6 * * 1" +``` +Produces `.github/workflows/kbagent-promote-{pull,validate,push}.yml` and +prints the exact `gh secret set` / `gh api` commands for Step 5. + +### Step 4 -- Bootstrap `main` from the destination project (one-time, per pipeline) +`main` should start out representing what's *already live* in the +destination project, not an empty tree -- otherwise the first promotion PR +would show every single config as "new," which is both wrong and a scary +first review. Locally, with the destination project's token: +```bash +export KBAGENT_PROJECT_FROM_ENV=1 KBC_TOKEN= KBC_STORAGE_API_URL= +kbagent sync init --project __env__ --directory +kbagent sync pull --project __env__ --directory +git add && git commit -m "Bootstrap from destination project" && git push +``` +Do this directly on `main`, not through a PR -- there is nothing to review +yet, it's just establishing the starting baseline. + +### Step 5 -- Set up GitHub secrets and the `prod` environment +Two Storage API token secrets per pipeline (`KBC_TOKEN__SOURCE`, +`KBC_TOKEN__DEST`) plus the `prod` GitHub Environment with required +reviewers -- the generator prints the exact `gh` commands. See +[references/secrets-setup.md](references/secrets-setup.md). + +### Step 6 -- Run a promotion end-to-end +1. Trigger `kbagent-promote-pull.yml` (`workflow_dispatch`, or wait for the + schedule) -- it opens/updates the `promote/update` PR against `main`. +2. Read the `kbagent-promote-validate.yml` check's dry-run output on that + PR -- confirm it matches what you expect to land in each destination + project. +3. Merge the PR. `kbagent-promote-push.yml` fires, waits for `prod` + environment approval, then pushes to every pipeline's destination + project. + +## Guardrails (state these to the user) +- **Never** add `--allow-plaintext-on-encrypt-failure` to the push workflow -- + it silently uploads `#`-secrets in cleartext if the Encryption API is down. +- The `prod` environment's required-reviewer gate applies to `push`-triggered + jobs too, not just `workflow_dispatch` -- confirm the reviewers are actually + configured, since a repo without them makes the "gate" a no-op. +- One PR covers every pipeline pulled in that run (`branch: promote/update`). + If pipelines are unrelated and reviewed by different people, consider + splitting them into separate repos or separate pull workflows instead of + forcing one combined review. +- `peter-evans/create-pull-request` is a third-party action -- pin it to a + full commit SHA (not just `@v7`) for a security-sensitive prod pipeline, + and mention this to the customer rather than silently leaving the tag pin. + +## Reference material +- [references/secrets-setup.md](references/secrets-setup.md) -- GitHub secrets/environment setup with `gh` commands. +- [references/env-injection.md](references/env-injection.md) -- why `KBAGENT_PROJECT_FROM_ENV`/`__env__` exists and how it differs from a registered `project add`. +- `scripts/generate_promotion_pipeline.py` -- the generator (stdlib only). diff --git a/plugins/kbagent/skills/kbagent-promotion-pipeline/references/env-injection.md b/plugins/kbagent/skills/kbagent-promotion-pipeline/references/env-injection.md new file mode 100644 index 00000000..1a986ef0 --- /dev/null +++ b/plugins/kbagent/skills/kbagent-promotion-pipeline/references/env-injection.md @@ -0,0 +1,37 @@ +# Why every step uses `KBAGENT_PROJECT_FROM_ENV` / `__env__` + +kbagent's normal mode of operation is a **registered project**: `kbagent +project add --project ALIAS --url URL --token TOKEN` writes the token into +`~/.config/keboola-agent-cli/config.json`, and every later command references +that alias. That's the right model for a developer's own machine, but wrong +for CI: it means a token would have to be written to disk (or the config +file would have to be committed, which is worse -- a secret in git history). + +Since 0.50.0, kbagent supports a headless alternative purpose-built for this: +set `KBAGENT_PROJECT_FROM_ENV=1` together with `KBC_TOKEN` and +`KBC_STORAGE_API_URL`, and kbagent synthesizes an **in-memory** project under +the reserved alias `__env__` for that process only -- no `project add`, no +`config.json` write, nothing to clean up afterward. Every command in this +skill's generated workflows passes `--project __env__` for exactly this +reason. + +## Two tokens, two projects, same alias name + +Because `__env__` is resolved from whatever `KBC_TOKEN` / +`KBC_STORAGE_API_URL` happen to be set in the current step's `env:` block, +the **same alias name** (`__env__`) can point at two completely different +physical Keboola projects across two steps in the same job -- the pull step +sets the source project's token, the validate/push steps set the destination +project's token. There is no conflict because each step's environment is +isolated; kbagent never persists what `__env__` resolved to. + +## What this buys you + +- The token is a GitHub Actions secret, masked in logs, never written to a + file kbagent (or a subsequent step) could accidentally commit. +- No `project add`/`project remove` housekeeping in CI -- the "project" + exists only for the duration of one step. +- The same generated workflow works identically whether the source and + destination happen to be on the same Keboola stack or different ones -- + `KBC_STORAGE_API_URL` is set explicitly per step from the pipeline + definition, not inferred from a registered project's stored URL. diff --git a/plugins/kbagent/skills/kbagent-promotion-pipeline/references/secrets-setup.md b/plugins/kbagent/skills/kbagent-promotion-pipeline/references/secrets-setup.md new file mode 100644 index 00000000..950a876e --- /dev/null +++ b/plugins/kbagent/skills/kbagent-promotion-pipeline/references/secrets-setup.md @@ -0,0 +1,59 @@ +# GitHub secrets / environment setup + +Each pipeline needs **two** Storage API token secrets -- one for the source +project, one for the destination project -- plus one shared `prod` +GitHub Environment used for push approval gating across every pipeline. + +## Per pipeline + +| Secret | Used by | Project | +|---|---|---| +| `KBC_TOKEN__SOURCE` | `kbagent-promote-pull.yml` | Source (e.g. dev) | +| `KBC_TOKEN__DEST` | `kbagent-promote-validate.yml`, `kbagent-promote-push.yml` | Destination (e.g. prod) | + +`` is the pipeline's `name`, uppercased and sanitized to +`[A-Za-z0-9_]` (the generator's `Pipeline.label` property) -- it always +matches what `generate_promotion_pipeline.py` prints in its secrets report, +so copy-paste from there rather than re-deriving it by hand. + +## Setup with `gh` + +```bash +REPO=/ + +# Per pipeline (repeat for each): +gh secret set KBC_TOKEN_SALESFORCE_SOURCE --repo "$REPO" # paste the dev project's token +gh secret set KBC_TOKEN_SALESFORCE_DEST --repo "$REPO" # paste the prod project's token + +# Shared push-approval environment (once per repo): +gh api -X PUT "repos/$REPO/environments/prod" +``` + +Then in the GitHub UI (or via the environments API): +1. Add **required reviewers** to the `prod` environment. This is what + actually makes `kbagent-promote-push.yml` block on approval -- the + `environment: prod` line in the generated workflow is a no-op without + reviewers configured. +2. Optionally restrict the `prod` environment to the `main` branch only. +3. Scope the `*_DEST` secrets to the `prod` environment if your org's policy + requires environment-scoped secrets (recommended for genuinely + production-facing tokens). + +## Why no token in config.json + +kbagent can read a committed `.kbagent/config.json` with registered project +aliases, but that file stores tokens on disk -- unsafe to commit. Every +generated workflow step instead sets `KBAGENT_PROJECT_FROM_ENV=1` + +`KBC_TOKEN` + `KBC_STORAGE_API_URL` for that one step only, so the token +exists solely as a masked GitHub secret in the runner's environment, never +written to a file. + +## Security guardrails + +- Do **not** commit `.kbagent/config.json` with tokens. +- Do **not** pass `--allow-plaintext-on-encrypt-failure` in CI. +- Prefer environment-scoped `*_DEST` secrets and required reviewers for any + pipeline whose destination is a genuinely production project. +- Pin `peter-evans/create-pull-request` to a full commit SHA, not just a + version tag, for a prod-adjacent pipeline (third-party action supply-chain + hygiene). diff --git a/plugins/kbagent/skills/kbagent-promotion-pipeline/scripts/generate_promotion_pipeline.py b/plugins/kbagent/skills/kbagent-promotion-pipeline/scripts/generate_promotion_pipeline.py new file mode 100644 index 00000000..935c0114 --- /dev/null +++ b/plugins/kbagent/skills/kbagent-promotion-pipeline/scripts/generate_promotion_pipeline.py @@ -0,0 +1,394 @@ +#!/usr/bin/env python3 +"""Generate a kbagent-native source-project -> destination-project promotion pipeline. + +This is a from-scratch generator (no existing repo to migrate) for the "one GitHub +repo covers the whole org" pattern: one or more named pipelines, each syncing a +directory between a SOURCE Keboola project (e.g. dev) and a DESTINATION project +(e.g. prod). It emits three GitHub Actions workflows: + + 1. kbagent-promote-pull.yml (workflow_dispatch + optional schedule) + Pulls every pipeline's directory from its SOURCE project and opens/updates + one PR against the main branch with the combined diff. + 2. kbagent-promote-validate.yml (pull_request against main) + For every pipeline, runs `sync push --dry-run` against the DESTINATION + project -- this is the cross-project diff: "if this PR merges, here is + exactly what changes in the destination project." + 3. kbagent-promote-push.yml (push to main, environment-gated) + Pushes every pipeline's directory to its DESTINATION project once the PR + has merged. + +Each pipeline needs two Storage API token secrets (`KBC_TOKEN__SOURCE` / +`KBC_TOKEN__DEST`) and uses kbagent's `KBAGENT_PROJECT_FROM_ENV=1` / +`__env__` env-injection model -- no token is ever committed to the repo. + +Stdlib only. Dry-run by default; pass ``--write`` to write files. + +Usage: + # Single pipeline via flags: + python generate_promotion_pipeline.py --write \\ + --name SALESFORCE --directory salesforce \\ + --source-stack-url https://connection.keboola.com \\ + --dest-stack-url https://connection.keboola.com \\ + --version X.Y.Z + + # Multiple pipelines (whole-org repo) via a JSON config: + python generate_promotion_pipeline.py --write --config pipelines.json --version X.Y.Z + +pipelines.json shape: + [ + {"name": "SALESFORCE", "directory": "salesforce", + "source_stack_url": "https://connection.keboola.com", + "dest_stack_url": "https://connection.keboola.com"}, + {"name": "GA4", "directory": "ga4", + "source_stack_url": "https://connection.keboola.com", + "dest_stack_url": "https://connection.keboola.com"} + ] +""" + +from __future__ import annotations + +import argparse +import json +import re +import sys +from dataclasses import dataclass +from pathlib import Path + +# --------------------------------------------------------------------------- # +# Pipeline definition +# --------------------------------------------------------------------------- # + + +@dataclass +class Pipeline: + """One source-project -> destination-project promotion pipeline.""" + + name: str + directory: str + source_stack_url: str + dest_stack_url: str + + @property + def label(self) -> str: + return re.sub(r"[^A-Za-z0-9]+", "_", self.name).strip("_").upper() or "PIPELINE" + + @property + def source_token_secret(self) -> str: + return f"KBC_TOKEN_{self.label}_SOURCE" + + @property + def dest_token_secret(self) -> str: + return f"KBC_TOKEN_{self.label}_DEST" + + +def _load_pipelines(args: argparse.Namespace) -> list[Pipeline]: + if args.config: + data = json.loads(Path(args.config).read_text(encoding="utf-8")) + return [ + Pipeline( + name=str(p["name"]), + directory=str(p["directory"]), + source_stack_url=_normalize_url(str(p["source_stack_url"])), + dest_stack_url=_normalize_url(str(p["dest_stack_url"])), + ) + for p in data + ] + missing = [ + flag + for flag, val in ( + ("--name", args.name), + ("--directory", args.directory), + ("--source-stack-url", args.source_stack_url), + ("--dest-stack-url", args.dest_stack_url), + ) + if not val + ] + if missing: + print( + f"error: --config or all of {', '.join(missing)} must be provided", + file=sys.stderr, + ) + sys.exit(2) + return [ + Pipeline( + name=args.name, + directory=args.directory, + source_stack_url=_normalize_url(args.source_stack_url), + dest_stack_url=_normalize_url(args.dest_stack_url), + ) + ] + + +def _normalize_url(host: str) -> str: + host = host.strip() + if host.startswith(("http://", "https://")): + return host + return f"https://{host}" + + +# --------------------------------------------------------------------------- # +# Workflow generation +# --------------------------------------------------------------------------- # + + +def _install_steps(version: str | None, git_ref: str | None) -> str: + if git_ref: + spec = f"git+https://github.com/keboola/cli@{git_ref}" + elif version: + spec = f"keboola-cli=={version}" + else: + spec = "keboola-cli" + return ( + " - name: Install uv\n" + " uses: astral-sh/setup-uv@v5\n" + " - name: Install kbagent\n" + f" run: uv tool install '{spec}'\n" + " - name: Show version\n" + " run: kbagent version\n" + ) + + +def _pipeline_step( + p: Pipeline, + step_name: str, + command: str, + token_secret: str, + stack_url: str, + json_output: bool = False, +) -> str: + prefix = "kbagent --json " if json_output else "kbagent " + return ( + f" - name: {step_name} ({p.name})\n" + " env:\n" + ' KBAGENT_PROJECT_FROM_ENV: "1"\n' + f" KBC_TOKEN: ${{{{ secrets.{token_secret} }}}}\n" + f" KBC_STORAGE_API_URL: {stack_url}\n" + " run: |\n" + f" {prefix}sync {command} --project __env__ --directory '{p.directory}'\n" + ) + + +def gen_pull(pipelines: list[Pipeline], schedule: str | None, main_branch: str) -> str: + on_block = " workflow_dispatch:\n" + if schedule: + on_block += f" schedule:\n - cron: '{schedule}'\n" + steps = "".join( + _pipeline_step(p, "Pull", "pull --force", p.source_token_secret, p.source_stack_url) + for p in pipelines + ) + paths = ", ".join(p.directory for p in pipelines) + return ( + "# Generated by kbagent-promotion-pipeline. Pulls every pipeline's SOURCE\n" + "# project and opens/updates one PR against the main branch.\n" + "name: kbagent promote - pull\n" + "on:\n" + f"{on_block}" + "permissions:\n" + " contents: write\n" + " pull-requests: write\n" + "jobs:\n" + " pull:\n" + " runs-on: ubuntu-latest\n" + " steps:\n" + " - uses: actions/checkout@v4\n" + f"{_INSTALL_TOKEN}" + f"{steps}" + " - name: Open promotion PR\n" + " uses: peter-evans/create-pull-request@v7\n" + " with:\n" + " branch: promote/update\n" + f" base: {main_branch}\n" + ' commit-message: "kbagent promote: pull latest config from source project(s)"\n' + ' title: "Promote: pull latest config from source project(s)"\n' + " body: |\n" + " Automated pull from the source project(s) for:\n" + f" {paths}\n\n" + " Review the diff, then merge to push it to the destination\n" + " project(s) -- see the validate check on this PR for the exact\n" + " destination-side change preview.\n" + ) + + +def gen_validate(pipelines: list[Pipeline]) -> str: + steps = "".join( + _pipeline_step( + p, + "Destination dry-run", + "push --dry-run", + p.dest_token_secret, + p.dest_stack_url, + json_output=True, + ) + for p in pipelines + ) + paths = "\n".join(f" - '{p.directory}/**'" for p in pipelines) + return ( + "# Generated by kbagent-promotion-pipeline. Cross-project diff: shows\n" + "# exactly what merging this PR would change in each DESTINATION project.\n" + "name: kbagent promote - validate\n" + "on:\n" + " pull_request:\n" + " paths:\n" + f"{paths}\n" + "permissions:\n" + " contents: read\n" + "jobs:\n" + " validate:\n" + " runs-on: ubuntu-latest\n" + " steps:\n" + " - uses: actions/checkout@v4\n" + f"{_INSTALL_TOKEN}" + f"{steps}" + ) + + +def gen_push(pipelines: list[Pipeline], main_branch: str) -> str: + steps = "".join( + _pipeline_step(p, "Push", "push", p.dest_token_secret, p.dest_stack_url) for p in pipelines + ) + paths = "\n".join(f" - '{p.directory}/**'" for p in pipelines) + return ( + "# Generated by kbagent-promotion-pipeline. Pushes every pipeline's\n" + "# directory to its DESTINATION project once merged to main.\n" + "# Gated by the 'prod' GitHub Environment -- add required reviewers there\n" + "# for manual approval even though the trigger is an automatic push.\n" + "name: kbagent promote - push\n" + "on:\n" + " push:\n" + f" branches: [{main_branch}]\n" + " paths:\n" + f"{paths}\n" + "permissions:\n" + " contents: read\n" + "jobs:\n" + " push:\n" + " environment: prod\n" + " runs-on: ubuntu-latest\n" + " steps:\n" + " - uses: actions/checkout@v4\n" + f"{_INSTALL_TOKEN}" + " # `sync push` encrypts #-secrets fail-closed by default. Do NOT add\n" + " # --allow-plaintext-on-encrypt-failure in CI.\n" + f"{steps}" + ) + + +_INSTALL_TOKEN = "@@INSTALL@@\n" + + +# --------------------------------------------------------------------------- # +# Secrets checklist +# --------------------------------------------------------------------------- # + + +def secrets_report(pipelines: list[Pipeline], repo_slug: str) -> str: + lines: list[str] = [] + lines.append("Required GitHub secrets (one SOURCE + one DEST token per pipeline):") + for p in pipelines: + lines.append( + f" gh secret set {p.source_token_secret} --repo {repo_slug} # {p.name} source project" + ) + lines.append( + f" gh secret set {p.dest_token_secret} --repo {repo_slug} # {p.name} destination project" + ) + lines.append("") + lines.append("Required GitHub Environment (for push approval gating):") + lines.append(f" gh api -X PUT repos/{repo_slug}/environments/prod") + lines.append(" # Then add required reviewers to 'prod' in the GitHub UI.") + return "\n".join(lines) + + +def _guess_repo_slug(repo: Path) -> str: + config = repo / ".git" / "config" + if config.exists(): + m = re.search( + r"github\.com[:/]([^/\s]+/[^/\s]+?)(?:\.git)?\s*$", + config.read_text(errors="ignore"), + re.MULTILINE, + ) + if m: + return m.group(1) + return "/" + + +# --------------------------------------------------------------------------- # +# Orchestration +# --------------------------------------------------------------------------- # + + +def run(args: argparse.Namespace) -> int: + repo = Path(args.repo_dir).resolve() + if not repo.is_dir(): + print(f"error: {repo} is not a directory", file=sys.stderr) + return 2 + + pipelines = _load_pipelines(args) + + print(f"{len(pipelines)} promotion pipeline(s):") + for p in pipelines: + print(f" - {p.name:<12} directory={p.directory}") + print(f" source: {p.source_stack_url} (secret {p.source_token_secret})") + print(f" dest: {p.dest_stack_url} (secret {p.dest_token_secret})") + + install = _install_steps(args.version, args.git_ref) + files = { + ".github/workflows/kbagent-promote-pull.yml": gen_pull( + pipelines, args.schedule, args.main_branch + ), + ".github/workflows/kbagent-promote-validate.yml": gen_validate(pipelines), + ".github/workflows/kbagent-promote-push.yml": gen_push(pipelines, args.main_branch), + } + files = {k: v.replace(_INSTALL_TOKEN, install) for k, v in files.items()} + + print(f"\nGenerated workflows ({'WRITING' if args.write else 'dry-run, use --write'}):") + for rel, content in files.items(): + target = repo / rel + print(f" - {rel} ({len(content.splitlines())} lines)") + if args.write: + target.parent.mkdir(parents=True, exist_ok=True) + target.write_text(content, encoding="utf-8") + + print("\n" + "=" * 70) + print(secrets_report(pipelines, _guess_repo_slug(repo))) + print("=" * 70) + + if not args.write: + print("\nDry-run only. Re-run with --write to create the files above.") + return 0 + + +def main(argv: list[str] | None = None) -> int: + ap = argparse.ArgumentParser( + description=__doc__, formatter_class=argparse.RawDescriptionHelpFormatter + ) + ap.add_argument("repo_dir", help="Path to the git repo to generate workflows into") + ap.add_argument( + "--write", action="store_true", help="Write the generated workflows (default: dry-run)" + ) + ap.add_argument("--config", help="JSON file with a list of pipeline definitions") + ap.add_argument("--name", help="Pipeline name (single-pipeline mode)") + ap.add_argument("--directory", help="Directory to sync (single-pipeline mode)") + ap.add_argument( + "--source-stack-url", help="Source project's stack URL/host (single-pipeline mode)" + ) + ap.add_argument( + "--dest-stack-url", help="Destination project's stack URL/host (single-pipeline mode)" + ) + grp = ap.add_mutually_exclusive_group() + grp.add_argument("--version", help="Pin kbagent to this PyPI version, e.g. X.Y.Z") + grp.add_argument("--git-ref", help="Pin kbagent to a git tag/ref, e.g. vX.Y.Z") + ap.add_argument( + "--main-branch", + default="main", + help="Branch promotion PRs merge into and that triggers the push workflow (default: main)", + ) + ap.add_argument( + "--schedule", + default=None, + help="Cron for scheduled pulls, e.g. '0 6 * * 1' (default: none)", + ) + return run(ap.parse_args(argv)) + + +if __name__ == "__main__": + raise SystemExit(main()) From 0994c391668f96c9541c84d6bdfa460f849ff582 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Va=C5=A1ko?= Date: Mon, 10 Aug 2026 12:45:54 +0200 Subject: [PATCH 2/3] fix(skill): fix promotion-pipeline convergence, approval isolation, and PR-trigger blockers Independent code-quality + OWASP/security review found two blockers and two majors that would have broken this skill in production. All fixed: - BLOCKER: `sync pull --directory ` against the SOURCE token overwrote 's manifest (bootstrapped once from DEST, Step 4) with SOURCE's config IDs, so every subsequent `sync push` to DEST failed to match existing configs by ID and created duplicates on every promotion cycle instead of converging. Fixed: pull now targets a throwaway scratch directory and only the *content* (never .keboola/manifest.json) is merged into the tracked directory, keeping DEST's ID mapping stable forever. - BLOCKER: `peter-evans/create-pull-request` opened PRs with the default GITHUB_TOKEN, which GitHub deliberately excludes from triggering pull_request workflows -- kbagent-promote-validate.yml never ran, so reviewers approved blind with no destination-side diff. Fixed: PR-open step now requires a PROMOTION_PR_TOKEN PAT, documented in secrets-setup.md. - MAJOR: every pipeline's push ran as a *step* inside one shared job, so one approval click unlocked every pipeline's destination push at once, undermining "a human approves this specific promotion." Fixed: push.yml now generates one job per pipeline, each with its own environment: prod -- GitHub's approval gate is per job run, not per environment name, so each pipeline now needs its own separate approval and one pipeline failing no longer silently blocks the others (no shared job, no needs: dependency). - HIGH (security): #-secret values were never called out as project-scope-bound ciphertext that can't decrypt across projects -- documented explicitly in secrets-setup.md and the Guardrails section. - MEDIUM (security): documented the missing required-status-check step so merging can't bypass the validate gate. - LOW (security): added input validation rejecting unsafe directory/ stack_url values and a pipeline-name label-collision guard, both of which would otherwise corrupt generated YAML/shell quoting. - Removed a redundant `sync init` before `sync pull` in the Step 4 bootstrap instructions (pull auto-inits). Added tests/test_generate_promotion_pipeline.py (12 cases) covering the new scratch-pull mechanic, per-pipeline job isolation, and the validation guards -- this generator had no test coverage before. Co-Authored-By: Claude Sonnet 5 --- .../kbagent-promotion-pipeline/SKILL.md | 75 +++++-- .../references/secrets-setup.md | 69 +++++- .../scripts/generate_promotion_pipeline.py | 205 ++++++++++++++---- tests/test_generate_promotion_pipeline.py | 107 +++++++++ 4 files changed, 387 insertions(+), 69 deletions(-) create mode 100644 tests/test_generate_promotion_pipeline.py diff --git a/plugins/kbagent/skills/kbagent-promotion-pipeline/SKILL.md b/plugins/kbagent/skills/kbagent-promotion-pipeline/SKILL.md index 655ad4ab..1ff30ec7 100644 --- a/plugins/kbagent/skills/kbagent-promotion-pipeline/SKILL.md +++ b/plugins/kbagent/skills/kbagent-promotion-pipeline/SKILL.md @@ -31,26 +31,43 @@ directly out of that primitive, using one shared directory per pipeline and two Storage API tokens (source, destination): 1. **Pull** (`kbagent-promote-pull.yml`, manual + optional schedule) runs - `sync pull --project __env__ --directory --force` against the - **source** project's token, for every configured pipeline, then opens (or - updates) **one PR** against `main` with the combined diff via - [`peter-evans/create-pull-request`](https://github.com/peter-evans/create-pull-request). + `sync pull --project __env__ --directory --force` against + the **source** project's token into a throwaway scratch directory, merges + its *content* (everything except `.keboola/manifest.json`) into the + tracked ``, then opens (or updates) **one PR** against `main` with + the combined diff via + [`peter-evans/create-pull-request`](https://github.com/peter-evans/create-pull-request) + (using a PAT, not the default token -- see + [references/secrets-setup.md](references/secrets-setup.md)). 2. **Validate** (`kbagent-promote-validate.yml`, on the PR) runs `sync push --dry-run --project __env__ --directory ` against the **destination** project's token, for every pipeline touched by the PR -- this is the cross-project diff: *if this PR merges, here is exactly what changes in the destination project.* Read this before approving. -3. **Push** (`kbagent-promote-push.yml`, on push to `main`) runs - `sync push --project __env__ --directory ` against the - **destination** project's token, gated by the `prod` GitHub Environment - (add required reviewers there for manual approval even though the trigger - is an automatic push-on-merge, not `workflow_dispatch`). +3. **Push** (`kbagent-promote-push.yml`, on push to `main`) runs, in a + **separate job per pipeline**, `sync push --project __env__ --directory + ` against the **destination** project's token, each job gated by the + `prod` GitHub Environment (add required reviewers there -- every job run + gets its own separate approval, so approving one pipeline never approves + another). `main` therefore always represents "the last thing approved and pushed to every destination project" -- the reviewable source of truth the whole repo is built around. A promotion is: pull opens a PR -> validate shows the destination-side diff -> a human approves and merges -> push ships it. +**Why pull goes through a scratch directory instead of pulling `` +directly:** ``'s `.keboola/manifest.json` is bootstrapped once from the +*destination* project (Step 4) and must stay bound to the destination's +config IDs forever after -- that ID mapping is what lets `sync push` +recognize "this is config X I already created" instead of creating a +duplicate every time. Pulling straight into `` with the *source* +token would overwrite that manifest with the source project's IDs, and the +next push would then fail to match anything in the destination and create +duplicates on every single promotion cycle. Pulling into a scratch +directory and merging only the content keeps the destination's ID mapping +stable while still picking up the source's changes. + Every step uses `KBAGENT_PROJECT_FROM_ENV=1` + the reserved `--project __env__` alias (kbagent's headless/CI auth model) -- no token is ever written to `config.json` or committed to the repo. See @@ -114,17 +131,22 @@ would show every single config as "new," which is both wrong and a scary first review. Locally, with the destination project's token: ```bash export KBAGENT_PROJECT_FROM_ENV=1 KBC_TOKEN= KBC_STORAGE_API_URL= -kbagent sync init --project __env__ --directory kbagent sync pull --project __env__ --directory git add && git commit -m "Bootstrap from destination project" && git push ``` -Do this directly on `main`, not through a PR -- there is nothing to review -yet, it's just establishing the starting baseline. +(`sync pull` auto-initializes when no manifest exists yet -- no separate +`sync init` needed.) Do this directly on `main`, not through a PR -- there +is nothing to review yet, it's just establishing the starting baseline. This +step is what binds ``'s manifest to the destination project's config +IDs -- see "Why pull goes through a scratch directory" above for why that +binding must never be overwritten by a later source pull. ### Step 5 -- Set up GitHub secrets and the `prod` environment Two Storage API token secrets per pipeline (`KBC_TOKEN__SOURCE`, -`KBC_TOKEN__DEST`) plus the `prod` GitHub Environment with required -reviewers -- the generator prints the exact `gh` commands. See +`KBC_TOKEN__DEST`), one repo-wide `PROMOTION_PR_TOKEN` (a PAT -- +the default `GITHUB_TOKEN` cannot open a PR that triggers `validate`), and +the `prod` GitHub Environment with required reviewers -- the generator +prints the exact `gh` commands. See [references/secrets-setup.md](references/secrets-setup.md). ### Step 6 -- Run a promotion end-to-end @@ -133,23 +155,38 @@ reviewers -- the generator prints the exact `gh` commands. See 2. Read the `kbagent-promote-validate.yml` check's dry-run output on that PR -- confirm it matches what you expect to land in each destination project. -3. Merge the PR. `kbagent-promote-push.yml` fires, waits for `prod` - environment approval, then pushes to every pipeline's destination - project. +3. Merge the PR. `kbagent-promote-push.yml` fires; each pipeline is its own + job, each waiting for its own `prod` environment approval, then pushing + to that pipeline's destination project. ## Guardrails (state these to the user) - **Never** add `--allow-plaintext-on-encrypt-failure` to the push workflow -- it silently uploads `#`-secrets in cleartext if the Encryption API is down. +- **Never promote `#`-secret values through `sync pull`/`sync push` across + projects** -- source-encrypted ciphertext cannot be decrypted by the + destination project. Set destination secrets independently, directly on + the destination project. See + [references/secrets-setup.md](references/secrets-setup.md#-secrets-do-not-promote-across-projects). - The `prod` environment's required-reviewer gate applies to `push`-triggered jobs too, not just `workflow_dispatch` -- confirm the reviewers are actually - configured, since a repo without them makes the "gate" a no-op. + configured, since a repo without them makes the "gate" a no-op. Each + pipeline gets its own job/approval (see secrets-setup.md), but every job + still needs those reviewers configured to mean anything. +- Add a **required status check** for `validate` on `main`'s branch + protection -- without it, a PR can merge even if validate never ran or + failed, silently degrading "PR-gated" to "PR-gated only if someone + happened to wait for the check." - One PR covers every pipeline pulled in that run (`branch: promote/update`). If pipelines are unrelated and reviewed by different people, consider splitting them into separate repos or separate pull workflows instead of - forcing one combined review. + forcing one combined review. (Push is already split per-pipeline; only + pull's PR is still combined.) - `peter-evans/create-pull-request` is a third-party action -- pin it to a full commit SHA (not just `@v7`) for a security-sensitive prod pipeline, and mention this to the customer rather than silently leaving the tag pin. +- It needs a `PROMOTION_PR_TOKEN` PAT, not the default `GITHUB_TOKEN` -- + otherwise the `validate` check never triggers on the PR it opens. See + [references/secrets-setup.md](references/secrets-setup.md). ## Reference material - [references/secrets-setup.md](references/secrets-setup.md) -- GitHub secrets/environment setup with `gh` commands. diff --git a/plugins/kbagent/skills/kbagent-promotion-pipeline/references/secrets-setup.md b/plugins/kbagent/skills/kbagent-promotion-pipeline/references/secrets-setup.md index 950a876e..2bb6fa94 100644 --- a/plugins/kbagent/skills/kbagent-promotion-pipeline/references/secrets-setup.md +++ b/plugins/kbagent/skills/kbagent-promotion-pipeline/references/secrets-setup.md @@ -1,8 +1,9 @@ # GitHub secrets / environment setup Each pipeline needs **two** Storage API token secrets -- one for the source -project, one for the destination project -- plus one shared `prod` -GitHub Environment used for push approval gating across every pipeline. +project, one for the destination project -- plus one repo-wide PAT for +opening promotion PRs, plus one `prod` GitHub Environment per pipeline used +for push approval gating (each pipeline gets its own approval; see below). ## Per pipeline @@ -14,18 +15,40 @@ GitHub Environment used for push approval gating across every pipeline. `` is the pipeline's `name`, uppercased and sanitized to `[A-Za-z0-9_]` (the generator's `Pipeline.label` property) -- it always matches what `generate_promotion_pipeline.py` prints in its secrets report, -so copy-paste from there rather than re-deriving it by hand. +so copy-paste from there rather than re-deriving it by hand. The generator +also rejects two pipeline names that sanitize to the same label, so a +secret name never silently collides between two different pipelines. + +## Repo-wide + +| Secret | Used by | Purpose | +|---|---|---| +| `PROMOTION_PR_TOKEN` | `kbagent-promote-pull.yml` | Opens the promotion PR as a real identity, not the default token | + +**Why this PAT is required, not optional:** `peter-evans/create-pull-request` +opens the PR using whatever token it's given. If that's the workflow's +default `GITHUB_TOKEN`, GitHub deliberately suppresses `pull_request`-triggered +workflow runs for PRs opened that way -- `kbagent-promote-validate.yml` would +never fire on the PR, and reviewers would approve blind with no destination-side +diff, silently defeating the whole point of this pipeline. Use a fine-grained +PAT (Contents: write, Pull requests: write, scoped to this repo) or a GitHub +App installation token instead, and set it as `PROMOTION_PR_TOKEN`. ## Setup with `gh` ```bash REPO=/ +# Once per repo: +gh secret set PROMOTION_PR_TOKEN --repo "$REPO" # paste the PAT described above + # Per pipeline (repeat for each): gh secret set KBC_TOKEN_SALESFORCE_SOURCE --repo "$REPO" # paste the dev project's token gh secret set KBC_TOKEN_SALESFORCE_DEST --repo "$REPO" # paste the prod project's token -# Shared push-approval environment (once per repo): +# Push-approval environment (once per repo -- every pipeline's push job +# references it, but each job run still gets its own separate approval, +# see "One environment, per-pipeline approval" below): gh api -X PUT "repos/$REPO/environments/prod" ``` @@ -34,11 +57,26 @@ Then in the GitHub UI (or via the environments API): actually makes `kbagent-promote-push.yml` block on approval -- the `environment: prod` line in the generated workflow is a no-op without reviewers configured. -2. Optionally restrict the `prod` environment to the `main` branch only. -3. Scope the `*_DEST` secrets to the `prod` environment if your org's policy +2. Add a **required status check** for the `validate` job on `main`'s branch + protection rules. Without this, a PR can be merged even if `validate` + never ran (e.g. the PAT above was missing) or actively failed -- + "PR-gated" is only true if merging is actually blocked on it. +3. Optionally restrict the `prod` environment to the `main` branch only. +4. Scope the `*_DEST` secrets to the `prod` environment if your org's policy requires environment-scoped secrets (recommended for genuinely production-facing tokens). +## One environment, per-pipeline approval + +`kbagent-promote-push.yml` generates **one job per pipeline**, each with +`environment: prod`. GitHub's required-reviewer gate is enforced per job +*run*, not per environment name -- even though every pipeline's job +references the same `prod` environment, each one gets its own separate +"Review deployments" prompt. Approving one pipeline's push does not approve +any other pipeline in the same workflow run, and one pipeline's job failing +does not block or skip the others (they have no `needs:` dependency on each +other). + ## Why no token in config.json kbagent can read a committed `.kbagent/config.json` with registered project @@ -48,10 +86,29 @@ generated workflow step instead sets `KBAGENT_PROJECT_FROM_ENV=1` + exists solely as a masked GitHub secret in the runner's environment, never written to a file. +## `#`-secrets do not promote across projects + +`sync pull`/`sync push` move plain configuration between projects, but a +config's `#`-prefixed (encrypted) values are stored on disk only as +project-scoped ciphertext (`KBC::ProjectSecure::...`) -- ciphertext encrypted +for the *source* project cannot be decrypted by the *destination* project. +A value that round-trips through this pipeline either fails `sync push`'s +fail-closed encryption check, or -- if something upstream forced plaintext +through -- lands in the destination as an inert string that looks like a +secret but isn't one, a silent outage waiting to happen. + +**Never promote `#`-secret values through this pipeline.** Set each +destination project's secrets independently and directly on that project +(`kbagent config variables-set`, `config update`, or `data-app secrets-set` +against the *destination* token) -- only the config's *structure* (which +keys exist) should ever come from a promotion PR, never `#`-secret contents. + ## Security guardrails - Do **not** commit `.kbagent/config.json` with tokens. - Do **not** pass `--allow-plaintext-on-encrypt-failure` in CI. +- Never promote `#`-secret *values* through `sync pull`/`sync push` across + projects -- see above. - Prefer environment-scoped `*_DEST` secrets and required reviewers for any pipeline whose destination is a genuinely production project. - Pin `peter-evans/create-pull-request` to a full commit SHA, not just a diff --git a/plugins/kbagent/skills/kbagent-promotion-pipeline/scripts/generate_promotion_pipeline.py b/plugins/kbagent/skills/kbagent-promotion-pipeline/scripts/generate_promotion_pipeline.py index 935c0114..4077f34f 100644 --- a/plugins/kbagent/skills/kbagent-promotion-pipeline/scripts/generate_promotion_pipeline.py +++ b/plugins/kbagent/skills/kbagent-promotion-pipeline/scripts/generate_promotion_pipeline.py @@ -81,10 +81,41 @@ def dest_token_secret(self) -> str: return f"KBC_TOKEN_{self.label}_DEST" +_SAFE_DIRECTORY_RE = re.compile(r"^[A-Za-z0-9][A-Za-z0-9_./-]*$") +_SAFE_URL_RE = re.compile(r"^https?://[A-Za-z0-9.-]+/?$") + + +def _validate_pipelines(pipelines: list[Pipeline]) -> None: + """Reject pipeline fields that would break out of generated YAML/shell quoting. + + ``directory``/``*_stack_url`` are admin-authored (a JSON config file or CLI + flags), not attacker-controlled at runtime, but the generator still embeds + them raw into single-quoted shell strings and YAML `paths:` lists -- a + value containing a quote, newline, or `..` would corrupt the generated + workflow. Fail fast at generation time instead (CWE-78-adjacent). + """ + labels_seen: dict[str, str] = {} + for p in pipelines: + if ".." in p.directory.split("/") or not _SAFE_DIRECTORY_RE.match(p.directory): + raise ValueError(f"pipeline {p.name!r}: unsafe directory {p.directory!r}") + for field, url in ( + ("source_stack_url", p.source_stack_url), + ("dest_stack_url", p.dest_stack_url), + ): + if not _SAFE_URL_RE.match(url): + raise ValueError(f"pipeline {p.name!r}: unsafe {field} {url!r}") + if p.label in labels_seen: + raise ValueError( + f"pipelines {labels_seen[p.label]!r} and {p.name!r} both sanitize to the " + f"secret-name label {p.label!r} -- rename one" + ) + labels_seen[p.label] = p.name + + def _load_pipelines(args: argparse.Namespace) -> list[Pipeline]: if args.config: data = json.loads(Path(args.config).read_text(encoding="utf-8")) - return [ + pipelines = [ Pipeline( name=str(p["name"]), directory=str(p["directory"]), @@ -93,33 +124,44 @@ def _load_pipelines(args: argparse.Namespace) -> list[Pipeline]: ) for p in data ] - missing = [ - flag - for flag, val in ( - ("--name", args.name), - ("--directory", args.directory), - ("--source-stack-url", args.source_stack_url), - ("--dest-stack-url", args.dest_stack_url), - ) - if not val - ] - if missing: - print( - f"error: --config or all of {', '.join(missing)} must be provided", - file=sys.stderr, - ) + else: + missing = [ + flag + for flag, val in ( + ("--name", args.name), + ("--directory", args.directory), + ("--source-stack-url", args.source_stack_url), + ("--dest-stack-url", args.dest_stack_url), + ) + if not val + ] + if missing: + print( + f"error: --config or all of {', '.join(missing)} must be provided", + file=sys.stderr, + ) + sys.exit(2) + pipelines = [ + Pipeline( + name=args.name, + directory=args.directory, + source_stack_url=_normalize_url(args.source_stack_url), + dest_stack_url=_normalize_url(args.dest_stack_url), + ) + ] + try: + _validate_pipelines(pipelines) + except ValueError as exc: + print(f"error: {exc}", file=sys.stderr) sys.exit(2) - return [ - Pipeline( - name=args.name, - directory=args.directory, - source_stack_url=_normalize_url(args.source_stack_url), - dest_stack_url=_normalize_url(args.dest_stack_url), - ) - ] + return pipelines def _normalize_url(host: str) -> str: + # kbagent's own `KBC_STORAGE_API_URL` consumption normalizes a bare host + # to `https://` (see `models.normalize_stack_url`), but the + # generator also uses this value in printed messages and YAML before + # kbagent ever sees it, so normalize once here too. host = host.strip() if host.startswith(("http://", "https://")): return host @@ -168,18 +210,61 @@ def _pipeline_step( ) +def _pull_and_merge_step(p: Pipeline) -> str: + """Pull SOURCE into a throwaway scratch dir, then merge its *content* -- + never its `.keboola/manifest.json` -- into the tracked directory. + + The tracked directory's manifest is bootstrapped once from the DEST + project (Step 4) and must stay bound to DEST's config IDs forever after; + overwriting it with a plain `sync pull --project __env__ --directory + '{p.directory}'` against the SOURCE token (the original approach) replaces + that manifest with SOURCE's IDs, so every subsequent `sync push` to DEST + fails to match any existing config by ID and recreates duplicates on every + promotion cycle instead of converging. Pulling into an ephemeral scratch + directory and copying only the content over keeps DEST's ID mapping + stable while still picking up SOURCE's additions/edits/deletions. + """ + scratch = f"/tmp/promote-scratch/{p.directory}" + return ( + f" - name: Pull {p.name} from source (scratch)\n" + " env:\n" + ' KBAGENT_PROJECT_FROM_ENV: "1"\n' + f" KBC_TOKEN: ${{{{ secrets.{p.source_token_secret} }}}}\n" + f" KBC_STORAGE_API_URL: {p.source_stack_url}\n" + " run: |\n" + f" kbagent sync pull --project __env__ --directory '{scratch}' --force\n" + f" - name: Merge {p.name} content into '{p.directory}' (manifest untouched)\n" + " run: |\n" + " python3 - <<'PYEOF'\n" + " import pathlib, shutil\n" + f" src = pathlib.Path({scratch!r})\n" + f" dst = pathlib.Path({p.directory!r})\n" + " dst.mkdir(parents=True, exist_ok=True)\n" + " for item in list(dst.iterdir()):\n" + " if item.name != '.keboola':\n" + " shutil.rmtree(item) if item.is_dir() else item.unlink()\n" + " for item in src.iterdir():\n" + " if item.name == '.keboola':\n" + " continue\n" + " target = dst / item.name\n" + " if item.is_dir():\n" + " shutil.copytree(item, target)\n" + " else:\n" + " shutil.copy2(item, target)\n" + " PYEOF\n" + ) + + def gen_pull(pipelines: list[Pipeline], schedule: str | None, main_branch: str) -> str: on_block = " workflow_dispatch:\n" if schedule: on_block += f" schedule:\n - cron: '{schedule}'\n" - steps = "".join( - _pipeline_step(p, "Pull", "pull --force", p.source_token_secret, p.source_stack_url) - for p in pipelines - ) + steps = "".join(_pull_and_merge_step(p) for p in pipelines) paths = ", ".join(p.directory for p in pipelines) return ( "# Generated by kbagent-promotion-pipeline. Pulls every pipeline's SOURCE\n" - "# project and opens/updates one PR against the main branch.\n" + "# project into a scratch dir, merges content (not the DEST-bound manifest)\n" + "# into the tracked directory, and opens/updates one PR against main.\n" "name: kbagent promote - pull\n" "on:\n" f"{on_block}" @@ -196,6 +281,12 @@ def gen_pull(pipelines: list[Pipeline], schedule: str | None, main_branch: str) " - name: Open promotion PR\n" " uses: peter-evans/create-pull-request@v7\n" " with:\n" + " # The default GITHUB_TOKEN cannot be used here: GitHub suppresses\n" + " # `pull_request`-triggered workflow runs (kbagent-promote-validate)\n" + " # for PRs opened by the default token, so validate would silently\n" + " # never run. Use a PAT/GitHub App token instead -- see\n" + " # references/secrets-setup.md.\n" + " token: ${{ secrets.PROMOTION_PR_TOKEN }}\n" " branch: promote/update\n" f" base: {main_branch}\n" ' commit-message: "kbagent promote: pull latest config from source project(s)"\n' @@ -242,16 +333,42 @@ def gen_validate(pipelines: list[Pipeline]) -> str: ) -def gen_push(pipelines: list[Pipeline], main_branch: str) -> str: - steps = "".join( - _pipeline_step(p, "Push", "push", p.dest_token_secret, p.dest_stack_url) for p in pipelines +def _push_job(p: Pipeline) -> str: + """One job per pipeline, each gated by its own `prod` environment run. + + GitHub's required-reviewer approval is per job *run*, not per environment + name -- two jobs in the same workflow run that both reference `prod` each + get their own separate approval prompt. The original design put every + pipeline's push as a *step* inside one shared job, so a single approval + click unlocked every pipeline at once, and one pipeline's failure could + leave later pipelines silently un-pushed. Splitting into independent jobs + fixes both: per-pipeline approval, and jobs run independently so one + failing does not block the others. + """ + job_id = f"push_{p.label.lower()}" + return ( + f" {job_id}:\n" + " environment: prod\n" + " runs-on: ubuntu-latest\n" + " steps:\n" + " - uses: actions/checkout@v4\n" + f"{_INSTALL_TOKEN}" + " # `sync push` encrypts #-secrets fail-closed by default. Do NOT add\n" + " # --allow-plaintext-on-encrypt-failure in CI.\n" + f"{_pipeline_step(p, 'Push', 'push', p.dest_token_secret, p.dest_stack_url)}" ) + + +def gen_push(pipelines: list[Pipeline], main_branch: str) -> str: paths = "\n".join(f" - '{p.directory}/**'" for p in pipelines) + jobs = "".join(_push_job(p) for p in pipelines) return ( "# Generated by kbagent-promotion-pipeline. Pushes every pipeline's\n" - "# directory to its DESTINATION project once merged to main.\n" - "# Gated by the 'prod' GitHub Environment -- add required reviewers there\n" - "# for manual approval even though the trigger is an automatic push.\n" + "# directory to its DESTINATION project once merged to main. Each\n" + "# pipeline is its OWN job, each gated by the 'prod' GitHub Environment --\n" + "# add required reviewers there. Every job run needs its own separate\n" + "# approval; approving one pipeline's push never approves another's, and\n" + "# one pipeline failing does not block the others.\n" "name: kbagent promote - push\n" "on:\n" " push:\n" @@ -261,15 +378,7 @@ def gen_push(pipelines: list[Pipeline], main_branch: str) -> str: "permissions:\n" " contents: read\n" "jobs:\n" - " push:\n" - " environment: prod\n" - " runs-on: ubuntu-latest\n" - " steps:\n" - " - uses: actions/checkout@v4\n" - f"{_INSTALL_TOKEN}" - " # `sync push` encrypts #-secrets fail-closed by default. Do NOT add\n" - " # --allow-plaintext-on-encrypt-failure in CI.\n" - f"{steps}" + f"{jobs}" ) @@ -292,9 +401,17 @@ def secrets_report(pipelines: list[Pipeline], repo_slug: str) -> str: f" gh secret set {p.dest_token_secret} --repo {repo_slug} # {p.name} destination project" ) lines.append("") + lines.append( + "Required GitHub PAT (default GITHUB_TOKEN cannot trigger the validate\n" + "check on the PR it opens -- see references/secrets-setup.md):" + ) + lines.append(f" gh secret set PROMOTION_PR_TOKEN --repo {repo_slug}") + lines.append("") lines.append("Required GitHub Environment (for push approval gating):") lines.append(f" gh api -X PUT repos/{repo_slug}/environments/prod") lines.append(" # Then add required reviewers to 'prod' in the GitHub UI.") + lines.append(" # Also add a required status check for 'validate' on the main") + lines.append(" # branch's protection rules, or a merge can bypass validate entirely.") return "\n".join(lines) diff --git a/tests/test_generate_promotion_pipeline.py b/tests/test_generate_promotion_pipeline.py new file mode 100644 index 00000000..4b906e03 --- /dev/null +++ b/tests/test_generate_promotion_pipeline.py @@ -0,0 +1,107 @@ +"""Tests for the kbagent-promotion-pipeline skill's generator script. + +Imported by path since the script lives under plugins/, not src/. +""" + +from __future__ import annotations + +import importlib.util +import sys +from pathlib import Path + +import pytest +import yaml + +_SCRIPT = ( + Path(__file__).parent.parent + / "plugins/kbagent/skills/kbagent-promotion-pipeline/scripts/generate_promotion_pipeline.py" +) +_spec = importlib.util.spec_from_file_location("generate_promotion_pipeline", _SCRIPT) +assert _spec is not None and _spec.loader is not None +_mod = importlib.util.module_from_spec(_spec) +sys.modules[_spec.name] = _mod +_spec.loader.exec_module(_mod) + +Pipeline = _mod.Pipeline +gen_pull = _mod.gen_pull +gen_push = _mod.gen_push +gen_validate = _mod.gen_validate +_validate_pipelines = _mod._validate_pipelines + + +def _parse_yaml(generated: str) -> dict: + """Generated workflows carry an unresolved @@INSTALL@@ placeholder until + `run()` substitutes real install steps -- fill in a stub so the YAML parses.""" + return yaml.safe_load(generated.replace(_mod._INSTALL_TOKEN, " - run: 'noop'\n")) + + +def _pipeline(name: str = "SALESFORCE", directory: str = "salesforce") -> Pipeline: + return Pipeline( + name=name, + directory=directory, + source_stack_url="https://connection.keboola.com", + dest_stack_url="https://connection.keboola.com", + ) + + +class TestPullMechanic: + def test_pulls_into_scratch_not_tracked_directory(self) -> None: + yml = gen_pull([_pipeline()], schedule=None, main_branch="main") + assert "/tmp/promote-scratch/salesforce" in yml + assert "--directory '/tmp/promote-scratch/salesforce' --force" in yml + + def test_merge_step_preserves_keboola_manifest(self) -> None: + yml = gen_pull([_pipeline()], schedule=None, main_branch="main") + assert "if item.name != '.keboola'" in yml + assert "if item.name == '.keboola'" in yml + + def test_pr_uses_pat_not_default_token(self) -> None: + yml = gen_pull([_pipeline()], schedule=None, main_branch="main") + assert "token: ${{ secrets.PROMOTION_PR_TOKEN }}" in yml + + def test_generated_yaml_is_valid(self) -> None: + yml = gen_pull([_pipeline()], schedule=None, main_branch="main") + parsed = _parse_yaml(yml) + assert "pull" in parsed["jobs"] + + +class TestPushIsolation: + def test_one_job_per_pipeline(self) -> None: + pipelines = [_pipeline("SALESFORCE", "salesforce"), _pipeline("GA4", "ga4")] + parsed = _parse_yaml(gen_push(pipelines, main_branch="main")) + assert set(parsed["jobs"]) == {"push_salesforce", "push_ga4"} + + def test_every_job_has_its_own_prod_environment(self) -> None: + pipelines = [_pipeline("SALESFORCE", "salesforce"), _pipeline("GA4", "ga4")] + parsed = _parse_yaml(gen_push(pipelines, main_branch="main")) + assert all(job["environment"] == "prod" for job in parsed["jobs"].values()) + + def test_jobs_have_no_needs_dependency_so_one_failure_does_not_block_others(self) -> None: + pipelines = [_pipeline("SALESFORCE", "salesforce"), _pipeline("GA4", "ga4")] + parsed = _parse_yaml(gen_push(pipelines, main_branch="main")) + assert all("needs" not in job for job in parsed["jobs"].values()) + + +class TestValidate: + def test_generated_yaml_is_valid(self) -> None: + parsed = _parse_yaml(gen_validate([_pipeline()])) + assert "validate" in parsed["jobs"] + + +class TestValidatePipelines: + def test_rejects_path_traversal_directory(self) -> None: + with pytest.raises(ValueError, match="unsafe directory"): + _validate_pipelines([_pipeline(directory="../etc")]) + + def test_rejects_quote_in_stack_url(self) -> None: + p = _pipeline() + p.source_stack_url = "https://connection.keboola.com'; rm -rf /" + with pytest.raises(ValueError, match="unsafe source_stack_url"): + _validate_pipelines([p]) + + def test_rejects_label_collision(self) -> None: + with pytest.raises(ValueError, match="secret-name label"): + _validate_pipelines([_pipeline("sales-force", "a"), _pipeline("sales force", "b")]) + + def test_accepts_distinct_safe_pipelines(self) -> None: + _validate_pipelines([_pipeline("SALESFORCE", "salesforce"), _pipeline("GA4", "ga4")]) From ae92b5a051dd849e9d70e99f080a664ed0893250 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Va=C5=A1ko?= Date: Mon, 10 Aug 2026 13:32:12 +0200 Subject: [PATCH 3/3] fix(skill): address Copilot review comments on promotion-pipeline generator - Escape directory/version/git-ref values with shlex.quote() before embedding them in generated shell commands, instead of naive single-quote wrapping. - The generated merge-content step now checks is_symlink() before deleting or copying entries: is_dir()/is_file() follow symlinks, so an unguarded rmtree()/copytree() could traverse outside the tracked directory. Deletion now unlinks a symlink directly instead of following it; a symlink in the pulled source content now fails the step loudly instead of silently copying through it (kbagent's own sync pull never produces symlinks). - _validate_pipelines now also rejects unsafe characters in pipeline `name` and two pipelines sharing the same `directory` (guaranteed wrong: two different tokens pulling/pushing into one folder). - --config parsing now catches OSError/JSONDecodeError/a missing required key and exits 2 with a clear message instead of an uncaught traceback. - Corrected SKILL.md wording: validate's dry-run step runs unconditionally for every configured pipeline once the workflow triggers, not only for "pipelines touched by the PR" (the paths: trigger gates the whole workflow, not individual per-pipeline steps). Added test coverage for all of the above (7 new cases). Co-Authored-By: Claude Sonnet 5 --- .../kbagent-promotion-pipeline/SKILL.md | 6 +- .../scripts/generate_promotion_pipeline.py | 77 ++++++++++++++----- tests/test_generate_promotion_pipeline.py | 42 +++++++++- 3 files changed, 100 insertions(+), 25 deletions(-) diff --git a/plugins/kbagent/skills/kbagent-promotion-pipeline/SKILL.md b/plugins/kbagent/skills/kbagent-promotion-pipeline/SKILL.md index 1ff30ec7..431fe487 100644 --- a/plugins/kbagent/skills/kbagent-promotion-pipeline/SKILL.md +++ b/plugins/kbagent/skills/kbagent-promotion-pipeline/SKILL.md @@ -41,8 +41,10 @@ two Storage API tokens (source, destination): [references/secrets-setup.md](references/secrets-setup.md)). 2. **Validate** (`kbagent-promote-validate.yml`, on the PR) runs `sync push --dry-run --project __env__ --directory ` against the - **destination** project's token, for every pipeline touched by the PR -- - this is the cross-project diff: *if this PR merges, here is exactly what + **destination** project's token, once per configured pipeline (the + `paths:` trigger only gates whether the workflow runs at all, not which + pipeline steps execute inside it -- every pipeline's dry-run always runs) + -- this is the cross-project diff: *if this PR merges, here is exactly what changes in the destination project.* Read this before approving. 3. **Push** (`kbagent-promote-push.yml`, on push to `main`) runs, in a **separate job per pipeline**, `sync push --project __env__ --directory diff --git a/plugins/kbagent/skills/kbagent-promotion-pipeline/scripts/generate_promotion_pipeline.py b/plugins/kbagent/skills/kbagent-promotion-pipeline/scripts/generate_promotion_pipeline.py index 4077f34f..cc1fbfb9 100644 --- a/plugins/kbagent/skills/kbagent-promotion-pipeline/scripts/generate_promotion_pipeline.py +++ b/plugins/kbagent/skills/kbagent-promotion-pipeline/scripts/generate_promotion_pipeline.py @@ -50,6 +50,7 @@ import argparse import json import re +import shlex import sys from dataclasses import dataclass from pathlib import Path @@ -83,19 +84,26 @@ def dest_token_secret(self) -> str: _SAFE_DIRECTORY_RE = re.compile(r"^[A-Za-z0-9][A-Za-z0-9_./-]*$") _SAFE_URL_RE = re.compile(r"^https?://[A-Za-z0-9.-]+/?$") +_SAFE_NAME_RE = re.compile(r"^[A-Za-z0-9][A-Za-z0-9 ._-]*$") def _validate_pipelines(pipelines: list[Pipeline]) -> None: """Reject pipeline fields that would break out of generated YAML/shell quoting. - ``directory``/``*_stack_url`` are admin-authored (a JSON config file or CLI - flags), not attacker-controlled at runtime, but the generator still embeds - them raw into single-quoted shell strings and YAML `paths:` lists -- a - value containing a quote, newline, or `..` would corrupt the generated - workflow. Fail fast at generation time instead (CWE-78-adjacent). + ``name``/``directory``/``*_stack_url`` are admin-authored (a JSON config + file or CLI flags), not attacker-controlled at runtime, but the generator + still embeds them into YAML string fields and (for ``directory``) a + shell command -- a value containing a quote, colon, newline, or `..` + would corrupt the generated workflow. Fail fast at generation time + instead (CWE-78-adjacent). Also rejects two pipelines that would collide + on the same tracked directory (guaranteed-wrong: two different tokens + pulling/pushing into one folder) or the same secret-name label. """ labels_seen: dict[str, str] = {} + directories_seen: dict[str, str] = {} for p in pipelines: + if not _SAFE_NAME_RE.match(p.name): + raise ValueError(f"pipeline name {p.name!r} contains unsafe characters") if ".." in p.directory.split("/") or not _SAFE_DIRECTORY_RE.match(p.directory): raise ValueError(f"pipeline {p.name!r}: unsafe directory {p.directory!r}") for field, url in ( @@ -104,6 +112,12 @@ def _validate_pipelines(pipelines: list[Pipeline]) -> None: ): if not _SAFE_URL_RE.match(url): raise ValueError(f"pipeline {p.name!r}: unsafe {field} {url!r}") + if p.directory in directories_seen: + raise ValueError( + f"pipelines {directories_seen[p.directory]!r} and {p.name!r} both use " + f"directory {p.directory!r} -- each pipeline needs its own directory" + ) + directories_seen[p.directory] = p.name if p.label in labels_seen: raise ValueError( f"pipelines {labels_seen[p.label]!r} and {p.name!r} both sanitize to the " @@ -112,18 +126,29 @@ def _validate_pipelines(pipelines: list[Pipeline]) -> None: labels_seen[p.label] = p.name +def _pipeline_from_dict(p: dict) -> Pipeline: + try: + return Pipeline( + name=str(p["name"]), + directory=str(p["directory"]), + source_stack_url=_normalize_url(str(p["source_stack_url"])), + dest_stack_url=_normalize_url(str(p["dest_stack_url"])), + ) + except KeyError as exc: + raise ValueError(f"pipeline entry missing required key {exc}") from exc + + def _load_pipelines(args: argparse.Namespace) -> list[Pipeline]: if args.config: - data = json.loads(Path(args.config).read_text(encoding="utf-8")) - pipelines = [ - Pipeline( - name=str(p["name"]), - directory=str(p["directory"]), - source_stack_url=_normalize_url(str(p["source_stack_url"])), - dest_stack_url=_normalize_url(str(p["dest_stack_url"])), - ) - for p in data - ] + try: + raw = Path(args.config).read_text(encoding="utf-8") + data = json.loads(raw) + if not isinstance(data, list): + raise ValueError("--config must contain a JSON list of pipeline objects") + pipelines = [_pipeline_from_dict(p) for p in data] + except (OSError, json.JSONDecodeError, ValueError) as exc: + print(f"error: --config {args.config!r} is invalid: {exc}", file=sys.stderr) + sys.exit(2) else: missing = [ flag @@ -184,7 +209,7 @@ def _install_steps(version: str | None, git_ref: str | None) -> str: " - name: Install uv\n" " uses: astral-sh/setup-uv@v5\n" " - name: Install kbagent\n" - f" run: uv tool install '{spec}'\n" + f" run: uv tool install {shlex.quote(spec)}\n" " - name: Show version\n" " run: kbagent version\n" ) @@ -206,7 +231,8 @@ def _pipeline_step( f" KBC_TOKEN: ${{{{ secrets.{token_secret} }}}}\n" f" KBC_STORAGE_API_URL: {stack_url}\n" " run: |\n" - f" {prefix}sync {command} --project __env__ --directory '{p.directory}'\n" + f" {prefix}sync {command} --project __env__ " + f"--directory {shlex.quote(p.directory)}\n" ) @@ -232,7 +258,8 @@ def _pull_and_merge_step(p: Pipeline) -> str: f" KBC_TOKEN: ${{{{ secrets.{p.source_token_secret} }}}}\n" f" KBC_STORAGE_API_URL: {p.source_stack_url}\n" " run: |\n" - f" kbagent sync pull --project __env__ --directory '{scratch}' --force\n" + f" kbagent sync pull --project __env__ --directory {shlex.quote(scratch)} " + "--force\n" f" - name: Merge {p.name} content into '{p.directory}' (manifest untouched)\n" " run: |\n" " python3 - <<'PYEOF'\n" @@ -241,11 +268,21 @@ def _pull_and_merge_step(p: Pipeline) -> str: f" dst = pathlib.Path({p.directory!r})\n" " dst.mkdir(parents=True, exist_ok=True)\n" " for item in list(dst.iterdir()):\n" - " if item.name != '.keboola':\n" - " shutil.rmtree(item) if item.is_dir() else item.unlink()\n" + " if item.name == '.keboola':\n" + " continue\n" + " # is_dir() follows symlinks -- unlink the link itself rather\n" + " # than rmtree-ing through it (which could delete outside ).\n" + " if item.is_symlink() or item.is_file():\n" + " item.unlink()\n" + " else:\n" + " shutil.rmtree(item)\n" " for item in src.iterdir():\n" " if item.name == '.keboola':\n" " continue\n" + " if item.is_symlink():\n" + " # kbagent's own `sync pull` output is always plain files/\n" + " # dirs, never symlinks -- refuse rather than guess at intent.\n" + " raise SystemExit(f'refusing to copy symlink from pull: {item}')\n" " target = dst / item.name\n" " if item.is_dir():\n" " shutil.copytree(item, target)\n" diff --git a/tests/test_generate_promotion_pipeline.py b/tests/test_generate_promotion_pipeline.py index 4b906e03..2cd21cf2 100644 --- a/tests/test_generate_promotion_pipeline.py +++ b/tests/test_generate_promotion_pipeline.py @@ -48,12 +48,16 @@ class TestPullMechanic: def test_pulls_into_scratch_not_tracked_directory(self) -> None: yml = gen_pull([_pipeline()], schedule=None, main_branch="main") assert "/tmp/promote-scratch/salesforce" in yml - assert "--directory '/tmp/promote-scratch/salesforce' --force" in yml + assert "--directory /tmp/promote-scratch/salesforce --force" in yml def test_merge_step_preserves_keboola_manifest(self) -> None: yml = gen_pull([_pipeline()], schedule=None, main_branch="main") - assert "if item.name != '.keboola'" in yml - assert "if item.name == '.keboola'" in yml + assert yml.count("if item.name == '.keboola'") == 2 + + def test_merge_step_does_not_follow_symlinks(self) -> None: + yml = gen_pull([_pipeline()], schedule=None, main_branch="main") + assert "item.is_symlink()" in yml + assert "refusing to copy symlink" in yml def test_pr_uses_pat_not_default_token(self) -> None: yml = gen_pull([_pipeline()], schedule=None, main_branch="main") @@ -99,9 +103,41 @@ def test_rejects_quote_in_stack_url(self) -> None: with pytest.raises(ValueError, match="unsafe source_stack_url"): _validate_pipelines([p]) + def test_rejects_unsafe_name(self) -> None: + with pytest.raises(ValueError, match="unsafe characters"): + _validate_pipelines([_pipeline(name="sales'; rm -rf /")]) + + def test_rejects_directory_collision(self) -> None: + with pytest.raises(ValueError, match="both use directory"): + _validate_pipelines([_pipeline("SALESFORCE", "shared"), _pipeline("GA4", "shared")]) + def test_rejects_label_collision(self) -> None: with pytest.raises(ValueError, match="secret-name label"): _validate_pipelines([_pipeline("sales-force", "a"), _pipeline("sales force", "b")]) def test_accepts_distinct_safe_pipelines(self) -> None: _validate_pipelines([_pipeline("SALESFORCE", "salesforce"), _pipeline("GA4", "ga4")]) + + +class TestConfigParsing: + def test_missing_config_file_exits_2_not_traceback(self, tmp_path, capsys) -> None: + with pytest.raises(SystemExit) as exc: + _mod.main(["--config", str(tmp_path / "missing.json"), str(tmp_path)]) + assert exc.value.code == 2 + assert "invalid" in capsys.readouterr().err + + def test_invalid_json_exits_2_not_traceback(self, tmp_path, capsys) -> None: + bad = tmp_path / "bad.json" + bad.write_text("{not valid json", encoding="utf-8") + with pytest.raises(SystemExit) as exc: + _mod.main(["--config", str(bad), str(tmp_path)]) + assert exc.value.code == 2 + assert "invalid" in capsys.readouterr().err + + def test_missing_required_key_exits_2_not_traceback(self, tmp_path, capsys) -> None: + cfg = tmp_path / "missing_key.json" + cfg.write_text('[{"name": "X", "directory": "x"}]', encoding="utf-8") + with pytest.raises(SystemExit) as exc: + _mod.main(["--config", str(cfg), str(tmp_path)]) + assert exc.value.code == 2 + assert "missing required key" in capsys.readouterr().err