-
Notifications
You must be signed in to change notification settings - Fork 0
feat(release): build iklo release binary after tests pass (epic 005 T009) #52
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -181,9 +181,28 @@ created with the packaged CLI binary attached. | |
| packaging, checksums, and the atomic release-creation step are separate | ||
| tasks (T009-T012) — this workflow does nothing on a real tag push yet | ||
| beyond validating it. | ||
| - [ ] **T009** [US2] Build the `iklo` executable in release mode | ||
| - [x] **T009** [US2] Build the `iklo` executable in release mode | ||
| (`cargo build --release -p iklo-cli`) only after `make test` passes | ||
| (FR-002). | ||
|
Comment on lines
+184
to
186
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win Replace the stale The workflow in Update those references to Proposed documentation fix-(`cargo build --release -p iklo-cli`) only after `make test` passes
+(`cargo build --release -p iklo-cli --locked`) only after `cargo test --locked` passes
...
-then `make test`, then `cargo build --release -p
+then `cargo test --locked`, then `cargo build --release -p
iklo-cli --locked`
...
-Verified locally: `make test` exits 0, ...
+Verified locally: `cargo test --locked` exits 0, ...Also applies to: 189-190, 195-196 🤖 Prompt for AI Agents |
||
| **Done 2026-08-13**: added toolchain install + `Swatinem/rust-cache` | ||
| (same pinned SHAs as `ci.yml`, confirmed byte-identical) to | ||
| `release.yml`, then `make test`, then `cargo build --release -p | ||
| iklo-cli --locked` as the final step. `--locked` added after | ||
| self-review: a release artifact should fail loud on `Cargo.lock` drift | ||
| rather than silently building against unrecorded dependency versions. | ||
| No `make build` (debug) step — T009's scope is only the release build, | ||
| gated on tests passing, not a redundant debug build first. Verified | ||
| locally: `make test` exits 0, `cargo build --release -p iklo-cli | ||
| --locked` exits 0 and produces `target/release/iklo`. Also updated the | ||
| workflow's least-privilege header comment, which still described the | ||
| T008-only state before this task added test/build steps. | ||
| Follow-up fix | ||
| after cubic-dev-ai review: the test step now runs `cargo test --locked` | ||
| directly rather than `make test`'s plain `cargo test` -- an unlocked | ||
| test run silently rewrites a drifted `Cargo.lock`, which would have | ||
| defeated the release build's own `--locked` guarantee by the time it | ||
| ran. Scoped to `release.yml` only, not `Makefile`/`ci.yml` (those are a | ||
| separate concern about local-dev/PR-check strictness, not this task). | ||
| - [ ] **T010** [US2] Package and upload the built executable as a GitHub | ||
| Release asset (FR-003, SC-002). | ||
| - [ ] **T011** [US2] Generate and publish SHA-256 checksums for every | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
Repository: rsenna/iklo
Length of output: 25445
🏁 Script executed:
Repository: rsenna/iklo
Length of output: 13131
🌐 Web query:
GitHub Actions dependency caching cache contents not signed verified cache poisoning restored files execution official documentation💡 Result:
GitHub Actions cache contents are not signed or verified, meaning restored files can be modified by unauthorized parties if they successfully poison the cache [1][2]. Because restored files are treated as trusted inputs and can be executed within a workflow, cache poisoning can lead to arbitrary code execution in privileged workflows [3][4][1]. Official documentation and security guidance emphasize the following regarding this risk: Security Risks Cache Contents Are Not Verified: GitHub does not perform cryptographic signing or integrity verification of cache archives upon restore [1][2]. Any workflow run with read access to a cache can extract its contents, and those contents are then used directly in subsequent steps [1][2]. Cache Poisoning Path: An attacker who gains the ability to write to the cache (e.g., through compromised dependencies or code execution in an unprivileged workflow) can inject malicious files [4][5]. Because privileged workflows (such as those running on the default branch) often restore these caches, the malicious files can be executed, leading to a compromise [3][6]. Scope and Access: Cache access is largely determined by branch structure [4][6]. Caches in the default branch are generally accessible to workflows running on feature branches, creating an asymmetry that attackers exploit to move from low-trust to high-trust execution contexts [6]. Mitigation and Best Practices Treat Restored Files as Untrusted: Always assume that files restored from the cache may be malicious [3][2]. Do not store sensitive information (e.g., secrets, credentials, tokens) in the cache, as they can be read by anyone capable of opening a pull request [1][2]. Restrict Cache Write Access: Limit which workflows have permission to write to the cache [1][2]. Use trusted triggers for workflows that modify caches and leverage environment protection rules to restrict access further [1][2]. Path Validation: While the core caching mechanism remains unverified, the
@actions/cachetoolkit has introduced an optional, opt-in path validation feature (DownloadOptions.pathValidation) [7]. When enabled (e.g., set to 'error'), this validates that extracted files resolve within the expected directories, preventing malicious archive entries from escaping the cache paths [7]. Harden Workflows: Follow strict workflow security practices to prevent vulnerabilities like script injection or execution of untrusted code, which are common entry points for cache poisoning [1][2][8]. Never run untrusted code in the context of the default branch [4][9].Citations:
@actions/cacheoptional path validation during restore actions/toolkit#2414🌐 Web query:
zizmor cache-poisoning GitHub Actions rust-cache cache-targets release workflow💡 Result:
The "cache-poisoning" audit in zizmor is a security check designed to detect workflows that are potentially vulnerable to cache-based supply chain attacks [1][2]. It specifically flags workflows—often those involved in publishing artifacts—that use cache-aware actions (like Swatinem/rust-cache) without sufficient protection, which could allow an attacker with access to a repository's GitHub Actions environment to poison the cache with malicious content [1][2][3]. The vulnerability arises because GitHub Actions caches are often shared or predictable [1][2]. If a release or publishing workflow restores a cache that was populated by a less-trusted workflow (e.g., one triggered by a pull request), an attacker may be able to inject malicious dependencies or artifacts that are then included in the final, published release [1][2][3]. To remediate these findings for Swatinem/rust-cache, zizmor recommends setting the following in your workflow step [1][2]: - Use lookup-only: true: This configures the action to only restore from the cache without ever saving new state to it, effectively neutralizing the poisoning vector [1][2][3]. While some users have attempted to use save-if: false to achieve the same goal, zizmor's audit logic has historically struggled to recognize this as a sufficient mitigation, leading to potential false positives [4]. The primary, officially recommended approach within the context of zizmor's security checks remains lookup-only: true [1][2]. Beyond Swatinem/rust-cache, this audit covers numerous other popular actions that interact with the GitHub cache, such as actions/setup-node, actions/setup-python, and astral-sh/setup-uv [1][5][6]. The general principle for all these actions is to disable cache-writing behaviors (via specific action inputs) or to remove caching entirely in jobs that produce sensitive release artifacts [1][2][3].
Citations:
🏁 Script executed:
Repository: rsenna/iklo
Length of output: 20477
🌐 Web query:
site:docs.zizmor.sh/audits cache-poisoning lookup-only rust-cache💡 Result:
The term cache-poisoning in the context of Swatinem/rust-cache and similar GitHub Actions refers to a security vulnerability where an attacker with access to a GITHUB_TOKEN can inject malicious payloads into the GitHub Actions cache [1]. If a subsequent workflow run restores this poisoned cache, it can lead to arbitrary code execution during the workflow runtime, which is particularly dangerous in release workflows that generate artifacts [1]. To mitigate this risk, security tools like zizmor flag the use of caching in sensitive workflows (such as those triggered by release or tag events) [1]. Using the lookup-only: true configuration for Swatinem/rust-cache is a recommended pattern to prevent this vulnerability [1]. Setting lookup-only to true ensures the action only attempts to restore existing cache entries without uploading or updating the cache during that specific workflow execution, thereby preventing an attacker from poisoning the cache with malicious build state [1].
Citations:
🏁 Script executed:
Repository: rsenna/iklo
Length of output: 24794
Do not restore or save caches in the release job.
Swatinem/rust-cacherestores unverified registry, git,target, and${CARGO_HOME}/binstate. Its post step can also save state after a cache miss. Remove this step or disable it for tag pushes.lookup-only: "true"alone is insufficient; combine it withsave-if: "false"if the action remains.🧰 Tools
🪛 zizmor (1.29.0)
[error] 43-43: runtime artifacts potentially vulnerable to a cache poisoning attack (cache-poisoning): enables caching by default
(cache-poisoning)
🤖 Prompt for AI Agents
Sources: MCP tools, Linters/SAST tools