Repository navigation
fix: stamp nightly Cargo versions portably on macOS - #835
Conversation
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| echo "Patched version to $v" | ||
| # One portable implementation keeps the bundle, Rust runtime floor | ||
| # checks and frontend aligned. BSD sed does not support GNU's 0,/.../. | ||
| run: node scripts/set-nightly-version.mjs '${{ inputs.updater_version }}' |
There was a problem hiding this comment.
SUGGESTION: Pass the version through env instead of interpolating the expression into the shell command
${{ inputs.updater_version }} is expanded by the Actions runner before bash parses the line, and it is substituted inside single quotes, so a value containing a ' breaks out of the quoting and runs as arbitrary shell. The regex validation in set-nightly-version.mjs happens afterwards and cannot protect the shell step. updater_version is currently produced by the nightly gate from tauri.conf.json, so the practical exposure is low, but the previous sed/inline-node version had the same exposure and the script now makes it easy to close: bind the input to an environment variable on the step and quote the variable.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
There was a problem hiding this comment.
Addressed in a998fc0: the step now binds NIGHTLY_VERSION through env and passes "$NIGHTLY_VERSION" to Node. The runtime validation no longer relies on shell quoting of a GitHub expression.
| import { join, resolve } from "node:path"; | ||
| import { afterEach, describe, expect, it } from "vitest"; | ||
|
|
||
| const script = resolve("scripts/set-nightly-version.mjs"); |
There was a problem hiding this comment.
SUGGESTION: Test location does not mirror src/ as required by .rules/testing.md
The rule states that test files live in a tests/ directory mirroring src/, with source logic in src/utils/. This file exercises scripts/set-nightly-version.mjs, so neither tests/utils/ nor a src/utils/ counterpart applies. A tests/scripts/set-nightly-version.test.ts would keep the directory mapping unambiguous and avoid implying a src/utils/nightlyVersion.ts module exists.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
There was a problem hiding this comment.
Moved to tests/scripts/set-nightly-version.test.ts in a998fc0. All seven regression tests still pass.
Code Review SummaryStatus: No Issues Found | Recommendation: Merge The two findings from the previous review are resolved by the latest commit Re-verified on the current HEAD ( Files Reviewed (3 files)
NotesNo new issues in the incremental diff. Previous Review Summary (commit 84ac156)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 84ac156)Status: 2 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)SUGGESTION
Files Reviewed (3 files)
NotesThe core fix is sound: the script validates the version shape and the One observation outside the inline-comment scope: the script uses repo-root-relative paths, which is correct for the Reviewed by free · Input: 0 · Output: 0 · Cached: 0 |
Preview buildThe preview build of commit |
Problem
Release-artifact verification of nightly 0.25.1-5 found that the signed macOS app and latest.json advertise 0.25.1-5, while
tabularis --versionreports 0.25.0. The SQL Server 1.0.0-beta.3 plugin is consequently rejected by the runtime version gate. Reproduced against the actual signed arm64 bundle in an isolated MCP home.The workflow uses the GNU sed address
0,/.../, which silently leaves Cargo.toml unchanged on BSD sed.Fix
Validation
Rollout
Publish the corrected nightly as 0.25.1-6; do not overwrite already-published 0.25.1-5 binaries. Refs TabularisDB/tabularis-sqlserver-plugin#26.