fix(ci): dispatch security-scan by cron name and deploy target #691

Merged
k.renner merged 1 commits from woodpecker-deployment-dispatch-pilot into master 2026-08-28 07:47:38 +00:00
Member

Pilot für #582. Beim Umbau kam ein zweiter Bug raus, der nichts mit 3.18.0 zu tun hat.

Der Cron-Gate hat nie gematcht

security-scan.yml gated auf:

- event: cron
  evaluate: 'PIPELINE == "security-scan"'

Crons können keine Variablen tragen. woodpecker-cli repo cron add nimmt --name, --branch, --schedule, --enabled — sonst nichts. PIPELINE ist bei jedem Cron-Lauf undefiniert, der Workflow fällt raus.

Der Cron daily-security-scan feuert durchaus. Was unter ihm läuft, zeigt Pipeline 6877:

e2e > clone            success
e2e > model-selection-tests  success
e2e > e2e-test         success

Nur e2e — weil e2e.yml sich per cron: daily-security-scan namentlich in diesen Cron einklinkt. Der Trivy-Scan und der Security-Fix-Agent laufen nicht. Das ist älter als 3.18.0 und hätte jeden Fix daran überlebt.

Der manuelle Gate ist der #582-Fall

Verifiziert im v3.18.0-Quelltext: builder.go:102-111 baut die Env für den Workflow-Root-Filter aus Metadaten plus globalen Envs. b.AdditionalEnvs existiert (Zeile 44) und geht an den Compiler (Zeile 223, WithNonPluginEnviron) — aber nie in diese Map. Also ist PIPELINE auch hier undefiniert.

Beides auf Selektoren umgestellt, die der Root-Filter sieht

when:
  - event: cron
    cron: daily-security-scan
  - event: deployment
    evaluate: 'CI_PIPELINE_DEPLOY_TARGET == "security-scan"'
Selektor Herkunft Vom Bug betroffen?
cron: <name> erstklassiger Filter im Constraint-Struct nein, keine Variable beteiligt
CI_PIPELINE_DEPLOY_TARGET Pipeline-Metadaten, metadata/environment.go:74 nein, keine AdditionalEnvs

Der Cron-Weg ist derselbe, den alert-monitor und e2e schon benutzen — die laufen nachweislich (318 bzw. mehrere Läufe im geprüften Fenster).

Dispatch wird damit:

woodpecker-cli pipeline deploy k.renner/homelab <pipeline-nr> security-scan

Ein benanntes Deployment-Ziel statt einer Variable, deren einziger Zweck war auszuwählen, welche der vierzehn Dateien laufen darf.

Was ich noch nicht weiß

Ob die Woodpecker-UI ein Deployment-Trigger-Feld anbietet, und ob das Repo Deployment-Events überhaupt zulässt — die CLI zeigt das Flag nicht. Klärt sich beim ersten echten Trigger nach dem Merge.

Scope

Pilot. Die restlichen dreizehn Pipelines bleiben unangetastet, bis dieser hier end-to-end bestätigt ist.

Refs #582

Pilot für #582. Beim Umbau kam ein zweiter Bug raus, der nichts mit 3.18.0 zu tun hat. ## Der Cron-Gate hat nie gematcht `security-scan.yml` gated auf: ```yaml - event: cron evaluate: 'PIPELINE == "security-scan"' ``` **Crons können keine Variablen tragen.** `woodpecker-cli repo cron add` nimmt `--name`, `--branch`, `--schedule`, `--enabled` — sonst nichts. `PIPELINE` ist bei jedem Cron-Lauf undefiniert, der Workflow fällt raus. Der Cron `daily-security-scan` feuert durchaus. Was unter ihm läuft, zeigt Pipeline **6877**: ``` e2e > clone success e2e > model-selection-tests success e2e > e2e-test success ``` Nur `e2e` — weil `e2e.yml` sich per `cron: daily-security-scan` **namentlich** in diesen Cron einklinkt. **Der Trivy-Scan und der Security-Fix-Agent laufen nicht.** Das ist älter als 3.18.0 und hätte jeden Fix daran überlebt. ## Der manuelle Gate ist der #582-Fall Verifiziert im v3.18.0-Quelltext: `builder.go:102-111` baut die Env für den Workflow-Root-Filter aus Metadaten plus globalen Envs. `b.AdditionalEnvs` existiert (Zeile 44) und geht an den Compiler (Zeile 223, `WithNonPluginEnviron`) — aber nie in diese Map. Also ist `PIPELINE` auch hier undefiniert. ## Beides auf Selektoren umgestellt, die der Root-Filter sieht ```yaml when: - event: cron cron: daily-security-scan - event: deployment evaluate: 'CI_PIPELINE_DEPLOY_TARGET == "security-scan"' ``` | Selektor | Herkunft | Vom Bug betroffen? | |---|---|---| | `cron: <name>` | erstklassiger Filter im Constraint-Struct | nein, keine Variable beteiligt | | `CI_PIPELINE_DEPLOY_TARGET` | Pipeline-Metadaten, `metadata/environment.go:74` | nein, keine `AdditionalEnvs` | Der Cron-Weg ist derselbe, den `alert-monitor` und `e2e` schon benutzen — die laufen nachweislich (318 bzw. mehrere Läufe im geprüften Fenster). Dispatch wird damit: ``` woodpecker-cli pipeline deploy k.renner/homelab <pipeline-nr> security-scan ``` Ein benanntes Deployment-Ziel statt einer Variable, deren einziger Zweck war auszuwählen, welche der vierzehn Dateien laufen darf. ## Was ich noch nicht weiß Ob die Woodpecker-UI ein Deployment-Trigger-Feld anbietet, und ob das Repo Deployment-Events überhaupt zulässt — die CLI zeigt das Flag nicht. Klärt sich beim ersten echten Trigger nach dem Merge. ## Scope **Pilot.** Die restlichen dreizehn Pipelines bleiben unangetastet, bis dieser hier end-to-end bestätigt ist. Refs #582
claude-bot added 1 commit 2026-08-28 07:39:26 +00:00
fix(ci): dispatch security-scan by cron name and deploy target
ci/woodpecker/pr/ci-scripts-test Pipeline was successful
ci/woodpecker/pr/ci-manifests-test Pipeline was successful
ci/woodpecker/pr/pr-review Pipeline was successful
ci/woodpecker/deployment/security-scan Pipeline was successful
96f11cb0bf
Two separate bugs in one `when` block, only one of which is #582.

The cron gate has never matched. It asked for `PIPELINE == "security-scan"` on
`event: cron`, and crons cannot carry variables — `repo cron add` takes name,
branch, schedule and enabled, and nothing else. So the variable is undefined on
every cron run and the workflow is filtered out. The `daily-security-scan` cron
does fire, and pipeline 6877 shows what actually ran under it: the `e2e`
workflow alone, which opts into that cron by name. The Trivy scan and the
security-fix agent have not been running on schedule. This predates 3.18.0 and
would have survived any fix to it.

The manual gate is the #582 case. Woodpecker 3.18 stopped merging manual-run
variables into the environment the workflow-root filter evaluates against
(builder.go:102-111 builds it from metadata plus global envs; AdditionalEnvs is
passed to the compiler at line 223 but never into this map), so
`PIPELINE == "security-scan"` cannot be true there either.

Both now use selectors the root filter can actually see:

  cron       -> matched by the cron's NAME, the way alert-monitor and e2e
                already do it. First-class filter, no variable involved.
  deployment -> CI_PIPELINE_DEPLOY_TARGET, which comes from pipeline metadata
                (metadata/environment.go:74) and therefore reaches the root
                filter on 3.18.

Dispatch becomes `woodpecker-cli pipeline deploy k.renner/homelab <n>
security-scan` — a named deployment target rather than a magic variable that
exists only to pick which of fourteen files is allowed to run.

Pilot for the remaining thirteen. Nothing else is converted until this one is
confirmed working end to end.
review-bot left a comment
Member

🤖 Automated review — model claude-sonnet-5 (complexity: normal), commit 96f11cb0bf6b50753db0660fa7e24f552e485d52.

Advisory review by the review-bot pipeline. It never approves; a human merges. Push new commits to get a fresh review.


Summary

This PR (#691) is a 14-line change to .woodpecker/security-scan.yml's trigger
gate: it replaces two evaluate: 'PIPELINE == "security-scan"' conditions
(cron and manual) with cron: daily-security-scan (first-class cron-name
match) and event: deployment + evaluate: 'CI_PIPELINE_DEPLOY_TARGET == "security-scan"', and adds an explanatory header comment. It's explicitly
scoped as a single-pipeline pilot for issue #582; the other thirteen
.woodpecker/*.yml files are untouched.

No blocking issues. The cron-name fix is well corroborated by the checkout
itself; the deployment-event replacement rests on a claim about Woodpecker's
internals that this repo has no way to verify locally, but the PR body already
flags that as unconfirmed and the pilot scope is designed to catch it before
it spreads to other pipelines.

🟡 Suggestions

  • Deployment-event mechanism is unverified from this repo. The header
    comment (.woodpecker/security-scan.yml:8-12) and PR body assert that
    CI_PIPELINE_DEPLOY_TARGET is populated for event: deployment pipelines
    (unlike manual-run PIPELINE, said to be dropped due to a Woodpecker 3.18
    builder bug). Woodpecker's server source isn't vendored in this repo, so I
    can't confirm metadata/environment.go:74 or builder.go:102-111 as cited.
    The PR author already acknowledges this ("Was ich noch nicht weiß…") and
    scopes the change as a pilot to be confirmed on the first real trigger — that's
    the right way to land an unverifiable-until-runtime claim, just flagging that
    the verification is still pending, not done.
  • Manual trigger path is fully removed, not just fixed. The old event: manual branch is dropped rather than repaired. That's consistent with the
    PR's narrative (manual-run variables are said to be unreliable pre-3.18.1),
    but it does mean there is now no way to hand-trigger security-scan from
    the Woodpecker UI's "run manually" flow — only cron or pipeline deploy ... security-scan. Worth confirming that's the intended operational model
    going forward (the CLI command in the header comment implies yes).
  • security-fix-agent's when: branch: master gate (.woodpecker/security-scan.yml:52-55, unchanged by this diff) is worth a second look under the new deployment trigger. If pipeline deploy is invoked against a pipeline number that ran on a non-master branch, it's not obvious from this repo whether the resulting deployment pipeline's branch metadata is master or the original run's branch. If it's the latter, deployment-triggered runs could silently skip the fix-agent step. Since this is pre-existing code untouched by the diff and the whole thing is a pilot to be validated on first trigger anyway, this doesn't need to block the PR — just flag it as something to watch when the deployment trigger is first exercised for real.

Tests

No automated tests apply here — this is a Woodpecker pipeline YAML trigger
change, not application code, and the repo has no CI-config test harness for
.woodpecker/*.yml (confirmed by directory listing). The PR's own stated
verification plan (confirm end-to-end after merge, pilot scope before
extending to other 13 pipelines) is the appropriate substitute for this kind
of change. The cron-name portion is de-risked in advance by two working
precedents already in the repo: alert-monitor.yml:38-39 (event: cron /
cron: alert-monitor) and e2e.yml:13-14 (event: cron / cron: daily-security-scan), both using the same first-class cron: matcher this
PR now adopts.

Recommendation

Looks reasonable — no blocking issues; comments above are optional.

🤖 **Automated review** — model `claude-sonnet-5` (complexity: normal), commit `96f11cb0bf6b50753db0660fa7e24f552e485d52`. > Advisory review by the `review-bot` pipeline. It never approves; a human merges. Push new commits to get a fresh review. --- ## Summary This PR (#691) is a 14-line change to `.woodpecker/security-scan.yml`'s trigger gate: it replaces two `evaluate: 'PIPELINE == "security-scan"'` conditions (cron and manual) with `cron: daily-security-scan` (first-class cron-name match) and `event: deployment` + `evaluate: 'CI_PIPELINE_DEPLOY_TARGET == "security-scan"'`, and adds an explanatory header comment. It's explicitly scoped as a single-pipeline pilot for issue #582; the other thirteen `.woodpecker/*.yml` files are untouched. No blocking issues. The cron-name fix is well corroborated by the checkout itself; the deployment-event replacement rests on a claim about Woodpecker's internals that this repo has no way to verify locally, but the PR body already flags that as unconfirmed and the pilot scope is designed to catch it before it spreads to other pipelines. ## 🟡 Suggestions - **Deployment-event mechanism is unverified from this repo.** The header comment (`.woodpecker/security-scan.yml:8-12`) and PR body assert that `CI_PIPELINE_DEPLOY_TARGET` is populated for `event: deployment` pipelines (unlike manual-run `PIPELINE`, said to be dropped due to a Woodpecker 3.18 builder bug). Woodpecker's server source isn't vendored in this repo, so I can't confirm `metadata/environment.go:74` or `builder.go:102-111` as cited. The PR author already acknowledges this ("Was ich noch nicht weiß…") and scopes the change as a pilot to be confirmed on the first real trigger — that's the right way to land an unverifiable-until-runtime claim, just flagging that the verification is still pending, not done. - **Manual trigger path is fully removed, not just fixed.** The old `event: manual` branch is dropped rather than repaired. That's consistent with the PR's narrative (manual-run variables are said to be unreliable pre-3.18.1), but it does mean there is now no way to hand-trigger `security-scan` from the Woodpecker UI's "run manually" flow — only cron or `pipeline deploy ... security-scan`. Worth confirming that's the intended operational model going forward (the CLI command in the header comment implies yes). - **`security-fix-agent`'s `when: branch: master` gate (`.woodpecker/security-scan.yml:52-55`, unchanged by this diff) is worth a second look under the new deployment trigger.** If `pipeline deploy` is invoked against a pipeline number that ran on a non-master branch, it's not obvious from this repo whether the resulting deployment pipeline's `branch` metadata is `master` or the original run's branch. If it's the latter, deployment-triggered runs could silently skip the fix-agent step. Since this is pre-existing code untouched by the diff and the whole thing is a pilot to be validated on first trigger anyway, this doesn't need to block the PR — just flag it as something to watch when the deployment trigger is first exercised for real. ## Tests No automated tests apply here — this is a Woodpecker pipeline YAML trigger change, not application code, and the repo has no CI-config test harness for `.woodpecker/*.yml` (confirmed by directory listing). The PR's own stated verification plan (confirm end-to-end after merge, pilot scope before extending to other 13 pipelines) is the appropriate substitute for this kind of change. The cron-name portion is de-risked in advance by two working precedents already in the repo: `alert-monitor.yml:38-39` (`event: cron` / `cron: alert-monitor`) and `e2e.yml:13-14` (`event: cron` / `cron: daily-security-scan`), both using the same first-class `cron:` matcher this PR now adopts. ## Recommendation Looks reasonable — no blocking issues; comments above are optional. <!-- pr-review:sha=96f11cb0bf6b50753db0660fa7e24f552e485d52 -->
k.renner merged commit f7034662ea into master 2026-08-28 07:47:38 +00:00
Sign in to join this conversation.