feat: bootstrapper i18n and latest OpenCode default#200
Conversation
…scope, shell scope, discovery, logger) - Add 1MB max native message size with early rejection to prevent memory DoS - Validate CORS origins from extension messages, reject wildcards, cap at 10 - Restrict native host port range to 4096-4127 (32 slots) with PID tracking - Scope Windows shell:true to npx-only to reduce command injection surface - Remove dev extension ID from production allowlist, default to strict mode - Gate native host logger behind CALY_OC_NATIVE_HOST_DEBUG=1 env var - Document public key placement via CALY_BUILD_OC_EXT_PUBLIC_KEY_B64 for discovery
…ning Introduce a cross-platform bootstrapper with persistent step-based installer UI and release artifact automation. Enforce native-host launcher version parity by preferring managed installs, rejecting mismatched global binaries, and falling back to versioned npx when needed.
🦋 Changeset detectedLatest commit: ee96589 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughThis PR adds a complete cross-platform Go bootstrapper application for installing CalyCode CLI, with multi-language UI support and automated Node.js/npm orchestration, alongside comprehensive security hardening of the native host feature including session tracking, CORS validation, message size limits, and port range restrictions. ChangesBootstrapper Installation & Native Host Hardening
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/cli/src/commands/opencode/implementation.ts (1)
479-588:⚠️ Potential issue | 🔴 Critical | ⚡ Quick win
restartcan still kill unrelated local processes.
allowedPidsis never checked beforetaskkill,kill -9, orfuser -k, so a restart on a colliding port can terminate any listener in the native-host range, not just a session this host launched. The Unix fast path is especially problematic becausefuser -kbypasses the allowlist entirely. Intersect discovered PIDs withallowedPidsfirst, and avoid blind kill helpers here.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/cli/src/commands/opencode/implementation.ts` around lines 479 - 588, In killProcessOnPort ensure we never kill processes outside the allowlist: instead of blindly calling taskkill, kill -9, or fuser -k, first collect candidate PIDs (Windows: pidsToKill parsed from netstat; Unix: enumerate PIDs via fuser or lsof without using -k), convert them to numbers and intersect with the provided allowedPids Set<number>, then only attempt to kill the intersection; if the intersection is empty log a message via logInfo and return false. Update the Windows branch (pidsToKill loop), the Unix fast path (remove fuser -k and instead gather PIDs into pids array), and the lsof handling (parse lsofOutput into numeric pids) to perform this allowlist filtering before any execSync kill/taskkill calls, and ensure logError is used when kills fail.
🧹 Nitpick comments (2)
plans/native-host-security-hardening.md (2)
650-658: ⚡ Quick winExpand sensitive field redaction list.
The redaction list covers common sensitive fields (
token,apiKey,secret,password,authorization), but consider adding:
bearerToken/beareraccessToken/refreshTokenprivateKey/privateKeyIdsessionId/sessionToken- Case-insensitive matching (fields might be
apikey,ApiKey, etc.)🔒 Enhanced redaction implementation
const SENSITIVE_FIELD_PATTERNS = [ /token/i, /key/i, /secret/i, /password/i, /auth/i, /bearer/i, /session/i, ]; const sanitizedMsg = { ...msg }; for (const [key, value] of Object.entries(sanitizedMsg)) { if (SENSITIVE_FIELD_PATTERNS.some(pattern => pattern.test(key))) { sanitizedMsg[key] = '[REDACTED]'; } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@plans/native-host-security-hardening.md` around lines 650 - 658, The current redaction in handleMessage only covers a small fixed set of keys; expand it to a case-insensitive, pattern-based check so fields like bearerToken, accessToken, privateKey, sessionId, and variants (ApiKey, apikey, Authorization) are caught. Replace the fixed-for loop that constructs sanitizedMsg before calling logger.log with a scan over Object.entries(sanitizedMsg) and match keys against a SENSITIVE_FIELD_PATTERNS array (e.g., /token/i, /key/i, /secret/i, /password/i, /auth/i, /bearer/i, /session/i) and set matched values to '[REDACTED]'; keep the symbol names handleMessage and logger.log to locate where to apply this change. Ensure you only mutate the copied sanitizedMsg and not the original msg.
620-645: ⚡ Quick winLog rotation implementation needs fsync consideration.
The log rotation logic correctly manages file size and maintains a rolling set of log files. However, consider whether
fs.renameSyncoperations should be followed by explicit sync to ensure durability in case of crashes during rotation. Additionally, the rotation happens in the critical logging path - consider whether failed rotation should disable logging or continue with warnings.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@plans/native-host-security-hardening.md` around lines 620 - 645, rotateIfNeeded currently does synchronous renames/unlinks without ensuring on-disk durability or clear non-blocking behavior: update rotateIfNeeded (and references to MAX_LOG_SIZE, MAX_LOG_FILES, logPath) to (1) after each fs.renameSync or fs.unlinkSync ensure durability by opening the target file/dir fds and calling fs.fsyncSync on the moved/created file and then fs.fsyncSync on the parent directory to persist directory metadata, (2) wrap those fsync calls in try/catch and convert any failures into logger.warn (do not disable logging), and (3) avoid blocking the critical logging path by performing the rotation work off the hot path (e.g., schedule async work or use a background worker/queue) so failures are non-fatal and only emit warnings from the rotation logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/build-bootstrapper.yml:
- Around line 83-86: The current workflow step that force-updates the floating
tag "bootstrapper-latest" runs for any event except workflow_dispatch; change
the condition so it only runs on push events to avoid moving the tag during
release/tag events — replace the existing if line ("if: github.event_name !=
'workflow_dispatch'") with a push-only check (e.g., "if: github.event_name ==
'push'") so the block that runs git tag -f bootstrapper-latest and git push -f
origin bootstrapper-latest only executes on push events.
- Around line 23-29: Pin the GitHub actions used (actions/checkout and
actions/setup-go) to specific commit SHAs instead of floating tags to harden
supply chain; for the Checkout step adjust the persist-credentials
setting—either keep persist-credentials: true so the later "Update
bootstrapper-latest tag" git push works, or set persist-credentials: false and
add an explicit authentication step before that push (e.g., gh auth login or
setting GITHUB_TOKEN) to ensure git push can authenticate; move the release tag
usage in the "gh release upload" run into an environment variable (e.g., env:
RELEASE_TAG: ${{ github.event.release.tag_name }}) and reference it as
"$RELEASE_TAG" in the shell command to avoid shell-interpretation issues; verify
behavior on release.published matches expectations (checkout will be at the
release ref).
In `@bootstrapper/install/cli.go`:
- Around line 23-25: When cmd.Run() returns an error in the install flow, do not
return an empty string if stderr.String() is empty; instead return a meaningful
message that includes the command error. Update the error handling around
cmd.Run() in cli.go so that when err != nil you return either stderr.String() if
non-empty or err.Error() (or a concatenation of both) so callers never receive
an empty error string; key symbols to change: the cmd.Run() error check and the
use of stderr.String().
In `@bootstrapper/install/node_darwin.go`:
- Around line 40-42: The exec.Command call that builds cmd currently wraps the
$(curl ...) invocation in extra double-quotes so bash -c treats the downloaded
script as a literal token; update the exec.Command argument passed to bash -c
(the exec.Command(...) that assigns to cmd) to remove the surrounding quotes so
the output of $(curl -fsSL
https://raw.githubusercontent.com/Homebrew/install/HEAD/install.sh) is executed
by bash. Ensure the resulting string passed to exec.Command is exactly the
command substitution without the extra double quotes.
In `@bootstrapper/install/node_unsupported.go`:
- Around line 6-8: Replace the hardcoded English string returned by InstallNode
with a localized message lookup; change the function InstallNode to call the
project's i18n/localization helper (e.g., the translation function used
elsewhere like T, i18n.T, translate, or similar) to return the localized
installer text and URL, ensuring you use the same message key used for similar
installer/error messages so translations are picked up consistently.
In `@bootstrapper/Makefile`:
- Around line 21-43: The windows, darwin-amd64 and darwin-arm64 Makefile targets
write binaries into $(OUT_DIR) but never create that directory, causing
first-run failures; add a step that ensures $(OUT_DIR) exists before invoking go
build (e.g. run mkdir -p $(OUT_DIR) or add a small prerequisite target that
creates $(OUT_DIR) and make windows/darwin-amd64/darwin-arm64 depend on it) so
the build commands that produce $(BINARY)-... artifacts and use $(MODULE) always
have a valid output directory.
In `@bootstrapper/ui/dialog_darwin.go`:
- Around line 45-53: ShowWelcome currently passes `'"OK", "Cancel"'` to
displayDialog but never observes the user's "Cancel" selection, which is
misleading; either remove the unused Cancel button or handle its return to abort
installation. Fix by updating ShowWelcome to only present the actionable button
(e.g., change the buttons argument passed to displayDialog to just `"OK"`) or by
capturing displayDialog's result in ShowWelcome and branching (e.g., if the
returned value indicates Cancel, stop the installation flow); locate ShowWelcome
and the call to displayDialog to implement the chosen approach.
In `@packages/cli/scripts/README.md`:
- Around line 59-63: The fenced code block containing the user agent mapping
should include a language specifier for proper syntax highlighting; update the
triple-backtick fence that wraps the three-line mapping (the block starting with
"win32 → calycode-installer-windows-x64.exe") to use a plain-text specifier
such as ```text so the lines display with correct highlighting.
- Around line 94-117: Update the fenced code block containing the directory tree
(the block that starts with "bootstrapper/" and includes "scripts/installer/"
and "scripts/dev/") in packages/cli/scripts/README.md by adding a language
specifier to the opening fence (e.g., change ``` to ```text or ```plaintext) so
the block is explicitly marked as plain text for consistent rendering and
markdown linting; keep the existing closing fence unchanged.
- Around line 14-16: Remove the extra blank line between the two blockquote
lines so the two lines beginning with "**macOS note:** Unzip the download..."
and "Apple requires notarization..." remain in the same markdown blockquote;
edit the README.md blockquote to join those lines without an empty line in
between.
In `@packages/cli/src/commands/opencode/implementation.ts`:
- Around line 730-747: The merged origin list returned by getAllowedCorsOrigins
currently bypasses the same validation applied in filterAndValidateOrigins (so
CALY_EXTRA_CORS_ORIGINS and env-sourced values can include "*" or exceed
MAX_CORS_ORIGINS); ensure you run filterAndValidateOrigins (or equivalent
validation logic) against the final combined origins array produced in
getAllowedCorsOrigins (and before passing into the OpenCode spawn/--cors flag),
trimming duplicates and enforcing MAX_CORS_ORIGINS and isValidCorsOrigin checks
for entries from CALY_EXTRA_CORS_ORIGINS, msg.origins, and any other sources so
the final list is validated and capped.
- Around line 244-248: The code in ensureManagedOpencodeInstalled (and the
similar block around lines 309-323) reuses any existing managed directory for
the given version string, which pins the literal "latest"; change the logic so
that when version === 'latest' you do not short-circuit to an existing
getManagedOpencodeBinPath(version) directory or instead resolve 'latest' to a
concrete release before reuse. Concretely: update ensureManagedOpencodeInstalled
(and the duplicate block) to either (A) call a function to resolve the actual
latest release version and use that concrete version for
getManagedOpencodeBinPath/pruneManagedOpencodeVersions, or (B) skip the cache
check when version === 'latest' so the install proceeds and overwrites/updates
the managed directory. Ensure references to getManagedOpencodeBinPath and
pruneManagedOpencodeVersions are adjusted accordingly.
In `@plans/native-host-security-hardening.md`:
- Around line 234-237: The comment and rationale around
NATIVE_HOST_PORT_RANGE_START, NATIVE_HOST_PORT_RANGE_SIZE and
NATIVE_HOST_PORT_RANGE_END are incorrect: Chrome Native Messaging does not use
TCP/UDP ports and 4096–4127 is inside the IANA Registered Ports range
(1024–49151), not the ephemeral/dynamic range. Update the inline comment and any
documentation to (1) state that 4096–4127 is the dedicated local server port
block used by our spawned local server (not Chrome Native Messaging), (2)
correct the “registered/dynamic” wording to “IANA Registered Ports
(1024–49151)”, (3) explain the choice of 32 slots starting at 4096 (e.g., for
managed PID/session tracking and predictable allocation) and note behavior when
a port is occupied (including existing boundary validation and stop-message
handling), and (4) document whether and how users can override the start/size
constants (NATIVE_HOST_PORT_RANGE_START, NATIVE_HOST_PORT_RANGE_SIZE) or
configure alternate ports.
---
Outside diff comments:
In `@packages/cli/src/commands/opencode/implementation.ts`:
- Around line 479-588: In killProcessOnPort ensure we never kill processes
outside the allowlist: instead of blindly calling taskkill, kill -9, or fuser
-k, first collect candidate PIDs (Windows: pidsToKill parsed from netstat; Unix:
enumerate PIDs via fuser or lsof without using -k), convert them to numbers and
intersect with the provided allowedPids Set<number>, then only attempt to kill
the intersection; if the intersection is empty log a message via logInfo and
return false. Update the Windows branch (pidsToKill loop), the Unix fast path
(remove fuser -k and instead gather PIDs into pids array), and the lsof handling
(parse lsofOutput into numeric pids) to perform this allowlist filtering before
any execSync kill/taskkill calls, and ensure logError is used when kills fail.
---
Nitpick comments:
In `@plans/native-host-security-hardening.md`:
- Around line 650-658: The current redaction in handleMessage only covers a
small fixed set of keys; expand it to a case-insensitive, pattern-based check so
fields like bearerToken, accessToken, privateKey, sessionId, and variants
(ApiKey, apikey, Authorization) are caught. Replace the fixed-for loop that
constructs sanitizedMsg before calling logger.log with a scan over
Object.entries(sanitizedMsg) and match keys against a SENSITIVE_FIELD_PATTERNS
array (e.g., /token/i, /key/i, /secret/i, /password/i, /auth/i, /bearer/i,
/session/i) and set matched values to '[REDACTED]'; keep the symbol names
handleMessage and logger.log to locate where to apply this change. Ensure you
only mutate the copied sanitizedMsg and not the original msg.
- Around line 620-645: rotateIfNeeded currently does synchronous renames/unlinks
without ensuring on-disk durability or clear non-blocking behavior: update
rotateIfNeeded (and references to MAX_LOG_SIZE, MAX_LOG_FILES, logPath) to (1)
after each fs.renameSync or fs.unlinkSync ensure durability by opening the
target file/dir fds and calling fs.fsyncSync on the moved/created file and then
fs.fsyncSync on the parent directory to persist directory metadata, (2) wrap
those fsync calls in try/catch and convert any failures into logger.warn (do not
disable logging), and (3) avoid blocking the critical logging path by performing
the rotation work off the hot path (e.g., schedule async work or use a
background worker/queue) so failures are non-fatal and only emit warnings from
the rotation logic.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9bf8993f-f43f-4a74-a490-05b8e1134184
📒 Files selected for processing (28)
.github/workflows/build-bootstrapper.yml.github/workflows/release.ymlbootstrapper/Makefilebootstrapper/go.modbootstrapper/i18n/i18n.gobootstrapper/install/cli.gobootstrapper/install/exec_darwin.gobootstrapper/install/exec_unsupported.gobootstrapper/install/exec_windows.gobootstrapper/install/messages.gobootstrapper/install/node.gobootstrapper/install/node_darwin.gobootstrapper/install/node_unsupported.gobootstrapper/install/node_windows.gobootstrapper/main.gobootstrapper/ui/dialog_darwin.gobootstrapper/ui/dialog_unsupported.gobootstrapper/ui/dialog_windows.gobootstrapper/ui/messages.gobootstrapper/ui/session.gobootstrapper/ui/session_darwin.gobootstrapper/ui/session_unsupported.gobootstrapper/ui/session_windows.godocs/extension-installer-flow.mdpackages/cli/scripts/README.mdpackages/cli/src/commands/opencode/implementation.tspackages/cli/src/utils/host-constants.tsplans/native-host-security-hardening.md
| - name: Checkout | ||
| uses: actions/checkout@v4 | ||
|
|
||
| - name: Setup Go | ||
| uses: actions/setup-go@v5 | ||
| with: | ||
| go-version: "1.21" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE=".github/workflows/build-bootstrapper.yml"
if [ ! -f "$FILE" ]; then
echo "Missing $FILE"
exit 1
fi
echo "=== File: $FILE ==="
nl -ba "$FILE" | sed -n '1,200p'Repository: calycode/xano-tools
Length of output: 180
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE=".github/workflows/build-bootstrapper.yml"
if [ ! -f "$FILE" ]; then
echo "Missing $FILE"
exit 1
fi
echo "=== File: $FILE ==="
awk '{printf "%4d\t%s\n", NR, $0}' "$FILE" | sed -n '1,200p'Repository: calycode/xano-tools
Length of output: 3447
🌐 Web query:
actions/checkout persist-credentials default true behavior git push
💡 Result:
The persist-credentials input for the actions/checkout GitHub Action has a default value of true [1][2]. When persist-credentials is set to true (the default), the action configures the runner's local Git environment with the authentication token provided to the workflow (typically the GITHUB_TOKEN) [1][3]. This allows subsequent steps in your job to execute authenticated Git commands—such as git fetch or git push—without requiring additional authentication setup [1][2][3]. Recent updates to actions/checkout have improved the security of this feature by storing these credentials in a separate file within the runner's temporary directory ($RUNNER_TEMP) rather than directly inside the repository's .git/config [2][3][4]. The action still performs cleanup by removing these credentials during the post-job phase [1][3]. If you set persist-credentials: false, the action will not persist these credentials into the Git configuration [1][5]. While this is considered a recommended security best practice to reduce potential credential exposure [5][6][7], it means you must handle Git authentication manually in any subsequent steps where you need to perform authorized operations like git push (e.g., by using an alternative authentication method or configuring credentials explicitly) [8][9][7].
Citations:
- 1: https://github.com/actions/checkout/blob/85e6279cec87321a52edac9c87bce653a07cf6c2/README.md
- 2: https://github.com/actions/checkout
- 3: https://github.com/actions/checkout/blob/main/README.md
- 4: Persist creds to a separate file actions/checkout#2286
- 5: https://yossarian.net/til/post/actions-checkout-can-leak-github-credentials/
- 6: Always emit
persist-credentials: falsewith actions/checkout github/gh-aw#2641 - 7: https://release-plz.dev/docs/github/persist-credentials
- 8: Replace persist-credentials with explicit git re-authentication github/gh-aw#2664
- 9: https://stackoverflow.com/questions/78825827/want-to-push-to-my-github-repository-from-my-github-actions-workflow
🌐 Web query:
actions/checkout release.published event which ref it checks out github.sha tag commit
💡 Result:
For a workflow triggered by the GitHub release event types (e.g., on: release: types: [published]), actions/checkout checks out the ref or SHA for the workflow trigger event; by default it uses “the reference or SHA for that event” (unless you set inputs.ref explicitly) [1]. So, if you configure actions/checkout without specifying ref, the commit it checks out corresponds to the commit SHA that GitHub Actions associates with that release event (i.e., the event’s ref/SHA), rather than a branch name [1]. If you want to be explicit in the release.published workflow, set the checkout ref to the commit SHA you intend to build (commonly derived from the event payload via the github.* context), using actions/checkout’s ref input which accepts a branch, tag, or SHA [1]. Note: This is consistent with actions/checkout’s documented behavior that the default checkout for the “repository that triggered a workflow” is the reference or SHA for that event, and it only fetches a single commit by default for that ref/SHA [1].
Citations:
🌐 Web query:
GitHub tag name allowed characters restrictions for shell injection
💡 Result:
Git/GitHub tag names aren’t “specially restricted” to prevent shell injection; instead, Git allows a fairly wide character set, and shell-injection risk comes from how tag strings are interpolated into shell commands by downstream tooling. 1) What characters are allowed for Git tag names (the practical restrictions) - Git is very permissive about what characters may appear in branch and tag names; you may need to escape/quote special characters when using the names in a command-line shell. GitHub’s docs note that Git has “very few restrictions” and only calls out exceptions like slash placement/duplication. [1] - For the underlying ref-name rules, Git’s check-ref-format documentation specifies key constraints for reference names: - A ref component cannot begin with “.” and cannot end with “.lock”. [2] - A ref cannot begin or end with “/” and cannot contain multiple consecutive “/”. [2] - A ref must contain at least one “/” in general (because refs are in hierarchies like heads/ or tags/); for one-level refs this rule can be waived with an option, but normal refnames (like refs/tags/…) do include “/”. [2] - A refname cannot contain the sequence “@{”. [2] (These are the main “format” restrictions Git enforces; they’re about ref parsing/semantics, not about preventing shell meta-characters like; or |.) - GitHub also adds a small additional push-time restriction on names that look like Git object IDs and on names beginning with “refs/” to avoid confusion. [1] 2) Why this relates to shell injection (what the “allowed characters restrictions” really mean) - The relevant security issue is not “GitHub’s tag character policy”; it’s whether a tag string (often from GITHUB_REF / user input) is inserted into a shell command string and executed with a shell. - A concrete example: an action had a bug where attacker-controlled “tag” input could run arbitrary shell commands; the fix was to properly quote/escape the tag string before passing it to exec. [3] 3) Safe guidance for preventing shell injection with tag values - Treat tag strings as untrusted input and do not concatenate them into shell command strings. - Prefer APIs that avoid a shell (e.g., spawn/execFile-style parameter passing rather than exec with a single command line). - If you must use a shell command string, you must quote/escape arguments so shell metacharacters are not interpreted. The referenced fix escapes the tag argument and wraps it in single quotes. [3] - Even with quoting, ensure your quoting context matches how the command is executed; security guidance emphasizes that quoting/escaping alone can be unreliable if you pass untrusted input into a shell without controlling the interpreter behavior. [4] 4) Practical “safe allowlist” approach (what to use instead of relying on GitHub/Git rules) - GitHub suggests using a safe default set of characters for branch/tag names to avoid needing escapes when working in shells: English letters (A–Z, a–z), digits (0–9), and punctuation. - _ /. [1] Answer in one line: There isn’t a GitHub-specific “shell-injection-safe” character restriction for tag names; Git allows many characters, and shell injection is prevented by handling tag strings as untrusted and avoiding/quoting shell interpretation when building commands. [1][2][3]
Citations:
- 1: https://docs.github.com/en/get-started/using-git/dealing-with-special-characters-in-branch-and-tag-names
- 2: https://git-scm.com/docs/git-check-ref-format/2.52.0
- 3: ericcornelissen/git-tag-annotation-action@9f30756
- 4: https://semgrep.dev/docs/learn/vulnerabilities/command-injection
Harden the workflow: pin actions and fix checkout-credentials handling for the tag push.
- Pin
actions/checkout@v4andactions/setup-go@v5to commit SHAs (supply-chain hardening). Checkoutcurrently keeps persisted Git credentials (nopersist-credentials: false). The workflow later runsgit pushin Update bootstrapper-latest tag, so settingpersist-credentials: falsewithout adding explicit git auth would break the job—either keep persisted credentials or update the workflow to authenticategit pushexplicitly before that step.gh release uploaduses${{ github.event.release.tag_name }}directly in therun:shell line; move it to an environment variable (e.g.,env: RELEASE_TAG: ...and use"$RELEASE_TAG") to avoid shell interpretation risks from tag names.- On
release.published, default checkout is the event’s ref/SHA, sobootstrapper-latestwill track the release-tag commit (not necessarily themainHEAD), matching the job’s actual behavior.
Suggested hardening patch (pin actions; checkout auth needs follow-up)
- - name: Checkout
- uses: actions/checkout@v4
+ - name: Checkout
+ uses: actions/checkout@<PINNED_COMMIT_SHA_FOR_V4>
- - name: Setup Go
- uses: actions/setup-go@v5
+ - name: Setup Go
+ uses: actions/setup-go@<PINNED_COMMIT_SHA_FOR_V5>
with:
go-version: "1.21"🧰 Tools
🪛 zizmor (1.25.2)
[warning] 23-24: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[error] 24-24: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[error] 27-27: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[error] 27-27: runtime artifacts potentially vulnerable to a cache poisoning attack (cache-poisoning): enables caching by default
(cache-poisoning)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/build-bootstrapper.yml around lines 23 - 29, Pin the
GitHub actions used (actions/checkout and actions/setup-go) to specific commit
SHAs instead of floating tags to harden supply chain; for the Checkout step
adjust the persist-credentials setting—either keep persist-credentials: true so
the later "Update bootstrapper-latest tag" git push works, or set
persist-credentials: false and add an explicit authentication step before that
push (e.g., gh auth login or setting GITHUB_TOKEN) to ensure git push can
authenticate; move the release tag usage in the "gh release upload" run into an
environment variable (e.g., env: RELEASE_TAG: ${{ github.event.release.tag_name
}}) and reference it as "$RELEASE_TAG" in the shell command to avoid
shell-interpretation issues; verify behavior on release.published matches
expectations (checkout will be at the release ref).
| if: github.event_name != 'workflow_dispatch' | ||
| run: | | ||
| git tag -f bootstrapper-latest | ||
| git push -f origin bootstrapper-latest |
There was a problem hiding this comment.
Limit floating-tag updates to push events only.
Current condition runs on release too. In that event, checkout may be at the release tag commit, which can move bootstrapper-latest away from intended branch HEAD.
Suggested fix
- - name: Update bootstrapper-latest tag
- if: github.event_name != 'workflow_dispatch'
+ - name: Update bootstrapper-latest tag
+ if: github.event_name == 'push' && github.ref == 'refs/heads/main'🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/build-bootstrapper.yml around lines 83 - 86, The current
workflow step that force-updates the floating tag "bootstrapper-latest" runs for
any event except workflow_dispatch; change the condition so it only runs on push
events to avoid moving the tag during release/tag events — replace the existing
if line ("if: github.event_name != 'workflow_dispatch'") with a push-only check
(e.g., "if: github.event_name == 'push'") so the block that runs git tag -f
bootstrapper-latest and git push -f origin bootstrapper-latest only executes on
push events.
| if err := cmd.Run(); err != nil { | ||
| return "", stderr.String() | ||
| } |
There was a problem hiding this comment.
Don’t swallow npm install failures when stderr is empty.
If cmd.Run() fails without stderr output, this currently returns an empty error string and can be treated as success by callers.
Suggested fix
if err := cmd.Run(); err != nil {
- return "", stderr.String()
+ if stderr.Len() > 0 {
+ return "", stderr.String()
+ }
+ return "", err.Error()
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if err := cmd.Run(); err != nil { | |
| return "", stderr.String() | |
| } | |
| if err := cmd.Run(); err != nil { | |
| if stderr.Len() > 0 { | |
| return "", stderr.String() | |
| } | |
| return "", err.Error() | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@bootstrapper/install/cli.go` around lines 23 - 25, When cmd.Run() returns an
error in the install flow, do not return an empty string if stderr.String() is
empty; instead return a meaningful message that includes the command error.
Update the error handling around cmd.Run() in cli.go so that when err != nil you
return either stderr.String() if non-empty or err.Error() (or a concatenation of
both) so callers never receive an empty error string; key symbols to change: the
cmd.Run() error check and the use of stderr.String().
| cmd := exec.Command("/bin/bash", "-c", | ||
| `"$(curl -fsSL https://raw.githubusercontent.com/Homebrew/install/HEAD/install.sh)"`, | ||
| ) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Quoted form (expected to fail):"
if bash -lc '"$(printf "%s" "echo ok")"'; then
echo "unexpected success"
else
echo "failed as expected"
fi
echo "Unquoted form (expected to succeed):"
bash -lc '$(printf "%s" "echo ok")'Repository: calycode/xano-tools
Length of output: 411
Fix Homebrew installer invocation quoting in bootstrapper/install/node_darwin.go (bash -c).
The extra double-quotes around $(curl ...) make bash -c treat the downloaded installer as a single command token instead of executing it.
Suggested fix
cmd := exec.Command("/bin/bash", "-c",
- `"$(curl -fsSL https://raw.githubusercontent.com/Homebrew/install/HEAD/install.sh)"`,
+ `$(curl -fsSL https://raw.githubusercontent.com/Homebrew/install/HEAD/install.sh)`,
)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| cmd := exec.Command("/bin/bash", "-c", | |
| `"$(curl -fsSL https://raw.githubusercontent.com/Homebrew/install/HEAD/install.sh)"`, | |
| ) | |
| cmd := exec.Command("/bin/bash", "-c", | |
| `$(curl -fsSL https://raw.githubusercontent.com/Homebrew/install/HEAD/install.sh)`, | |
| ) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@bootstrapper/install/node_darwin.go` around lines 40 - 42, The exec.Command
call that builds cmd currently wraps the $(curl ...) invocation in extra
double-quotes so bash -c treats the downloaded script as a literal token; update
the exec.Command argument passed to bash -c (the exec.Command(...) that assigns
to cmd) to remove the surrounding quotes so the output of $(curl -fsSL
https://raw.githubusercontent.com/Homebrew/install/HEAD/install.sh) is executed
by bash. Ensure the resulting string passed to exec.Command is exactly the
command substitution without the extra double quotes.
| func InstallNode() string { | ||
| return "Unsupported platform. Please install Node.js 18+ manually from https://nodejs.org" | ||
| } |
There was a problem hiding this comment.
Use localized installer message instead of hardcoded English text.
This path currently bypasses i18n and breaks localization consistency for user-facing errors.
Suggested fix
func InstallNode() string {
- return "Unsupported platform. Please install Node.js 18+ manually from https://nodejs.org"
+ return msg.NodeInstallManual
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@bootstrapper/install/node_unsupported.go` around lines 6 - 8, Replace the
hardcoded English string returned by InstallNode with a localized message
lookup; change the function InstallNode to call the project's i18n/localization
helper (e.g., the translation function used elsewhere like T, i18n.T, translate,
or similar) to return the localized installer text and URL, ensuring you use the
same message key used for similar installer/error messages so translations are
picked up consistently.
| ``` | ||
|
|
||
| ### Environment Variables (Unix) | ||
|
|
||
| `install.sh` / `install-unix.sh` supports: | ||
|
|
||
| - `CALYCODE_VERSION=1.2.3` - install a specific version | ||
| - `CALYCODE_SKIP_NATIVE_HOST=1` - skip `caly-xano opencode init` | ||
|
|
||
| Example: | ||
|
|
||
| ```bash | ||
| curl -fsSL https://links.calycode.com/install-cli-unix | CALYCODE_VERSION=1.2.3 CALYCODE_SKIP_NATIVE_HOST=1 bash -s -- | ||
| win32 → calycode-installer-windows-x64.exe | ||
| darwin → calycode-installer-darwin-arm64 (Apple Silicon) or darwin-x64 (Intel) | ||
| linux → Fall back to shell script: curl ... | bash | ||
| ``` |
There was a problem hiding this comment.
Add language specifier to code block.
The code block showing user agent mapping should specify a language for proper syntax highlighting. Consider using text or plaintext.
📝 Proposed fix
-```
+```text
win32 → calycode-installer-windows-x64.exe
darwin → calycode-installer-darwin-arm64 (Apple Silicon) or darwin-x64 (Intel)
linux → Fall back to shell script: curl ... | bash🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 59-59: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/cli/scripts/README.md` around lines 59 - 63, The fenced code block
containing the user agent mapping should include a language specifier for proper
syntax highlighting; update the triple-backtick fence that wraps the three-line
mapping (the block starting with "win32 → calycode-installer-windows-x64.exe")
to use a plain-text specifier such as ```text so the lines display with correct
highlighting.
| ``` | ||
| bootstrapper/ # Go bootstrapper source (primary) | ||
| ├── main.go # Entry point — orchestrate install flow | ||
| ├── install/ | ||
| │ ├── node.go # Node.js version detection (cross-platform) | ||
| │ ├── node_windows.go # Windows: winget install | ||
| │ ├── node_darwin.go # macOS: brew install | ||
| │ ├── node_unsupported.go # Stub for other platforms | ||
| │ └── cli.go # npm install + oc init | ||
| ├── ui/ | ||
| │ ├── dialog_windows.go # Windows: MessageBox native dialogs | ||
| │ ├── dialog_darwin.go # macOS: osascript dialogs | ||
| │ └── dialog_unsupported.go# Stub for other platforms | ||
| └── Makefile # Cross-compile targets | ||
|
|
||
| scripts/installer/ # Shell fallbacks (kept for CI / headless) | ||
| ├── install.sh # Unix installer | ||
| ├── install.ps1 # Windows PowerShell installer | ||
| └── install.bat # Windows CMD wrapper | ||
|
|
||
| scripts/dev/ # Development-only (for CLI contributors) | ||
| ├── install-unix.sh | ||
| └── install-win.bat | ||
| ``` |
There was a problem hiding this comment.
Add language specifier to directory structure code block.
The directory structure code block should specify a language (e.g., text or plaintext) for consistent rendering and to satisfy markdown linting rules.
📝 Proposed fix
-```
+```text
bootstrapper/ # Go bootstrapper source (primary)
├── main.go # Entry point — orchestrate install flow🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 94-94: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/cli/scripts/README.md` around lines 94 - 117, Update the fenced code
block containing the directory tree (the block that starts with "bootstrapper/"
and includes "scripts/installer/" and "scripts/dev/") in
packages/cli/scripts/README.md by adding a language specifier to the opening
fence (e.g., change ``` to ```text or ```plaintext) so the block is explicitly
marked as plain text for consistent rendering and markdown linting; keep the
existing closing fence unchanged.
| function ensureManagedOpencodeInstalled(version: string): string { | ||
| const managedBinPath = getManagedOpencodeBinPath(version); | ||
| if (fileExists(managedBinPath)) { | ||
| pruneManagedOpencodeVersions(5); | ||
| return managedBinPath; |
There was a problem hiding this comment.
latest gets pinned after the first managed install.
Both reuse paths short-circuit as soon as versions/latest/... exists, so the first opencode-ai@latest you install is effectively cached forever. That defeats the new default channel and prevents later runs from picking up newer OpenCode releases unless the user manually deletes the managed install. Treat latest as non-cacheable, or resolve it to a concrete version before reusing a managed directory.
Also applies to: 309-323
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/cli/src/commands/opencode/implementation.ts` around lines 244 - 248,
The code in ensureManagedOpencodeInstalled (and the similar block around lines
309-323) reuses any existing managed directory for the given version string,
which pins the literal "latest"; change the logic so that when version ===
'latest' you do not short-circuit to an existing
getManagedOpencodeBinPath(version) directory or instead resolve 'latest' to a
concrete release before reuse. Concretely: update ensureManagedOpencodeInstalled
(and the duplicate block) to either (A) call a function to resolve the actual
latest release version and use that concrete version for
getManagedOpencodeBinPath/pruneManagedOpencodeVersions, or (B) skip the cache
check when version === 'latest' so the install proceeds and overwrites/updates
the managed directory. Ensure references to getManagedOpencodeBinPath and
pruneManagedOpencodeVersions are adjusted accordingly.
| function filterAndValidateOrigins(rawOrigins: unknown, knownExtensionIds: string[]): string[] { | ||
| if (!Array.isArray(rawOrigins)) { | ||
| return []; | ||
| } | ||
|
|
||
| const valid: string[] = []; | ||
| for (const origin of rawOrigins) { | ||
| if (typeof origin !== 'string') continue; | ||
| if (!isValidCorsOrigin(origin, knownExtensionIds)) continue; | ||
| const trimmed = origin.trim(); | ||
| if (!valid.includes(trimmed)) { | ||
| valid.push(trimmed); | ||
| } | ||
| if (valid.length >= MAX_CORS_ORIGINS) break; | ||
| } | ||
|
|
||
| return valid; | ||
| } |
There was a problem hiding this comment.
The new CORS guard only covers msg.origins.
This validator never runs on CALY_EXTRA_CORS_ORIGINS, so *, wildcard patterns, or an oversized env-supplied list still flow through getAllowedCorsOrigins() into --cors unchanged. Apply the same validation and cap to the final merged origin list before spawning OpenCode. As per coding guidelines, "Validate all user inputs and API responses."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/cli/src/commands/opencode/implementation.ts` around lines 730 - 747,
The merged origin list returned by getAllowedCorsOrigins currently bypasses the
same validation applied in filterAndValidateOrigins (so CALY_EXTRA_CORS_ORIGINS
and env-sourced values can include "*" or exceed MAX_CORS_ORIGINS); ensure you
run filterAndValidateOrigins (or equivalent validation logic) against the final
combined origins array produced in getAllowedCorsOrigins (and before passing
into the OpenCode spawn/--cors flag), trimming duplicates and enforcing
MAX_CORS_ORIGINS and isValidCorsOrigin checks for entries from
CALY_EXTRA_CORS_ORIGINS, msg.origins, and any other sources so the final list is
validated and capped.
| const NATIVE_HOST_PORT_RANGE_START = 4096; | ||
| const NATIVE_HOST_PORT_RANGE_SIZE = 32; // 4096–4127 | ||
| const NATIVE_HOST_PORT_RANGE_END = NATIVE_HOST_PORT_RANGE_START + NATIVE_HOST_PORT_RANGE_SIZE - 1; | ||
| ``` |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if there are known service assignments in the 4096-4127 range
rg -i "409[6-9]|41[0-2][0-7]" --type md -C 2Repository: calycode/xano-tools
Length of output: 6723
🌐 Web query:
IANA port ranges registered dynamic ephemeral 4096-4127
💡 Result:
The IANA port range 4096-4127 is part of the Registered Ports range (1024-49151) and is not designated as dynamic or ephemeral [1][2][3]. According to the Internet Assigned Numbers Authority (IANA), port numbers are categorized into three distinct ranges [4][1]: 1. System Ports (0-1023): Reserved for system services and common applications [1][2][3]. 2. Registered Ports (1024-49151): Assigned by IANA for specific applications and services [1][2][3]. 3. Dynamic and/or Private Ports (49152-65535): Reserved for temporary, ephemeral use by applications and are not assigned by IANA [1][2][3]. Because ports 4096-4127 fall within the 1024-49151 range, they are classified as Registered Ports. They are not ephemeral ports, which are defined by IANA as the 49152-65535 range [1][2]. While individual ports within the Registered range may be unassigned or reserved for specific protocols in the IANA registry, this does not make the entire block dynamic or ephemeral [4][5].
Citations:
- 1: https://jumpcloud.com/it-index/what-is-an-ephemeral-port
- 2: https://en.wikipedia.org/wiki/Ephemeral_port
- 3: https://whatismyipaddress.com/port-list
- 4: https://www.iana.org/assignments/service-names-port-numbers/service-names-port-numbers.xhtml?page=1
- 5: https://www.iana.org/assignments/service-names-port-numbers/service-names-port-numbers.xhtml?page=2&search=Reserved
🌐 Web query:
Chrome native messaging host port range convention
💡 Result:
There is no “host port range” convention for Chrome Native Messaging. Chrome Native Messaging does not use TCP/UDP network ports at all. Instead, when an extension connects to a native messaging host (e.g., via runtime.connectNative or runtime.sendNativeMessage), Chrome launches the native host process and communicates with it over standard input/output streams: messages are JSON (UTF-8) framed with a 32-bit length prefix in native byte order. [1] So, any “port range” you might be thinking of is unrelated to the Native Messaging protocol; Native Messaging hosts are configured via a native host manifest (including the stdio transport), not via listening on a network port. [1]
Citations:
Fix framing: 4096–4127 is for the spawned local server, not Chrome Native Messaging ports—and update the “dynamic” rationale.
- Chrome Native Messaging doesn’t use TCP/UDP ports at all (it communicates with native hosts over stdio), so there’s no “native host port range convention” to align with.
- 4096–4127 (32 slots) falls under IANA Registered Ports (1024–49151), not the ephemeral/dynamic range (49152–65535)—so the comment’s “registered/dynamic” wording should be corrected.
- The repo already documents/implements validation for boundaries (4096 and 4127) and related stop-message behavior, but the docs should explicitly state why 32 slots and this particular block starting at 4096 were chosen (e.g., for managed PID/session tracking) and whether users can override if their local ports are already occupied.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@plans/native-host-security-hardening.md` around lines 234 - 237, The comment
and rationale around NATIVE_HOST_PORT_RANGE_START, NATIVE_HOST_PORT_RANGE_SIZE
and NATIVE_HOST_PORT_RANGE_END are incorrect: Chrome Native Messaging does not
use TCP/UDP ports and 4096–4127 is inside the IANA Registered Ports range
(1024–49151), not the ephemeral/dynamic range. Update the inline comment and any
documentation to (1) state that 4096–4127 is the dedicated local server port
block used by our spawned local server (not Chrome Native Messaging), (2)
correct the “registered/dynamic” wording to “IANA Registered Ports
(1024–49151)”, (3) explain the choice of 32 slots starting at 4096 (e.g., for
managed PID/session tracking and predictable allocation) and note behavior when
a port is occupied (including existing boundary validation and stop-message
handling), and (4) document whether and how users can override the start/size
constants (NATIVE_HOST_PORT_RANGE_START, NATIVE_HOST_PORT_RANGE_SIZE) or
configure alternate ports.
1 participant
What changed\n- add bootstrapper localization catalogs and wire installer/ui/install messages through i18n\n- add new locales: es, de, fr, hu, sr\n- switch macOS bootstrapper release artifacts to zip files and update docs/URLs\n- default OpenCode channel to latest in caly-xano opencode flows\n- keep version pinning when browser/native-host sends an explicit ocVersion\n\n## Validation\n- built bootstrapper targets (windows, darwin x64, darwin arm64) and produced macOS zip artifacts\n- built CLI package with pnpm --filter @calycode/cli build\n\n## Notes\n- branch includes prior hardening/bootstrapper commits already on dev\n
Summary by CodeRabbit
New Features
Bug Fixes
Documentation