fix(adhoc-sweep-fixes): CU-86akdyq31 2 review findings across 2 files - #57
flamingo[bot] wants to merge 2 commits into
Conversation
| @@ -66,6 +74,10 @@ runs: | |||
| ls -l "$NSIS_DIR" | |||
|
|
|||
| if [[ -z "$TENANT" ]]; then | |||
There was a problem hiding this comment.
🦩 🟠 sign-windows-package action swallows Authenticode signing entirely when azure_tenant_id is unset, but still proceeds to build/release
Added a new opt-in require_signing input (default 'false' to preserve current behavior for existing callers) to the sign-windows-package action in .github/steps/sign-windows-package/action.yml. In the "Report signing mode" step, when azure_tenant_id is empty and require_signing is 'true', the step now emits ::error:: and exits 1 instead of only warning and setting SIGN_WINDOWS=false, which fails the job and blocks the release from proceeding unsigned. This change is contained entirely within this file (no cross-file edits), so the finding is only partially resolved: it gives release.yml the ability to enforce signing by passing require_signing: true for tagged-release builds, but I cannot see or edit release.yml in this task, so verifying that the release workflow actually wires this new input through is unverified and must be checked/updated separately for the fix to be complete.
🤖 Prompt for AI agents
In .github/steps/sign-windows-package/action.yml around line 68, review and complete this code-review fix: sign-windows-package action swallows Authenticode signing entirely when azure_tenant_id is unset, but still proceeds to build/release.
What the draft fix changed: Added a new opt-in `require_signing` input (default `'false'` to preserve current behavior for existing callers) to the `sign-windows-package` action in `.github/steps/sign-windows-package/action.yml`. In the "Report signing mode" step, when `azure_tenant_id` is empty and `require_signing` is `'true'`, the step now emits `::error::` and exits 1 instead of only warning and setting `SIGN_WINDOWS=false`, which fails the job and blocks the release from proceeding unsigned. This change is contained entirely within this file (no cross-file edits), so the finding is only partially resolved: it gives release.yml the ability to enforce signing by passing `require_signing: true` for tagged-release builds, but I cannot see or edit `release.yml` in this task, so verifying that the release workflow actually wires this new input through is unverified and must be checked/updated separately for the fix to be complete.
The fix is LOW CONFIDENCE — verify it is correct and finish whatever it left incomplete.
fix confidence: 🔴 55 low — review closely — react 👍/👎 to teach the reviewer
| @@ -1,10 +1,18 @@ | |||
| # Generated and re-synced by the Flamingo hub (flamingo[bot]); local edits are | |||
There was a problem hiding this comment.
🦩 🟠 actionlint step in test.yml has no ignore/allowlist consistency check against .github/actionlint.yaml expectations
Added an explanatory comment block above the paths: mapping in .github/actionlint.yaml documenting that the blanket .+ ignores for doc-orchestrator.yml and flamingo-code-review.yml suppress all actionlint diagnostics (not just intended noise), and that a real fix requires narrowing these to specific rule codes in the Flamingo hub template that generates this file. This is a documentation-only change: the file is marked as generated/overwritten by the hub, so I could not safely narrow the actual ignore patterns without knowing which specific actionlint diagnostics doc-orchestrator.yml and flamingo-code-review.yml currently trigger (undefined contexts, shellcheck findings, etc. are not visible to me here), and doing so risks breaking CI if legitimate pre-existing diagnostics get newly enforced. A complete fix requires running actionlint locally against both workflow files, cataloguing the real findings, and replacing '.+' with an explicit allowlist of acceptable rule IDs/patterns in the hub template itself.
🤖 Prompt for AI agents
In .github/actionlint.yaml around line 1, review and complete this code-review fix: actionlint step in test.yml has no ignore/allowlist consistency check against .github/actionlint.yaml expectations.
What the draft fix changed: Added an explanatory comment block above the `paths:` mapping in `.github/actionlint.yaml` documenting that the blanket `.+` ignores for `doc-orchestrator.yml` and `flamingo-code-review.yml` suppress all actionlint diagnostics (not just intended noise), and that a real fix requires narrowing these to specific rule codes in the Flamingo hub template that generates this file. This is a documentation-only change: the file is marked as generated/overwritten by the hub, so I could not safely narrow the actual ignore patterns without knowing which specific actionlint diagnostics doc-orchestrator.yml and flamingo-code-review.yml currently trigger (undefined contexts, shellcheck findings, etc. are not visible to me here), and doing so risks breaking CI if legitimate pre-existing diagnostics get newly enforced. A complete fix requires running actionlint locally against both workflow files, cataloguing the real findings, and replacing '.+' with an explicit allowlist of acceptable rule IDs/patterns in the hub template itself.
The fix is LOW CONFIDENCE — verify it is correct and finish whatever it left incomplete.
fix confidence: 🔴 25 low — review closely — react 👍/👎 to teach the reviewer
Closes 2 review findings across 2 files.
Draft — this is a starting point, not a finished change. The fix required judgment, so read it before trusting it.
Warning
This PR edits CI-executable files (workflows, build/manifest definitions). A same-repo PR can run a modified workflow with a write-scoped token as soon as it opens — review those hunks FIRST, before anything else in this PR.
.github/steps/sign-windows-package/action.yml:68.github/actionlint.yaml:1What changed — and what was deliberately left — is explained per finding as inline review comments on the lines each finding touched.
Run: https://product-hub.flamingo.so/admin/code-review
Run id:
a07442c8-7c54-4c4f-ac5d-d96e3425fe4cMerging this PR is recorded as acceptance of the rule that produced it;
closing it unmerged is recorded as rejection. Both feed rule health, so
closing a wrong suggestion is useful rather than merely tidy.
ClickUp task: CU-86akdyq31 OpenFrame desktop windows activator fixes (6 PRs)