Restore Workbench Bridge resource proof - #470
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
📜 Recent review details
|
| Layer / File(s) | Summary |
|---|---|
Packaging config and build wiring .gitignore, packages/desktop/electron-builder.yml, scripts/build-with-builder.js, .github/workflows/_build-reusable.yml |
Adds resources/Bridge packaging inputs, gates Bridge preparation to macOS builds, and requires a real Bridge in the macOS builder workflow. |
prepareEvaosDesktopBridgeResource.js: source resolution and outputs scripts/prepareEvaosDesktopBridgeResource.js |
Defines Bridge source/ref policy helpers, source discovery, git checkout resolution, token redaction, binary selection, wrapper support, placeholder handling, manifest writing, and CLI exports. |
afterPack.js: Bridge verification scripts/afterPack.js |
Adds Bridge resource validation helpers, labeled JSON parsing, and Bridge verification in the non-thin-shell afterPack flow. |
Workflow Bridge checks and manifest proof .github/workflows/pr-checks.yml, .github/workflows/workbench-functional-smoke.yml |
Validates bundled Bridge resources in PR checks and wires functional-smoke to pass Bridge inputs, enforce manifest matching, and report Bridge metadata. |
Tests: prepareEvaosDesktopBridgeResource tests/unit/bootstrap/prepareEvaosDesktopBridgeResource.test.ts |
Covers real source prep, placeholder fallback, ref validation, cache behavior, and token redaction. |
Tests: afterPack and build-with-builder Bridge integration tests/unit/bootstrap/afterPackPackagingProfile.test.ts, tests/unit/bootstrap/buildWithBuilder.test.ts |
Expands macOS packaging-profile coverage and asserts Bridge preparation runs or skips under the expected build paths. |
Estimated code review effort
🎯 4 (Complex) | ⏱️ ~60 minutes
Possibly related issues
#461: Directly tracks the Bridge resource proof and packaging contract restored by this PR.
Possibly related PRs
- 100yenadmin/evaOS-GUI#375: Modifies the same
workbench-functional-smoke.ymlBridge verification path and manifest/ref checks. - 100yenadmin/evaOS-GUI#377: Overlaps on Bridge source-token wiring in the functional-smoke build step.
- 100yenadmin/evaOS-GUI#455: Touches the same thin smoke Bridge packaging proof flow in
pr-checks.yml.
Suggested labels
kind:integration, area:native-companion
🚥 Pre-merge checks | ✅ 4 | ❌ 1
❌ Failed checks (1 warning)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage | Docstring coverage is 10.00% which is insufficient. The required threshold is 80.00%. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
✅ Passed checks (4 passed)
| Check name | Status | Explanation |
|---|---|---|
| Title check | ✅ Passed | It succinctly captures the main change: restoring Bridge resource proof for Workbench smoke. |
| Description check | ✅ Passed | It covers the problem, linked issue, implementation, release boundary, and validation, with only non-critical template sections omitted. |
| Linked Issues check | ✅ Passed | The changes restore deterministic Bridge preparation, ship the resource bundle, and tighten smoke checks exactly as #461 requires. |
| Out of Scope Changes check | ✅ Passed | The extra workflow, packaging, and test changes all support Bridge resource proof and validation; no unrelated feature work stands out. |
✨ Finishing Touches
📝 Generate docstrings
- Create stacked PR
- Commit on current branch
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Commit unit tests in branch
codex/workbench-bridge-resource-proof-461
Comment @coderabbitai help to get the list of available commands.
coderabbitai
Bot
added
bug
9cbec97 to
c6255e9
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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/pr-checks.yml:
- Line 728: The smoke job currently only reads the Bridge manifest in the
BRIDGE_SUMMARY assignment and never fails when the manifest is a real bundle.
Update the PR checks workflow logic around the manifest parsing step to
explicitly assert m.placeholder === true before proceeding, and make the job
exit non-zero if the Bridge manifest is not the diagnostic placeholder so the
placeholder contract is enforced.
In @.github/workflows/workbench-functional-smoke.yml:
- Around line 168-173: The bridge ref verification in the workbench smoke
workflow is comparing the requested bridge_ref directly against the resolved
manifest value, which breaks for valid non-SHA refs like branches, tags, or
short SHAs. Update the check around the BRIDGE_VERSION and
WORKBENCH_SMOKE_BRIDGE_REF comparison to either require bridge_ref to be a full
immutable SHA before proceeding or compare against a manifest field that
preserves the original requested ref. Keep the existing bridge manifest
validation flow intact, but make the match logic consistent with how the bridge
is actually resolved and recorded.
In `@scripts/prepareEvaosDesktopBridgeResource.js`:
- Around line 445-454: Add JSDoc to each public helper exported from
prepareEvaosDesktopBridgeResource.js so callers/tests can understand the
env-contracts and side effects without reading the implementation. Document the
callable symbols bridgeWrapperScript, defaultBridgeSourceRef, isMachOExecutable,
isMutableBridgeSourceRef, resolveBridgeSourceDir, sanitizeCommandText, and
shouldRejectPlaceholder directly at their function declarations, keeping the
descriptions aligned with how they are used and what inputs/outputs or mutations
they involve.
- Around line 58-73: The source selection in
sourceCandidates()/resolveBridgeSourceDir() still allows implicit local
checkouts to win before the pinned checkout path is used, which can bypass the
approved ref. Update the candidate resolution flow so release-like and
functional-smoke paths only use a checked-out source when it comes from
prepareBridgeSourceCheckout()/the pinned ref, and do not fall back to
../evaos-desktop-bridge or the hard-coded /Volumes mirrors in that mode. Keep
the existing env overrides (EVAOS_DESKTOP_BRIDGE_SOURCE_DIR,
EVAOS_DESKTOP_BRIDGE_DISABLE_DEFAULT_CANDIDATES,
EVAOS_DESKTOP_BRIDGE_SOURCE_REF) working, but make the default candidate list
impossible to use when a pinned-proof build is required.
In `@tests/unit/bootstrap/afterPackPackagingProfile.test.ts`:
- Around line 171-180: This positive afterPack test is still sensitive to
external release/real-bridge environment flags, so it can fail
nondeterministically when runner env vars are set. In the test around
createDarwinContext and afterPack, explicitly isolate or override
EVAOS_DESKTOP_BRIDGE_REQUIRE_REAL, EVAOS_BETA_PUBLIC_RELEASE, and
EVAOS_BETA_REQUIRE_SIGNING so the shell-script Bridge helper path is always
exercised. Restore the original env state after the assertion to keep the test
self-contained.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 0a055848-b808-48b1-aba4-692355515057
📒 Files selected for processing (10)
.github/workflows/pr-checks.yml.github/workflows/workbench-functional-smoke.yml.gitignorepackages/desktop/electron-builder.ymlscripts/afterPack.jsscripts/build-with-builder.jsscripts/prepareEvaosDesktopBridgeResource.jstests/unit/bootstrap/afterPackPackagingProfile.test.tstests/unit/bootstrap/buildWithBuilder.test.tstests/unit/bootstrap/prepareEvaosDesktopBridgeResource.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: Thin App Smoke (macos-arm64)
⚠️ CI failures not shown inline (2)
GitHub Actions: Workbench Functional Smoke / 0_macOS arm64 unpacked app.txt: Workbench Functional Smoke
Conclusion: failure
##[group]Run set -euo pipefail
�[36;1mset -euo pipefail�[0m
�[36;1mnode scripts/evaosBetaReleaseGate.js audit-config�[0m
�[36;1mnode scripts/evaosUpstreamGuardrailAudit.js�[0m
shell: /bin/bash --noprofile --norc -e -o pipefail {0}
env:
BUN_INSTALL_REGISTRY: https://registry.npmjs.org/
CI: true
CSC_IDENTITY_AUTO_DISCOVERY: false
EVAOS_BETA_PUBLIC_RELEASE: false
EVAOS_BETA_REQUIRE_SIGNING: false
EVAOS_FINALIZE_MAC_DMG: false
EVAOS_DMG_CODESIGN: false
ELECTRON_BUILDER_COMPRESSION_LEVEL: 1
WORKBENCH_SMOKE_BRIDGE_REF: 207f6528461ecae51c39efd2654733c1b07d39a4
pythonLocation: /Users/runner/hostedtoolcache/Python/3.12.10/arm64
PKG_CONFIG_PATH: /Users/runner/hostedtoolcache/Python/3.12.10/arm64/lib/pkgconfig
Python_ROOT_DIR: /Users/runner/hostedtoolcache/Python/3.12.10/arm64
Python2_ROOT_DIR: /Users/runner/hostedtoolcache/Python/3.12.10/arm64
Python3_ROOT_DIR: /Users/runner/hostedtoolcache/Python/3.12.10/arm64
##[endgroup]
ENOENT: no such file or directory, open '/Users/runner/work/evaOS-GUI/evaOS-GUI/packages/desktop/src/process/evaosBetaSafety.ts'
##[error]Process completed with exit code 1.
GitHub Actions: Workbench Functional Smoke / macOS arm64 unpacked app: Workbench Functional Smoke
Conclusion: failure
##[group]Run set -euo pipefail
�[36;1mset -euo pipefail�[0m
�[36;1mnode scripts/evaosBetaReleaseGate.js audit-config�[0m
�[36;1mnode scripts/evaosUpstreamGuardrailAudit.js�[0m
shell: /bin/bash --noprofile --norc -e -o pipefail {0}
env:
BUN_INSTALL_REGISTRY: https://registry.npmjs.org/
CI: true
CSC_IDENTITY_AUTO_DISCOVERY: false
EVAOS_BETA_PUBLIC_RELEASE: false
EVAOS_BETA_REQUIRE_SIGNING: false
EVAOS_FINALIZE_MAC_DMG: false
EVAOS_DMG_CODESIGN: false
ELECTRON_BUILDER_COMPRESSION_LEVEL: 1
WORKBENCH_SMOKE_BRIDGE_REF: 207f6528461ecae51c39efd2654733c1b07d39a4
pythonLocation: /Users/runner/hostedtoolcache/Python/3.12.10/arm64
PKG_CONFIG_PATH: /Users/runner/hostedtoolcache/Python/3.12.10/arm64/lib/pkgconfig
Python_ROOT_DIR: /Users/runner/hostedtoolcache/Python/3.12.10/arm64
Python2_ROOT_DIR: /Users/runner/hostedtoolcache/Python/3.12.10/arm64
Python3_ROOT_DIR: /Users/runner/hostedtoolcache/Python/3.12.10/arm64
##[endgroup]
ENOENT: no such file or directory, open '/Users/runner/work/evaOS-GUI/evaOS-GUI/packages/desktop/src/process/evaosBetaSafety.ts'
##[error]Process completed with exit code 1.
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Unused function parameters must be prefixed with underscore (_)
Use single-element arrays inline if they fit on one line; require trailing commas in multi-line arrays/objects; use single quotes for strings
Use English for code comments; provide JSDoc for public functions
Files:
scripts/build-with-builder.jstests/unit/bootstrap/prepareEvaosDesktopBridgeResource.test.tstests/unit/bootstrap/buildWithBuilder.test.tstests/unit/bootstrap/afterPackPackagingProfile.test.tsscripts/prepareEvaosDesktopBridgeResource.jsscripts/afterPack.js
**/*[!.module].ts
📄 CodeRabbit inference engine (AGENTS.md)
Utility files must use camelCase naming (e.g., formatDate.ts)
Files:
tests/unit/bootstrap/prepareEvaosDesktopBridgeResource.test.tstests/unit/bootstrap/buildWithBuilder.test.tstests/unit/bootstrap/afterPackPackagingProfile.test.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use@icon-park/reactfor icons
TypeScript strict mode must be enabled; no 'any' types, no implicit returns
Use path aliases:@/,@process/,@renderer/* for imports
Prefer 'type' over 'interface' for type definitions (per Oxlint config)
Files:
tests/unit/bootstrap/prepareEvaosDesktopBridgeResource.test.tstests/unit/bootstrap/buildWithBuilder.test.tstests/unit/bootstrap/afterPackPackagingProfile.test.ts
**/*.{test,spec}.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Vitest 4 for testing with a coverage target of at least 80%
Files:
tests/unit/bootstrap/prepareEvaosDesktopBridgeResource.test.tstests/unit/bootstrap/buildWithBuilder.test.tstests/unit/bootstrap/afterPackPackagingProfile.test.ts
🪛 ast-grep (0.44.0)
tests/unit/bootstrap/prepareEvaosDesktopBridgeResource.test.ts
[warning] 6-6: Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import { spawnSync } from 'node:child_process';
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process-typescript)
scripts/prepareEvaosDesktopBridgeResource.js
[error] 377-377: An archive entry path (e.g. entry.path / entry.fileName / header.name) is joined to an output directory without validating that the resolved path stays inside that directory. A malicious archive can use "../" sequences to escape the extraction directory and overwrite arbitrary files (Zip Slip). Resolve the path and verify it starts with the normalized output directory, or strip traversal with path.basename, before writing the entry.
Context: path.join(dir, entry.name)
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(zip-slip-archive-extraction-javascript)
[warning] 192-192: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.readFileSync(filePath, { encoding: null, flag: 'r' })
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
[warning] 208-227: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.writeFileSync(
helperPath,
`#!/bin/sh
set -eu
HELPER_DIR="$(CDPATH= cd -- "$(dirname -- "$0")" && pwd)"
if [ -x "$HELPER_DIR/peekaboo" ]; then
exec "$HELPER_DIR/peekaboo" "$@"
fi
for candidate in /opt/homebrew/bin/peekaboo /usr/local/bin/peekaboo; do
if [ -x "$candidate" ]; then
exec "$candidate" "$@"
fi
done
echo "evaos-connector-helper: bundled peekaboo was not found. Rebuild evaOS Workbench with Bridge/bin/peekaboo or install the Workbench connector package." >&2
exit 127
`
)
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
[warning] 234-248: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.writeFileSync(
peekabooPath,
`#!/bin/sh
set -eu
for candidate in /opt/homebrew/bin/peekaboo /usr/local/bin/peekaboo; do
if [ -x "$candidate" ]; then
exec "$candidate" "$@"
fi
done
echo "peekaboo: bundled helper binary was not found. Rebuild evaOS Workbench with Bridge/bin/peekaboo or install the Workbench connector package." >&2
exit 127
`
)
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
[warning] 308-308: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.writeFileSync(wrapperPath, bridgeWrapperScript())
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
[warning] 328-342: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.writeFileSync(
wrapperPath,
`#!/bin/sh
set -eu
if [ "${1:-}" = "--version" ] || [ "${1:-}" = "version" ]; then
echo "evaos-desktop-bridge diagnostic-placeholder"
exit 0
fi
echo "evaos-desktop-bridge diagnostic placeholder: ${escapeForShellDoubleQuotes(reason)}" >&2
echo "This PR/build artifact is not valid for Mac pairing release proof. Configure EVAOS_DESKTOP_BRIDGE_SOURCE_DIR or EVAOS_DESKTOP_BRIDGE_SOURCE_TOKEN for a real release build." >&2
exit 78
`
)
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
[warning] 347-347: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.writeFileSync(path.join(bridgeResourceDir, 'manifest.json'), ${JSON.stringify(manifest, null, 2)}\n)
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
[warning] 4-4: Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: require('child_process')
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process)
scripts/afterPack.js
[warning] 132-132: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.readFileSync(filePath, 'utf8')
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
[warning] 162-162: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.readFileSync(filePath, { encoding: null, flag: 'r' })
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
🔇 Additional comments (6)
.github/workflows/pr-checks.yml (1)
655-657: LGTM!Also applies to: 683-702, 738-741
scripts/afterPack.js (1)
14-15: LGTM!Also applies to: 40-52, 131-174, 187-220, 338-338, 412-412
.github/workflows/workbench-functional-smoke.yml (1)
19-23: LGTM!Also applies to: 38-38, 121-135, 191-191
tests/unit/bootstrap/afterPackPackagingProfile.test.ts (1)
7-118: LGTM!Also applies to: 187-236
tests/unit/bootstrap/buildWithBuilder.test.ts (1)
116-116: LGTM!Also applies to: 131-133, 178-181
packages/desktop/electron-builder.yml (1)
117-118: 🩺 Stability & AvailabilityVerify thin-shell packaging still has a Bridge source directory.
extraResourcesis unconditional here, but the PR context says the Bridge prep step is skipped forthin-shelland.gitignorenow keepsresources/Bridgeout of the repo. Ifelectron-builderstill errors on a missingfrompath, thin-shell packaging will fail beforeafterPackcan skip Bridge verification. Please confirm this entry is profile-gated elsewhere or that thin-shell builds create a stubresources/Bridgefirst.
coderabbitai
Bot
added
area:release
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 `@scripts/build-with-builder.js`:
- Around line 581-583: The Bridge payload can be left stale in the workspace
when a later thin-shell build reuses `resources/Bridge` from a previous
non-thin-shell build. Update the `scripts/build-with-builder.js` packaging flow
around the `execSync('node scripts/prepareEvaosDesktopBridgeResource.js', ...)`
step to either clear `resources/Bridge` for the `thin-shell` path or skip Bridge
preparation for that profile entirely. Make the behavior conditional on the
build profile so `packages/desktop/electron-builder.yml` does not accidentally
package an old Bridge directory, since `scripts/afterPack.js` does not verify
Bridge for thin-shell.
In `@scripts/prepareEvaosDesktopBridgeResource.js`:
- Around line 96-108: After the clone/fetch flow in
prepareEvaosDesktopBridgeResource, the tokenized remote URL is left persisted in
the local Git config via repoWithToken() and remote.origin.url. Update the clone
success path in the try block and the fallback path using runGit to reset origin
back to the clean repo URL once the checkout completes, so the cached bridge
repo never keeps the private token. Use the existing helpers repoWithToken(),
sanitizeRepoForLog(), and runGit() to locate and patch the clone logic.
- Around line 51-55: The isMutableBridgeSourceRef() check is too narrow because
it only rejects exact main/master/head values, allowing mutable refs like
refs/heads/main and origin/main to pass. Update the logic in
isMutableBridgeSourceRef() to require a pinned commit object ID instead of a
branch-like ref, or resolve the provided ref to its target object ID and
validate that it is immutable before allowing it through.
In `@tests/unit/bootstrap/prepareEvaosDesktopBridgeResource.test.ts`:
- Around line 50-54: The test setup for runPrepare is overriding PATH with a
hardcoded POSIX-only value, which can hide inherited tools on Windows or custom
environments. Update the env passed into runPrepare so it preserves the existing
process.env.PATH while still allowing test-specific overrides, and keep the
reference to runPrepare and the env block in
prepareEvaosDesktopBridgeResource.test to locate the change.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: dadd3585-b471-442b-926e-75fc22774b94
📒 Files selected for processing (10)
.github/workflows/pr-checks.yml.github/workflows/workbench-functional-smoke.yml.gitignorepackages/desktop/electron-builder.ymlscripts/afterPack.jsscripts/build-with-builder.jsscripts/prepareEvaosDesktopBridgeResource.jstests/unit/bootstrap/afterPackPackagingProfile.test.tstests/unit/bootstrap/buildWithBuilder.test.tstests/unit/bootstrap/prepareEvaosDesktopBridgeResource.test.ts
📜 Review details
⚠️ CI failures not shown inline (9)
GitHub Actions: Workbench Functional Smoke / 0_macOS arm64 unpacked app.txt: Workbench Functional Smoke
Conclusion: failure
##[group]Run set -euo pipefail
�[36;1mset -euo pipefail�[0m
�[36;1mif [ -z "${BRIDGE_SOURCE_***REDACTED*** ]; then�[0m
�[36;1m echo "::error::A private bridge source token is required to fetch electricsheephq/evaos-desktop-bridge for functional smoke."�[0m
GitHub Actions: Workbench Functional Smoke / macOS arm64 unpacked app: Workbench Functional Smoke
Conclusion: failure
##[group]Run set -euo pipefail
�[36;1mset -euo pipefail�[0m
�[36;1mif [ -z "${BRIDGE_SOURCE_***REDACTED*** ]; then�[0m
�[36;1m echo "::error::A private bridge source token is required to fetch electricsheephq/evaos-desktop-bridge for functional smoke."�[0m
GitHub Actions: Workbench Functional Smoke / macOS arm64 unpacked app: Workbench Functional Smoke
Conclusion: failure
##[group]Run set -euo pipefail
�[36;1mset -euo pipefail�[0m
�[36;1mAPP_PATH="$(find out -type d -name '*.app' -print -quit)"�[0m
�[36;1mif [ -z "$APP_PATH" ]; then�[0m
�[36;1m echo "::error::No .app produced under out/"�[0m
GitHub Actions: PR Checks / Thin App Smoke (macos-arm64): Restore Workbench Bridge resource proof
Conclusion: failure
##[group]Run set -euo pipefail
�[36;1mset -euo pipefail�[0m
�[36;1mecho "=========================================="�[0m
�[36;1mecho "VERIFY UNPACKED APP RESOURCE SHAPE"�[0m
�[36;1mecho "=========================================="�[0m
�[36;1mAPP_PATH="$(find out -type d -name '*.app' -print -quit)"�[0m
�[36;1mif [ -z "$APP_PATH" ]; then�[0m
�[36;1m echo "::error::No .app produced under out/"�[0m
GitHub Actions: PR Checks / Unit Tests (macos-14): Restore Workbench Bridge resource proof
Conclusion: failure
9:26
at ConfigServiceImpl.initialize �[90m(/Users/runner/work/evaOS-GUI/evaOS-GUI/�[39mpackages/desktop/src/common/config/configService.ts:67:5�[90m)�[39m
at ConfigServiceImpl.whenReady �[90m(/Users/runner/work/evaOS-GUI/evaOS-GUI/�[39mpackages/desktop/src/common/config/configService.ts:76:17�[90m)�[39m {
code: �[32m'ERR_INVALID_URL'�[39m,
input: �[32m'/api/settings/client'�[39m
}
}
�[90mstderr�[2m | tests/unit/feedback/MessageToolGroupFeedback.dom.test.tsx
�[22m�[39mFailed to load initial color scheme: TypeError: Failed to parse URL from /api/settings/client
�[90m at node:internal/deps/undici/undici:14976:13�[39m
�[90m at processTicksAndRejections (node:internal/process/task_queues:103:5)�[39m
at fetchJson �[90m(/Users/runner/work/evaOS-GUI/evaOS-GUI/�[39mpackages/desktop/src/common/config/configService.ts:27:20�[90m)�[39m
at �[90m/Users/runner/work/evaOS-GUI/evaOS-GUI/�[39mpackages/desktop/src/common/config/configService.ts:59:20 {
[cause]: TypeError: Invalid URL
�[90m at new URL (node:internal/url:818:25)�[39m
at new URL �[90m(file:///Users/runner/work/evaOS-GUI/evaOS-GUI/�[39mnode_modules/�[4m.bun�[24m/vitest@4.1.0+634674f7a4778bc9/node_modules/�[4mvitest�[24m/dist/chunks/index.EY6TCHpo.js:557:2�[90m)�[39m
�[90m at new Request (node:internal/deps/undici/undici:9804:25)�[39m
�[90m at fetch (node:internal/deps/undici/undici:10533:25)�[39m
�[90m at fetch (node:internal/deps/undici/undici:14974:10)�[39m
�[90m at fetch (node:internal/bootstrap/web/exposed-window-or-worker:75:12)�[39m
at fetchJson �[90m(/Users/runner/work/evaOS-GUI/evaOS-GUI/�[39mpackages/desktop/src/common/config/configService.ts:27:26�[90m)�[39m
at �[90m/Users/runner/work/evaOS-GUI/evaOS-GUI/�[39mpackages/desktop/src/common/config/configService.ts:59:26
at ConfigServiceImpl.initialize �[90m(/Users/runner/work/evaOS-GUI/evaOS-GUI/�[39mpackages/desktop/src/common/config/configService.ts:67:5...
GitHub Actions: PR Checks / 5_Unit Tests (macos-14).txt: Restore Workbench Bridge resource proof
Conclusion: failure
2m4 tests�[22m�[2m)�[22m�[32m 172�[2mms�[22m�[39m
�[32m✓�[39m �[30m�[45m dom �[49m�[39m tests/unit/settings/SystemSettings.dom.test.tsx �[2m(�[22m�[2m5 tests�[22m�[2m)�[22m�[32m 35�[2mms�[22m�[39m
�[32m✓�[39m �[30m�[45m dom �[49m�[39m tests/unit/feedback/FeedbackButton.dom.test.tsx �[2m(�[22m�[2m5 tests�[22m�[2m)�[22m�[32m 238�[2mms�[22m�[39m
�[32m✓�[39m �[30m�[45m dom �[49m�[39m tests/unit/feedback/McpServerHeaderFeedback.dom.test.tsx �[2m(�[22m�[2m4 tests�[22m�[2m)�[22m�[32m 155�[2mms�[22m�[39m
�[32m✓�[39m �[30m�[45m dom �[49m�[39m tests/unit/previews/MarkdownViewer.dom.test.tsx �[2m(�[22m�[2m3 tests�[22m�[2m)�[22m�[32m 58�[2mms�[22m�[39m
�[90mstdout�[2m | tests/unit/feedback/MessageAgentStatusFeedback.dom.test.tsx
�[22m�[39mbuild.buildStorage global
build.buildStorage system
�[32m✓�[39m �[30m�[45m dom �[49m�[39m tests/unit/renderer/messageToolGroupSummary.dom.test.tsx �[2m(�[22m�[2m1 test�[22m�[2m)�[22m�[32m 41�[2mms�[22m�[39m
�[90mstdout�[2m | tests/unit/feedback/MessageAgentStatusFeedback.dom.test.tsx�[2m > �[22m�[2mMessageAgentStatus — FeedbackButton wiring�[2m > �[22m�[2mdoes not render FeedbackButton on successful statuses
�[22m�[39m[httpBridge] GET /api/agents (no body)
[httpBridge] GET /api/assistants (no body)
�[90mstderr�[2m | tests/unit/feedback/MessageAgentStatusFeedback.dom.test.tsx�[2m > �[22m�[2mMessageAgentStatus — FeedbackButton wiring�[2m > �[22m�[2mdoes not render FeedbackButton on successful statuses
�[22m�[39mFailed to load assistants for conversation selector: TypeError: Failed to parse URL from /api/assistants
�[90m at node:internal/deps/undici/undici:14976:13�[39m
�[90m at processTicksAndRejections (node:internal/process/task_queues:103:5)�[39m
at httpRequest �[90m(/Users/runner/work/evaOS-GUI/evaOS-GUI/�[39mpackages/desktop/src/common/adapter/httpBridge.ts:191:20�[90m)�[39m {
[cause]: TypeError: Invalid URL
�[90m at new URL (node:internal/url:818:25)�[39m
at new URL �[90m(file:///Users/runn...
GitHub Actions: PR Checks / Coverage Test: Restore Workbench Bridge resource proof
Conclusion: failure
[39m
at �[90m/home/runner/work/evaOS-GUI/evaOS-GUI/�[39mtests/unit/assets/presetAssistantResources.test.ts:44:21
at �[90mfile:///home/runner/work/evaOS-GUI/evaOS-GUI/�[39mnode_modules/�[4m.bun�[24m/@vitest+runner@4.1.0/node_modules/�[4m@vitest/runner�[24m/dist/index.js:1258:20 {
[cause]: Error: connect ECONNREFUSED 127.0.0.1:13400
�[90m at TCPConnectWrap.afterConnect [as oncomplete] (node:net:1638:16)�[39m {
errno: �[33m-111�[39m,
code: �[32m'ECONNREFUSED'�[39m,
syscall: �[32m'connect'�[39m,
address: �[32m'127.0.0.1'�[39m,
port: �[33m13400�[39m
}
}
�[32m✓�[39m �[30m�[43m node �[49m�[39m tests/unit/common/OpenAI2GeminiConverter.test.ts �[2m(�[22m�[2m2 tests�[22m�[2m)�[22m�[32m 4�[2mms�[22m�[39m
�[32m✓�[39m �[30m�[43m node �[49m�[39m tests/unit/assets/presetAssistantResources.test.ts �[2m(�[22m�[2m6 tests�[22m�[2m)�[22m�[32m 45�[2mms�[22m�[39m
�[32m✓�[39m �[30m�[43m node �[49m�[39m tests/unit/renderer/warmupConversation.test.ts �[2m(�[22m�[2m2 tests�[22m�[2m)�[22m�[32m 6�[2mms�[22m�[39m
�[32m✓�[39m �[30m�[43m node �[49m�[39m tests/unit/common/platformConstants.test.ts �[2m(�[22m�[2m6 tests�[22m�[2m)�[22m�[32m 5�[2mms�[22m�[39m
�[32m✓�[39m �[30m�[43m node �[49m�[39m tests/unit/extension/extensionMapperIntegration.test.ts �[2m(�[22m�[2m3 tests�[22m�[2m)�[22m�[32m 6�[2mms�[22m�[39m
�[32m✓�[39m �[30m�[43m node �[49m�[39m tests/unit/common-adapter/workspaceMapper.test.ts �[2m(�[22m�[2m2 tests�[22m�[2m)�[22m�[32m 5�[2mms�[22m�[39m
�[32m✓�[39m �[30m�[43m node �[49m�[39m tests/unit/common/normalizeToolCall.test.ts �[2m(�[22m�[2m1 test�[22m�[2m)�[22m�[32m 5�[2mms�[22m�[39m
�[32m✓�[39m �[30m�[43m node �[49m�[39m tests/unit/cron/cronUtils.test.ts �[2m(�[22m�[2m2 tests�[22m�[2m)�[22m�[32m 4�[2mms�[22m�[39m
�[32m✓�[39m �[30m�[43m node �[49m�[39m tests/unit/chat/atFileQuery.test.ts �[2m(�[22m�[2m3 tests�[22m�[2m)�[22m�[32m 5�[2mms�[22m�[39m
�[90mstdout�[2m | tests/unit/chat/toolCallLogging.test.ts
�[22m�[39mbuil...
GitHub Actions: PR Checks / 0_Thin App Smoke (macos-arm64).txt: Restore Workbench Bridge resource proof
Conclusion: failure
##[group]Run set -euo pipefail
�[36;1mset -euo pipefail�[0m
�[36;1mecho "=========================================="�[0m
�[36;1mecho "VERIFY UNPACKED APP RESOURCE SHAPE"�[0m
�[36;1mecho "=========================================="�[0m
�[36;1mAPP_PATH="$(find out -type d -name '*.app' -print -quit)"�[0m
�[36;1mif [ -z "$APP_PATH" ]; then�[0m
�[36;1m echo "::error::No .app produced under out/"�[0m
GitHub Actions: PR Checks / 4_Coverage Test.txt: Restore Workbench Bridge resource proof
Conclusion: failure
load.ts | 0 | 0 | 0 | 0 | 20-113
...leSelector.ts | 0 | 0 | 0 | 0 | 21-48
...steService.ts | 0 | 0 | 0 | 0 | 23-92
...ewLauncher.ts | 5.08 | 0 | 0 | 5.17 | 25-29,71-204
...ploadState.ts | 74.71 | 67.39 | 45 | 76.92 | ...49-250,255-276
...ceSelector.ts | 0 | 0 | 0 | 0 | 21-59
...erer/hooks/mcp | 0 | 0 | 0 | 0 |
catalog.ts | 0 | 0 | 0 | 0 | 16-117
index.ts | 0 | 0 | 0 | 0 |
messageQueue.ts | 0 | 0 | 0 | 0 | 15-77
...Connection.ts | 0 | 0 | 0 | 0 | 14-266
useMcpModal.ts | 0 | 100 | 0 | 0 | 8-50
useMcpOAuth.ts | 0 | 0 | 0 | 0 | 16-181
...ServerCRUD.ts | 0 | 0 | 0 | 0 | 9-148
useMcpServers.ts | 0 | 0 | 0 | 0 | 11-76
...r/hooks/system | 7.95 | 3.64 | 3.65 | 8.01 |
...lesEnabled.ts | 44.44 | 0 | 0 | 50 | 7,10,16-17
useDeepLink.ts | 0 | 0 | 0 | 0 | 27-91
useExtI18n.ts | 0 | 0 | 0 | 0 | 18-67
...ttingsTabs.ts | 12.12 | 0 | 0 | 13.33 | 21-78
...ationClick.ts | 0 | 0 | 0 | 0 | 15-34
...lDetection.ts | 0 | 0 | 0 | 0 | 59-186
usePwaMode.ts | 0 | 0 | 0 | 0 | 15-30
...peechInput.ts | 0 | 0 | 0 | 0 | 40-423
useTheme.ts | 58.69 | 70 | 33.33 | 57.77 | 53-95
...derer/hooks/ui | 12.11 | 3.24 | 4.28 | 12.1 |
...olorScheme.ts | 57.77 | 62.5 | 33.33 | 56.81 | 65-116
...nShortcuts.ts | 0 | 0 | 0 | 0 | 11-74
useDebounce.ts | 0 | 0 | 0 | 0 | 11-38
...
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Unused function parameters must be prefixed with underscore (_)
Use single-element arrays inline if they fit on one line; require trailing commas in multi-line arrays/objects; use single quotes for strings
Use English for code comments; provide JSDoc for public functions
Files:
scripts/build-with-builder.jstests/unit/bootstrap/prepareEvaosDesktopBridgeResource.test.tstests/unit/bootstrap/buildWithBuilder.test.tstests/unit/bootstrap/afterPackPackagingProfile.test.tsscripts/afterPack.jsscripts/prepareEvaosDesktopBridgeResource.js
**/*[!.module].ts
📄 CodeRabbit inference engine (AGENTS.md)
Utility files must use camelCase naming (e.g., formatDate.ts)
Files:
tests/unit/bootstrap/prepareEvaosDesktopBridgeResource.test.tstests/unit/bootstrap/buildWithBuilder.test.tstests/unit/bootstrap/afterPackPackagingProfile.test.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use@icon-park/reactfor icons
TypeScript strict mode must be enabled; no 'any' types, no implicit returns
Use path aliases:@/,@process/,@renderer/* for imports
Prefer 'type' over 'interface' for type definitions (per Oxlint config)
Files:
tests/unit/bootstrap/prepareEvaosDesktopBridgeResource.test.tstests/unit/bootstrap/buildWithBuilder.test.tstests/unit/bootstrap/afterPackPackagingProfile.test.ts
**/*.{test,spec}.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Vitest 4 for testing with a coverage target of at least 80%
Files:
tests/unit/bootstrap/prepareEvaosDesktopBridgeResource.test.tstests/unit/bootstrap/buildWithBuilder.test.tstests/unit/bootstrap/afterPackPackagingProfile.test.ts
🪛 ast-grep (0.44.0)
tests/unit/bootstrap/prepareEvaosDesktopBridgeResource.test.ts
[warning] 6-6: Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import { spawnSync } from 'node:child_process';
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process-typescript)
scripts/afterPack.js
[warning] 132-132: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.readFileSync(filePath, 'utf8')
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
[warning] 162-162: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.readFileSync(filePath, { encoding: null, flag: 'r' })
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
scripts/prepareEvaosDesktopBridgeResource.js
[error] 377-377: An archive entry path (e.g. entry.path / entry.fileName / header.name) is joined to an output directory without validating that the resolved path stays inside that directory. A malicious archive can use "../" sequences to escape the extraction directory and overwrite arbitrary files (Zip Slip). Resolve the path and verify it starts with the normalized output directory, or strip traversal with path.basename, before writing the entry.
Context: path.join(dir, entry.name)
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(zip-slip-archive-extraction-javascript)
[warning] 4-4: Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: require('child_process')
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process)
[warning] 192-192: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.readFileSync(filePath, { encoding: null, flag: 'r' })
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
[warning] 208-227: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.writeFileSync(
helperPath,
`#!/bin/sh
set -eu
HELPER_DIR="$(CDPATH= cd -- "$(dirname -- "$0")" && pwd)"
if [ -x "$HELPER_DIR/peekaboo" ]; then
exec "$HELPER_DIR/peekaboo" "$@"
fi
for candidate in /opt/homebrew/bin/peekaboo /usr/local/bin/peekaboo; do
if [ -x "$candidate" ]; then
exec "$candidate" "$@"
fi
done
echo "evaos-connector-helper: bundled peekaboo was not found. Rebuild evaOS Workbench with Bridge/bin/peekaboo or install the Workbench connector package." >&2
exit 127
`
)
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
[warning] 234-248: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.writeFileSync(
peekabooPath,
`#!/bin/sh
set -eu
for candidate in /opt/homebrew/bin/peekaboo /usr/local/bin/peekaboo; do
if [ -x "$candidate" ]; then
exec "$candidate" "$@"
fi
done
echo "peekaboo: bundled helper binary was not found. Rebuild evaOS Workbench with Bridge/bin/peekaboo or install the Workbench connector package." >&2
exit 127
`
)
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
[warning] 308-308: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.writeFileSync(wrapperPath, bridgeWrapperScript())
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
[warning] 328-342: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.writeFileSync(
wrapperPath,
`#!/bin/sh
set -eu
if [ "${1:-}" = "--version" ] || [ "${1:-}" = "version" ]; then
echo "evaos-desktop-bridge diagnostic-placeholder"
exit 0
fi
echo "evaos-desktop-bridge diagnostic placeholder: ${escapeForShellDoubleQuotes(reason)}" >&2
echo "This PR/build artifact is not valid for Mac pairing release proof. Configure EVAOS_DESKTOP_BRIDGE_SOURCE_DIR or EVAOS_DESKTOP_BRIDGE_SOURCE_TOKEN for a real release build." >&2
exit 78
`
)
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
[warning] 347-347: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.writeFileSync(path.join(bridgeResourceDir, 'manifest.json'), ${JSON.stringify(manifest, null, 2)}\n)
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
🪛 GitHub Actions: PR Checks / 5_Unit Tests (macos-14).txt
tests/unit/bootstrap/afterPackPackagingProfile.test.ts
[warning] 1-1: Packaging profile checks emitted warning: app.asar.unpacked not found.
🪛 GitHub Actions: PR Checks / Unit Tests (macos-14)
tests/unit/bootstrap/afterPackPackagingProfile.test.ts
[warning] 1-1: Packaging profile checks emitted warning: app.asar.unpacked not found.
🔇 Additional comments (5)
.github/workflows/pr-checks.yml (1)
728-728: Still fail this lane when the Bridge manifest is not a diagnostic placeholder.This step only changes the summary text. It still allows a real Bridge bundle to pass, which weakens the secret-free placeholder contract described for PR smoke. Fail here when
manifest.placeholder !== true..github/workflows/workbench-functional-smoke.yml (1)
176-181: Normalizebridge_refbefore comparing it to the manifest commit.
prepareEvaosDesktopBridgeResource.jsrecordssourceCommitasgit rev-parse HEAD, so valid inputs like branches, tags, or short SHAs resolve to a full commit in the manifest. The current exact string compare will reject those successful builds. Either require a full 40-character SHA up front or resolveWORKBENCH_SMOKE_BRIDGE_REFto its commit before this check.scripts/prepareEvaosDesktopBridgeResource.js (2)
58-82: Still bypasses pinned checkout through implicit local candidates.
functional-smokeuses the default pinned ref whenEVAOS_DESKTOP_BRIDGE_SOURCE_REFis unset, butsourceCandidates()still allows implicit local directories beforeprepareBridgeSourceCheckout(). This is the same pinned-proof bypass already raised on the previous revision.
445-454: Exported helpers still need JSDoc.This is the same public-helper documentation gap already raised on the previous revision. As per coding guidelines,
**/*.{ts,tsx,js,jsx}:provide JSDoc for public functions.Source: Coding guidelines
tests/unit/bootstrap/afterPackPackagingProfile.test.ts (1)
171-180: Still sensitive to ambient release/real-bridge env flags.This positive Darwin functional-smoke case still only isolates
EVAOS_PACKAGING_PROFILE, so inheritedEVAOS_DESKTOP_BRIDGE_REQUIRE_REAL/ release flags can force a different verification path and make the test nondeterministic.
c6255e9 to
ac6554f
Compare
coderabbitai
Bot
added
bug
ac6554f to
d37074d
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
.github/workflows/pr-checks.yml (1)
728-728: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winAssert this PR smoke bundles the diagnostic Bridge placeholder.
Line 728 only summarizes the manifest, so a real Bridge bundle can pass and stop proving the secret-free placeholder contract. Fail unless
m.placeholder === true.Proposed fix
- BRIDGE_SUMMARY="$(node -e "const fs=require('fs'); const m=JSON.parse(fs.readFileSync(process.argv[1], 'utf8')); console.log(m.placeholder ? 'diagnostic-placeholder' : (m.sourceCommit || m.commit || m.version || 'unknown'))" "$BRIDGE_MANIFEST")" + BRIDGE_SUMMARY="$(node -e "const fs=require('fs'); const m=JSON.parse(fs.readFileSync(process.argv[1], 'utf8')); if (!m.placeholder) { console.error('::error::Expected thin-app-smoke to bundle the diagnostic Bridge placeholder'); process.exit(1); } console.log('diagnostic-placeholder')" "$BRIDGE_MANIFEST")"🤖 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/pr-checks.yml at line 728, The Bridge manifest check in the pr-checks workflow only summarizes metadata and can pass for a real bundle, so update the BRIDGE_SUMMARY logic to explicitly assert the manifest’s placeholder flag is true. Use the existing manifest parsing around BRIDGE_MANIFEST/BRIDGE_SUMMARY and make the step fail unless m.placeholder === true, so this smoke test only validates the diagnostic Bridge placeholder contract.
🤖 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 `@scripts/afterPack.js`:
- Around line 160-174: The Bridge helper check in
isMachOExecutable/requireMachOExecutable only validates Mach-O format and can
miss architecture mismatches. Update the afterPack verification path to use
targetArch, pass it into the Bridge helper validation, and assert the helper
contains the expected arm64 or x86_64 architecture before allowing release-like
builds to pass. Make the change in the requireMachOExecutable flow and any
related Bridge helper verification calls so the architecture check is enforced
consistently.
---
Duplicate comments:
In @.github/workflows/pr-checks.yml:
- Line 728: The Bridge manifest check in the pr-checks workflow only summarizes
metadata and can pass for a real bundle, so update the BRIDGE_SUMMARY logic to
explicitly assert the manifest’s placeholder flag is true. Use the existing
manifest parsing around BRIDGE_MANIFEST/BRIDGE_SUMMARY and make the step fail
unless m.placeholder === true, so this smoke test only validates the diagnostic
Bridge placeholder contract.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 92b9216b-9870-4004-96ad-a54701892944
📒 Files selected for processing (11)
.github/workflows/_build-reusable.yml.github/workflows/pr-checks.yml.github/workflows/workbench-functional-smoke.yml.gitignorepackages/desktop/electron-builder.ymlscripts/afterPack.jsscripts/build-with-builder.jsscripts/prepareEvaosDesktopBridgeResource.jstests/unit/bootstrap/afterPackPackagingProfile.test.tstests/unit/bootstrap/buildWithBuilder.test.tstests/unit/bootstrap/prepareEvaosDesktopBridgeResource.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (3)
- GitHub Check: Thin App Smoke (macos-arm64)
- GitHub Check: macOS arm64 unpacked app
- GitHub Check: Coverage Test
🧰 Additional context used
📓 Path-based instructions (4)
**/*[!.module].ts
📄 CodeRabbit inference engine (AGENTS.md)
Utility files must use camelCase naming (e.g., formatDate.ts)
Files:
tests/unit/bootstrap/buildWithBuilder.test.tstests/unit/bootstrap/afterPackPackagingProfile.test.tstests/unit/bootstrap/prepareEvaosDesktopBridgeResource.test.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Unused function parameters must be prefixed with underscore (_)
Use single-element arrays inline if they fit on one line; require trailing commas in multi-line arrays/objects; use single quotes for strings
Use English for code comments; provide JSDoc for public functions
Files:
tests/unit/bootstrap/buildWithBuilder.test.tsscripts/afterPack.jsscripts/build-with-builder.jstests/unit/bootstrap/afterPackPackagingProfile.test.tstests/unit/bootstrap/prepareEvaosDesktopBridgeResource.test.tsscripts/prepareEvaosDesktopBridgeResource.js
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use@icon-park/reactfor icons
TypeScript strict mode must be enabled; no 'any' types, no implicit returns
Use path aliases:@/,@process/,@renderer/* for imports
Prefer 'type' over 'interface' for type definitions (per Oxlint config)
Files:
tests/unit/bootstrap/buildWithBuilder.test.tstests/unit/bootstrap/afterPackPackagingProfile.test.tstests/unit/bootstrap/prepareEvaosDesktopBridgeResource.test.ts
**/*.{test,spec}.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Vitest 4 for testing with a coverage target of at least 80%
Files:
tests/unit/bootstrap/buildWithBuilder.test.tstests/unit/bootstrap/afterPackPackagingProfile.test.tstests/unit/bootstrap/prepareEvaosDesktopBridgeResource.test.ts
🪛 ast-grep (0.44.0)
scripts/afterPack.js
[warning] 132-132: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.readFileSync(filePath, 'utf8')
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
[warning] 163-163: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.readFileSync(filePath, { encoding: null, flag: 'r' })
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
tests/unit/bootstrap/prepareEvaosDesktopBridgeResource.test.ts
[warning] 6-6: Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import { spawnSync } from 'node:child_process';
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process-typescript)
scripts/prepareEvaosDesktopBridgeResource.js
[warning] 4-4: Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: require('child_process')
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process)
[error] 412-412: An archive entry path (e.g. entry.path / entry.fileName / header.name) is joined to an output directory without validating that the resolved path stays inside that directory. A malicious archive can use "../" sequences to escape the extraction directory and overwrite arbitrary files (Zip Slip). Resolve the path and verify it starts with the normalized output directory, or strip traversal with path.basename, before writing the entry.
Context: path.join(dir, entry.name)
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(zip-slip-archive-extraction-javascript)
[warning] 226-226: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.readFileSync(filePath, { encoding: null, flag: 'r' })
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
[warning] 242-261: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.writeFileSync(
helperPath,
`#!/bin/sh
set -eu
HELPER_DIR="$(CDPATH= cd -- "$(dirname -- "$0")" && pwd)"
if [ -x "$HELPER_DIR/peekaboo" ]; then
exec "$HELPER_DIR/peekaboo" "$@"
fi
for candidate in /opt/homebrew/bin/peekaboo /usr/local/bin/peekaboo; do
if [ -x "$candidate" ]; then
exec "$candidate" "$@"
fi
done
echo "evaos-connector-helper: bundled peekaboo was not found. Rebuild evaOS Workbench with Bridge/bin/peekaboo or install the Workbench connector package." >&2
exit 127
`
)
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
[warning] 268-282: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.writeFileSync(
peekabooPath,
`#!/bin/sh
set -eu
for candidate in /opt/homebrew/bin/peekaboo /usr/local/bin/peekaboo; do
if [ -x "$candidate" ]; then
exec "$candidate" "$@"
fi
done
echo "peekaboo: bundled helper binary was not found. Rebuild evaOS Workbench with Bridge/bin/peekaboo or install the Workbench connector package." >&2
exit 127
`
)
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
[warning] 342-342: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.writeFileSync(wrapperPath, bridgeWrapperScript())
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
[warning] 362-376: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.writeFileSync(
wrapperPath,
`#!/bin/sh
set -eu
if [ "${1:-}" = "--version" ] || [ "${1:-}" = "version" ]; then
echo "evaos-desktop-bridge diagnostic-placeholder"
exit 0
fi
echo "evaos-desktop-bridge diagnostic placeholder: ${escapeForShellDoubleQuotes(reason)}" >&2
echo "This PR/build artifact is not valid for Mac pairing release proof. Configure EVAOS_DESKTOP_BRIDGE_SOURCE_DIR or EVAOS_DESKTOP_BRIDGE_SOURCE_TOKEN for a real release build." >&2
exit 78
`
)
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
[warning] 381-381: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.writeFileSync(path.join(bridgeResourceDir, 'manifest.json'), ${JSON.stringify(manifest, null, 2)}\n)
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
🔇 Additional comments (15)
scripts/afterPack.js (1)
11-15: LGTM!Also applies to: 40-52, 131-159, 188-213, 219-221, 339-339
.github/workflows/pr-checks.yml (1)
655-657: LGTM!Also applies to: 683-702, 738-741
.github/workflows/workbench-functional-smoke.yml (1)
19-23: LGTM!Also applies to: 38-38, 115-143, 176-186, 204-204
tests/unit/bootstrap/afterPackPackagingProfile.test.ts (1)
7-35: LGTM!Also applies to: 54-71, 81-136, 146-153, 196-261
tests/unit/bootstrap/buildWithBuilder.test.ts (1)
110-118: LGTM!Also applies to: 132-142, 187-190
scripts/prepareEvaosDesktopBridgeResource.js (4)
53-58:isMutableBridgeSourceRefstill only blocks baremain/master/head. Branch-qualified refs (refs/heads/main,origin/main,develop) still pass and keep proof lanes on mutable sources. Require a pinned commit object ID, or resolve the ref and compare object IDs.
130-142: Tokenized remote persists in.git/config.repoWithToken()writeshttps://x-access-token:<token>@github.com/...asremote.origin.url; resetoriginto the clean URL after both the primary clone and the fallback fetch/checkout path so the cached repo never retains the token.
481-490: Exported helpers lack JSDoc. The public surface (bridgeWrapperScript,isMachOExecutable,isMutableBridgeSourceRef,resolveBridgeSourceDir,sanitizeCommandText,shouldRejectPlaceholder,sourceCandidates) still exposes env-contracts/side effects without docs. As per coding guidelines:provide JSDoc for public functions.
108-117:resolveBridgeSourceDirnow runsassertPinnedSourceDir(sourceDir)before returning a local candidate, so pinned-proof lanes validate the local checkout against the requested ref instead of silently accepting an arbitrary local HEAD. Prior bypass concern resolved.LGTM!
tests/unit/bootstrap/prepareEvaosDesktopBridgeResource.test.ts (1)
86-90:runPreparestill overwritesPATHwith a POSIX-only list, which can hide inheritedgit/tooling on Windows or custom installs and fail for reasons unrelated to the behavior under test. Preserve the inheritedPATH(or append to it) instead of replacing it..gitignore (1)
207-207: LGTM!packages/desktop/electron-builder.yml (1)
136-138: DestinationBridgematches the layout verified inscripts/afterPack.js(Bridge/evaos-desktop-bridge,Bridge/manifest.json). Contract is consistent.LGTM!
.github/workflows/_build-reusable.yml (1)
452-452: LGTM!scripts/build-with-builder.js (2)
122-191: The indentation-tracking rewrite correctly handles the nestedmac.extraResourcesblock, andresources/BridgeinTHIN_SHELL_REMOVED_RESOURCES(Line 33) ensures the thin-shell config strips the Bridge mapping — so a stale on-diskresources/Bridgewon't be packaged into a thin-shell build. This addresses the earlier stale-leak concern.LGTM!
599-605: macOS-gated Bridge prep aligns withelectron-builder.ymlshipping Bridge only undermac:. Consistent.LGTM!
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (4)
.github/workflows/pr-checks.yml (1)
728-728: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winEnforce the diagnostic-placeholder contract.
This only summarizes
manifest.placeholder; it does not fail if the smoke artifact bundles a real Bridge. Assertm.placeholder === trueso this PR lane continues proving the secret-free placeholder path.Possible fix
- BRIDGE_SUMMARY="$(node -e "const fs=require('fs'); const m=JSON.parse(fs.readFileSync(process.argv[1], 'utf8')); console.log(m.placeholder ? 'diagnostic-placeholder' : (m.sourceCommit || m.commit || m.version || 'unknown'))" "$BRIDGE_MANIFEST")" + BRIDGE_SUMMARY="$(node -e "const fs=require('fs'); const m=JSON.parse(fs.readFileSync(process.argv[1], 'utf8')); if (!m.placeholder) { console.error('::error::Expected thin-app-smoke to bundle the diagnostic Bridge placeholder'); process.exit(1); } console.log('diagnostic-placeholder')" "$BRIDGE_MANIFEST")"🤖 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/pr-checks.yml at line 728, The BRIDGE_SUMMARY logic in the workflow only reports whether the manifest has a placeholder flag, but it does not enforce the diagnostic-placeholder contract. Update the Node check in the workflow step that reads BRIDGE_MANIFEST so it explicitly asserts m.placeholder === true and fails the job if the smoke artifact contains a real Bridge, keeping the PR lane on the secret-free placeholder path.scripts/prepareEvaosDesktopBridgeResource.js (2)
53-58: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winReject all branch-like refs in proof lanes.
This still accepts mutable refs such as
refs/heads/main,origin/main, ordevelop, so functional-smoke/release proof can fetch moving source. Require a full commit SHA here, or resolve the ref and persist/compare the resolved commit before allowing it.Possible SHA-only guard
function isMutableBridgeSourceRef(ref) { const normalized = String(ref || '') .trim() .toLowerCase(); - return !normalized || normalized === 'main' || normalized === 'master' || normalized === 'head'; + return !/^[0-9a-f]{40}$/.test(normalized); }🤖 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 `@scripts/prepareEvaosDesktopBridgeResource.js` around lines 53 - 58, The mutable-ref check in isMutableBridgeSourceRef is too permissive because it still allows branch-like inputs such as refs/heads/main, origin/main, and develop. Tighten the guard so proof lanes only accept a full commit SHA, or resolve any ref to a commit and persist/compare that resolved SHA before proceeding. Update the validation path in prepareEvaosDesktopBridgeResource to reject all non-SHA refs consistently.
491-500: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAdd JSDoc to the exported helpers.
These functions are now public module surface for tests/callers; document their env contracts and side effects at the declarations. As per coding guidelines,
**/*.{ts,tsx,js,jsx}:provide JSDoc for public functions.🤖 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 `@scripts/prepareEvaosDesktopBridgeResource.js` around lines 491 - 500, The exported helpers in the module surface need JSDoc because they are now public for tests and callers. Add concise JSDoc at each helper declaration, especially bridgeWrapperScript, defaultBridgeSourceRef, isMachOExecutable, isMutableBridgeSourceRef, resolveBridgeSourceDir, sanitizeCommandText, shouldRejectPlaceholder, and sourceCandidates, documenting any environment assumptions, inputs/outputs, and side effects so the public contract is clear.Source: Coding guidelines
scripts/afterPack.js (1)
171-174: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winValidate Bridge helper architecture, not just Mach-O format.
targetArchis available, butrequireMachOExecutable()only checks magic bytes. Adarwin-arm64build can still pass with an x64-only helper. PasstargetArchinto Bridge verification and assert the helper contains the expectedarm64/x86_64slice before release-like builds pass.Also applies to: 214-216, 413-413
🤖 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 `@scripts/afterPack.js` around lines 171 - 174, The Bridge helper validation in requireMachOExecutable() only checks that the file is a Mach-O binary, so update the Bridge verification flow to also use targetArch and confirm the helper includes the expected arm64 or x86_64 slice before release-like builds succeed. Pass targetArch into the relevant Bridge check path, inspect the helper architecture in the same area as requireMachOExecutable(), and fail with a clear error if the slice does not match the build target.
🤖 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 `@scripts/prepareEvaosDesktopBridgeResource.js`:
- Line 476: The manifest entry built in prepareEvaosDesktopBridgeResource is
leaking an absolute local path via sourcePath: bridgeSourceDir. Update the
resource metadata construction to stop storing bridgeSourceDir directly, and
instead use a non-sensitive source label while preserving requestedSourceRef and
sourceCommit. Locate the manifest object in prepareEvaosDesktopBridgeResource
and replace the sourcePath assignment with a safe identifier that won’t expose
usernames or workspace paths.
In `@tests/unit/bootstrap/afterPackPackagingProfile.test.ts`:
- Around line 22-35: The Bridge test setup is leaving appleId in the
environment, which can make the afterPackPackagingProfile tests behave like
release validation unexpectedly. Update the shared env helpers in
afterPackPackagingProfile.test.ts—specifically clearBridgeTestEnv and
restoreBridgeTestEnv—to include appleId alongside the other Bridge test env
vars, so the functional-smoke path in afterPack stays isolated from any
preexisting appleId value.
In `@tests/unit/bootstrap/prepareEvaosDesktopBridgeResource.test.ts`:
- Around line 51-60: The fixture setup in prepareEvaosDesktopBridgeResource.test
relies on several spawnSync git calls but ignores their exit codes, so failures
are masked until later assertions. Update the test helper around the git
init/config/add/commit and rev-parse steps to check each spawnSync result and
fail immediately with the actual git error/output if any command fails, so the
setup error is reported at the source instead of during Bridge preparation.
- Line 126: The fake repository URI is being built with manual file:// string
interpolation, which breaks on Windows and prevents the missing-repo path from
being exercised correctly. Update the test setup in
prepareEvaosDesktopBridgeResource.test.ts to use pathToFileURL() from node:url
and reference its href for both fake repo URI cases, keeping the existing
createTempDir and join usage but avoiding direct string concatenation.
---
Duplicate comments:
In @.github/workflows/pr-checks.yml:
- Line 728: The BRIDGE_SUMMARY logic in the workflow only reports whether the
manifest has a placeholder flag, but it does not enforce the
diagnostic-placeholder contract. Update the Node check in the workflow step that
reads BRIDGE_MANIFEST so it explicitly asserts m.placeholder === true and fails
the job if the smoke artifact contains a real Bridge, keeping the PR lane on the
secret-free placeholder path.
In `@scripts/afterPack.js`:
- Around line 171-174: The Bridge helper validation in requireMachOExecutable()
only checks that the file is a Mach-O binary, so update the Bridge verification
flow to also use targetArch and confirm the helper includes the expected arm64
or x86_64 slice before release-like builds succeed. Pass targetArch into the
relevant Bridge check path, inspect the helper architecture in the same area as
requireMachOExecutable(), and fail with a clear error if the slice does not
match the build target.
In `@scripts/prepareEvaosDesktopBridgeResource.js`:
- Around line 53-58: The mutable-ref check in isMutableBridgeSourceRef is too
permissive because it still allows branch-like inputs such as refs/heads/main,
origin/main, and develop. Tighten the guard so proof lanes only accept a full
commit SHA, or resolve any ref to a commit and persist/compare that resolved SHA
before proceeding. Update the validation path in
prepareEvaosDesktopBridgeResource to reject all non-SHA refs consistently.
- Around line 491-500: The exported helpers in the module surface need JSDoc
because they are now public for tests and callers. Add concise JSDoc at each
helper declaration, especially bridgeWrapperScript, defaultBridgeSourceRef,
isMachOExecutable, isMutableBridgeSourceRef, resolveBridgeSourceDir,
sanitizeCommandText, shouldRejectPlaceholder, and sourceCandidates, documenting
any environment assumptions, inputs/outputs, and side effects so the public
contract is clear.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 4072a674-d1df-4170-aa2d-db72664e03ba
📒 Files selected for processing (11)
.github/workflows/_build-reusable.yml.github/workflows/pr-checks.yml.github/workflows/workbench-functional-smoke.yml.gitignorepackages/desktop/electron-builder.ymlscripts/afterPack.jsscripts/build-with-builder.jsscripts/prepareEvaosDesktopBridgeResource.jstests/unit/bootstrap/afterPackPackagingProfile.test.tstests/unit/bootstrap/buildWithBuilder.test.tstests/unit/bootstrap/prepareEvaosDesktopBridgeResource.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (3)
- GitHub Check: Thin App Smoke (macos-arm64)
- GitHub Check: macOS arm64 unpacked app
- GitHub Check: Coverage Test
🧰 Additional context used
📓 Path-based instructions (4)
**/*[!.module].ts
📄 CodeRabbit inference engine (AGENTS.md)
Utility files must use camelCase naming (e.g., formatDate.ts)
Files:
tests/unit/bootstrap/buildWithBuilder.test.tstests/unit/bootstrap/prepareEvaosDesktopBridgeResource.test.tstests/unit/bootstrap/afterPackPackagingProfile.test.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Unused function parameters must be prefixed with underscore (_)
Use single-element arrays inline if they fit on one line; require trailing commas in multi-line arrays/objects; use single quotes for strings
Use English for code comments; provide JSDoc for public functions
Files:
tests/unit/bootstrap/buildWithBuilder.test.tsscripts/build-with-builder.jstests/unit/bootstrap/prepareEvaosDesktopBridgeResource.test.tsscripts/afterPack.jstests/unit/bootstrap/afterPackPackagingProfile.test.tsscripts/prepareEvaosDesktopBridgeResource.js
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use@icon-park/reactfor icons
TypeScript strict mode must be enabled; no 'any' types, no implicit returns
Use path aliases:@/,@process/,@renderer/* for imports
Prefer 'type' over 'interface' for type definitions (per Oxlint config)
Files:
tests/unit/bootstrap/buildWithBuilder.test.tstests/unit/bootstrap/prepareEvaosDesktopBridgeResource.test.tstests/unit/bootstrap/afterPackPackagingProfile.test.ts
**/*.{test,spec}.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Vitest 4 for testing with a coverage target of at least 80%
Files:
tests/unit/bootstrap/buildWithBuilder.test.tstests/unit/bootstrap/prepareEvaosDesktopBridgeResource.test.tstests/unit/bootstrap/afterPackPackagingProfile.test.ts
🪛 ast-grep (0.44.0)
tests/unit/bootstrap/prepareEvaosDesktopBridgeResource.test.ts
[warning] 6-6: Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import { spawnSync } from 'node:child_process';
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process-typescript)
scripts/afterPack.js
[warning] 132-132: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.readFileSync(filePath, 'utf8')
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
[warning] 163-163: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.readFileSync(filePath, { encoding: null, flag: 'r' })
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
scripts/prepareEvaosDesktopBridgeResource.js
[error] 422-422: An archive entry path (e.g. entry.path / entry.fileName / header.name) is joined to an output directory without validating that the resolved path stays inside that directory. A malicious archive can use "../" sequences to escape the extraction directory and overwrite arbitrary files (Zip Slip). Resolve the path and verify it starts with the normalized output directory, or strip traversal with path.basename, before writing the entry.
Context: path.join(dir, entry.name)
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(zip-slip-archive-extraction-javascript)
[warning] 236-236: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.readFileSync(filePath, { encoding: null, flag: 'r' })
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
[warning] 252-271: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.writeFileSync(
helperPath,
`#!/bin/sh
set -eu
HELPER_DIR="$(CDPATH= cd -- "$(dirname -- "$0")" && pwd)"
if [ -x "$HELPER_DIR/peekaboo" ]; then
exec "$HELPER_DIR/peekaboo" "$@"
fi
for candidate in /opt/homebrew/bin/peekaboo /usr/local/bin/peekaboo; do
if [ -x "$candidate" ]; then
exec "$candidate" "$@"
fi
done
echo "evaos-connector-helper: bundled peekaboo was not found. Rebuild evaOS Workbench with Bridge/bin/peekaboo or install the Workbench connector package." >&2
exit 127
`
)
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
[warning] 278-292: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.writeFileSync(
peekabooPath,
`#!/bin/sh
set -eu
for candidate in /opt/homebrew/bin/peekaboo /usr/local/bin/peekaboo; do
if [ -x "$candidate" ]; then
exec "$candidate" "$@"
fi
done
echo "peekaboo: bundled helper binary was not found. Rebuild evaOS Workbench with Bridge/bin/peekaboo or install the Workbench connector package." >&2
exit 127
`
)
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
[warning] 352-352: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.writeFileSync(wrapperPath, bridgeWrapperScript())
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
[warning] 372-386: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.writeFileSync(
wrapperPath,
`#!/bin/sh
set -eu
if [ "${1:-}" = "--version" ] || [ "${1:-}" = "version" ]; then
echo "evaos-desktop-bridge diagnostic-placeholder"
exit 0
fi
echo "evaos-desktop-bridge diagnostic placeholder: ${escapeForShellDoubleQuotes(reason)}" >&2
echo "This PR/build artifact is not valid for Mac pairing release proof. Configure EVAOS_DESKTOP_BRIDGE_SOURCE_DIR or EVAOS_DESKTOP_BRIDGE_SOURCE_TOKEN for a real release build." >&2
exit 78
`
)
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
[warning] 391-391: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.writeFileSync(path.join(bridgeResourceDir, 'manifest.json'), ${JSON.stringify(manifest, null, 2)}\n)
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
[warning] 4-4: Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: require('child_process')
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process)
🔇 Additional comments (9)
.gitignore (1)
207-207: LGTM!packages/desktop/electron-builder.yml (1)
136-138: LGTM!.github/workflows/_build-reusable.yml (1)
452-452: LGTM!scripts/build-with-builder.js (1)
126-154: LGTM!Also applies to: 274-279, 572-572, 599-605
scripts/prepareEvaosDesktopBridgeResource.js (1)
1-52: LGTM!Also applies to: 60-475, 477-490
scripts/afterPack.js (1)
11-15: LGTM!Also applies to: 40-158, 188-213, 219-220, 339-339
.github/workflows/pr-checks.yml (1)
655-657: LGTM!Also applies to: 683-702, 738-741
.github/workflows/workbench-functional-smoke.yml (1)
19-23: LGTM!Also applies to: 38-38, 115-143, 176-186, 204-204
tests/unit/bootstrap/buildWithBuilder.test.ts (1)
110-142: LGTM!Also applies to: 187-190
d37074d to
bcddb37
Compare
bcddb37 to
697783e
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
scripts/prepareEvaosDesktopBridgeResource.js (2)
55-60: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winReject all branch-like refs in proof lanes.
This only rejects exact
main/master/head, sorefs/heads/main,origin/main, anddevelopstill pass and can package a moving Bridge source. Require a full commit SHA for proof/release lanes, or resolve the ref to an immutable object ID before checkout.Possible fix
function isMutableBridgeSourceRef(ref) { const normalized = String(ref || '') .trim() .toLowerCase(); - return !normalized || normalized === 'main' || normalized === 'master' || normalized === 'head'; + return !/^[0-9a-f]{40}$/.test(normalized); }🤖 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 `@scripts/prepareEvaosDesktopBridgeResource.js` around lines 55 - 60, The Bridge source ref validation in isMutableBridgeSourceRef is too narrow and still allows branch-like refs such as refs/heads/main, origin/main, and develop to pass. Update the proof/release gating logic in prepareEvaosDesktopBridgeResource.js to reject any non-immutable ref and only accept a full commit SHA, or resolve the supplied ref to an immutable object ID before checkout. Make sure the existing mutable-ref check and any checkout/package path using isMutableBridgeSourceRef are updated consistently.
167-169: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winDo not persist credentials supplied in
EVAOS_DESKTOP_BRIDGE_SOURCE_REPO.
resetBridgeSourceOrigin(repo)writes the configured URL into.git/config. If that URL already contains credentials, the cache still stores the token.Possible fix
+function repoUrlForStoredOrigin(repo) { + try { + const url = new URL(repo); + if (url.protocol === 'https:' && (url.username || url.password)) { + url.username = ''; + url.password = ''; + return url.toString(); + } + } catch { + // Local paths and scp-style Git remotes are not WHATWG URLs. + } + return repo; +} + function resetBridgeSourceOrigin(repo) { if (!fs.existsSync(path.join(bridgeSourceCacheDir, '.git'))) return; - runGit(['remote', 'set-url', 'origin', repo], bridgeSourceCacheDir, repo); + runGit(['remote', 'set-url', 'origin', repoUrlForStoredOrigin(repo)], bridgeSourceCacheDir, repo); }🤖 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 `@scripts/prepareEvaosDesktopBridgeResource.js` around lines 167 - 169, `resetBridgeSourceOrigin(repo)` is persisting the exact `EVAOS_DESKTOP_BRIDGE_SOURCE_REPO` URL into the bridge cache’s `.git/config`, so sanitize the repo URL before calling `runGit(['remote', 'set-url', 'origin', ...])` and strip any embedded credentials first. Update the helper to derive a credential-free remote URL (or rebuild it from the parsed URL without username/password) and use that value when resetting `origin`, while keeping the rest of the cache reset behavior unchanged.tests/unit/bootstrap/afterPackPackagingProfile.test.ts (1)
22-24: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winMirror all Bridge release env reads in the test isolation list.
afterPackreadsEVAOS_BETA_PUBLIC_RELEASEandEVAOS_BETA_REQUIRE_SIGNINGdirectly. Add them explicitly here; theSetwill dedupe ifRELEASE_ENV_FLAGSalready includes them.Proposed fix
const BRIDGE_ENV_FLAGS = Array.from( - new Set([...RELEASE_ENV_FLAGS, 'appleId', 'APPLE_ID', 'EVAOS_DESKTOP_BRIDGE_REQUIRE_REAL', 'EVAOS_PACKAGING_PROFILE']) + new Set([ + ...RELEASE_ENV_FLAGS, + 'appleId', + 'APPLE_ID', + 'EVAOS_DESKTOP_BRIDGE_REQUIRE_REAL', + 'EVAOS_BETA_PUBLIC_RELEASE', + 'EVAOS_BETA_REQUIRE_SIGNING', + 'EVAOS_PACKAGING_PROFILE', + ]) );🤖 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 `@tests/unit/bootstrap/afterPackPackagingProfile.test.ts` around lines 22 - 24, The afterPack packaging profile test isolation list is missing Bridge release env variables that are read directly by the code. Update BRIDGE_ENV_FLAGS in afterPackPackagingProfile.test.ts to explicitly include EVAOS_BETA_PUBLIC_RELEASE and EVAOS_BETA_REQUIRE_SIGNING alongside the existing RELEASE_ENV_FLAGS entries, so the test mirrors all Bridge release env reads and the Set can dedupe duplicates..github/workflows/pr-checks.yml (1)
728-728: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winEnforce the placeholder contract before summarizing.
This smoke lane allows placeholders and disables default Bridge candidates, so it should fail if a real Bridge manifest appears instead of proving the diagnostic-placeholder path.
Proposed fix
- BRIDGE_SUMMARY="$(node -e "const fs=require('fs'); const m=JSON.parse(fs.readFileSync(process.argv[1], 'utf8')); console.log(m.placeholder ? 'diagnostic-placeholder' : (m.sourceCommit || m.commit || m.version || 'unknown'))" "$BRIDGE_MANIFEST")" + BRIDGE_SUMMARY="$(node -e "const fs=require('fs'); const m=JSON.parse(fs.readFileSync(process.argv[1], 'utf8')); if (m.placeholder !== true) { console.error('::error::Expected thin-app-smoke to bundle the diagnostic Bridge placeholder'); process.exit(1); } console.log('diagnostic-placeholder')" "$BRIDGE_MANIFEST")"🤖 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/pr-checks.yml at line 728, The Bridge summary step is accepting real manifest values instead of enforcing the diagnostic-placeholder contract. Update the workflow logic around the BRIDGE_SUMMARY assignment to explicitly verify m.placeholder is true before producing the summary, and fail the smoke lane when a real manifest is present; use the existing BRIDGE_MANIFEST parsing and the BRIDGE_SUMMARY step to keep the check close to where the manifest is summarized.
🤖 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 `@tests/unit/bootstrap/prepareEvaosDesktopBridgeResource.test.ts`:
- Around line 167-179: Expand the proof-lane guard test in
prepareEvaosDesktopBridgeResource to cover branch-alias mutable refs instead of
only exact "main". Add cases in the existing "rejects mutable bridge refs for
release-like proof lanes" test for "refs/heads/main", "origin/main", and another
non-pinned branch name, and keep asserting the runPrepare result fails with the
pinned-ref error so the guard is checked against alias forms too.
---
Duplicate comments:
In @.github/workflows/pr-checks.yml:
- Line 728: The Bridge summary step is accepting real manifest values instead of
enforcing the diagnostic-placeholder contract. Update the workflow logic around
the BRIDGE_SUMMARY assignment to explicitly verify m.placeholder is true before
producing the summary, and fail the smoke lane when a real manifest is present;
use the existing BRIDGE_MANIFEST parsing and the BRIDGE_SUMMARY step to keep the
check close to where the manifest is summarized.
In `@scripts/prepareEvaosDesktopBridgeResource.js`:
- Around line 55-60: The Bridge source ref validation in
isMutableBridgeSourceRef is too narrow and still allows branch-like refs such as
refs/heads/main, origin/main, and develop to pass. Update the proof/release
gating logic in prepareEvaosDesktopBridgeResource.js to reject any non-immutable
ref and only accept a full commit SHA, or resolve the supplied ref to an
immutable object ID before checkout. Make sure the existing mutable-ref check
and any checkout/package path using isMutableBridgeSourceRef are updated
consistently.
- Around line 167-169: `resetBridgeSourceOrigin(repo)` is persisting the exact
`EVAOS_DESKTOP_BRIDGE_SOURCE_REPO` URL into the bridge cache’s `.git/config`, so
sanitize the repo URL before calling `runGit(['remote', 'set-url', 'origin',
...])` and strip any embedded credentials first. Update the helper to derive a
credential-free remote URL (or rebuild it from the parsed URL without
username/password) and use that value when resetting `origin`, while keeping the
rest of the cache reset behavior unchanged.
In `@tests/unit/bootstrap/afterPackPackagingProfile.test.ts`:
- Around line 22-24: The afterPack packaging profile test isolation list is
missing Bridge release env variables that are read directly by the code. Update
BRIDGE_ENV_FLAGS in afterPackPackagingProfile.test.ts to explicitly include
EVAOS_BETA_PUBLIC_RELEASE and EVAOS_BETA_REQUIRE_SIGNING alongside the existing
RELEASE_ENV_FLAGS entries, so the test mirrors all Bridge release env reads and
the Set can dedupe duplicates.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: e1934291-4bf9-4c1a-ab84-3072bc2c358f
📒 Files selected for processing (11)
.github/workflows/_build-reusable.yml.github/workflows/pr-checks.yml.github/workflows/workbench-functional-smoke.yml.gitignorepackages/desktop/electron-builder.ymlscripts/afterPack.jsscripts/build-with-builder.jsscripts/prepareEvaosDesktopBridgeResource.jstests/unit/bootstrap/afterPackPackagingProfile.test.tstests/unit/bootstrap/buildWithBuilder.test.tstests/unit/bootstrap/prepareEvaosDesktopBridgeResource.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: Thin App Smoke (macos-arm64)
- GitHub Check: macOS arm64 unpacked app
🧰 Additional context used
📓 Path-based instructions (4)
**/*[!.module].ts
📄 CodeRabbit inference engine (AGENTS.md)
Utility files must use camelCase naming (e.g., formatDate.ts)
Files:
tests/unit/bootstrap/buildWithBuilder.test.tstests/unit/bootstrap/afterPackPackagingProfile.test.tstests/unit/bootstrap/prepareEvaosDesktopBridgeResource.test.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Unused function parameters must be prefixed with underscore (_)
Use single-element arrays inline if they fit on one line; require trailing commas in multi-line arrays/objects; use single quotes for strings
Use English for code comments; provide JSDoc for public functions
Files:
tests/unit/bootstrap/buildWithBuilder.test.tstests/unit/bootstrap/afterPackPackagingProfile.test.tsscripts/build-with-builder.jsscripts/afterPack.jstests/unit/bootstrap/prepareEvaosDesktopBridgeResource.test.tsscripts/prepareEvaosDesktopBridgeResource.js
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use@icon-park/reactfor icons
TypeScript strict mode must be enabled; no 'any' types, no implicit returns
Use path aliases:@/,@process/,@renderer/* for imports
Prefer 'type' over 'interface' for type definitions (per Oxlint config)
Files:
tests/unit/bootstrap/buildWithBuilder.test.tstests/unit/bootstrap/afterPackPackagingProfile.test.tstests/unit/bootstrap/prepareEvaosDesktopBridgeResource.test.ts
**/*.{test,spec}.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Vitest 4 for testing with a coverage target of at least 80%
Files:
tests/unit/bootstrap/buildWithBuilder.test.tstests/unit/bootstrap/afterPackPackagingProfile.test.tstests/unit/bootstrap/prepareEvaosDesktopBridgeResource.test.ts
🪛 ast-grep (0.44.0)
scripts/afterPack.js
[warning] 137-137: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.readFileSync(filePath, 'utf8')
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
[warning] 168-168: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.readFileSync(filePath, { encoding: null, flag: 'r' })
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
[warning] 176-176: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.readFileSync(filePath, { encoding: null, flag: 'r' })
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
tests/unit/bootstrap/prepareEvaosDesktopBridgeResource.test.ts
[warning] 6-6: Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import { spawnSync } from 'node:child_process';
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process-typescript)
scripts/prepareEvaosDesktopBridgeResource.js
[warning] 243-243: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.readFileSync(filePath, { encoding: null, flag: 'r' })
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
[warning] 259-278: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.writeFileSync(
helperPath,
`#!/bin/sh
set -eu
HELPER_DIR="$(CDPATH= cd -- "$(dirname -- "$0")" && pwd)"
if [ -x "$HELPER_DIR/peekaboo" ]; then
exec "$HELPER_DIR/peekaboo" "$@"
fi
for candidate in /opt/homebrew/bin/peekaboo /usr/local/bin/peekaboo; do
if [ -x "$candidate" ]; then
exec "$candidate" "$@"
fi
done
echo "evaos-connector-helper: bundled peekaboo was not found. Rebuild evaOS Workbench with Bridge/bin/peekaboo or install the Workbench connector package." >&2
exit 127
`
)
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
[warning] 285-299: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.writeFileSync(
peekabooPath,
`#!/bin/sh
set -eu
for candidate in /opt/homebrew/bin/peekaboo /usr/local/bin/peekaboo; do
if [ -x "$candidate" ]; then
exec "$candidate" "$@"
fi
done
echo "peekaboo: bundled helper binary was not found. Rebuild evaOS Workbench with Bridge/bin/peekaboo or install the Workbench connector package." >&2
exit 127
`
)
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
[warning] 360-360: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.writeFileSync(wrapperPath, bridgeWrapperScript())
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
[warning] 380-394: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.writeFileSync(
wrapperPath,
`#!/bin/sh
set -eu
if [ "${1:-}" = "--version" ] || [ "${1:-}" = "version" ]; then
echo "evaos-desktop-bridge diagnostic-placeholder"
exit 0
fi
echo "evaos-desktop-bridge diagnostic placeholder: ${escapeForShellDoubleQuotes(reason)}" >&2
echo "This PR/build artifact is not valid for Mac pairing release proof. Configure EVAOS_DESKTOP_BRIDGE_SOURCE_DIR or EVAOS_DESKTOP_BRIDGE_SOURCE_TOKEN for a real release build." >&2
exit 78
`
)
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
[warning] 399-399: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.writeFileSync(path.join(bridgeResourceDir, 'manifest.json'), ${JSON.stringify(manifest, null, 2)}\n)
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
[error] 436-436: An archive entry path (e.g. entry.path / entry.fileName / header.name) is joined to an output directory without validating that the resolved path stays inside that directory. A malicious archive can use "../" sequences to escape the extraction directory and overwrite arbitrary files (Zip Slip). Resolve the path and verify it starts with the normalized output directory, or strip traversal with path.basename, before writing the entry.
Context: path.join(dir, entry.name)
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(zip-slip-archive-extraction-javascript)
[warning] 4-4: Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: require('child_process')
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process)
🔇 Additional comments (9)
.gitignore (1)
207-207: LGTM!packages/desktop/electron-builder.yml (1)
136-138: LGTM!scripts/build-with-builder.js (1)
126-154: LGTM!Also applies to: 274-278, 572-605
.github/workflows/_build-reusable.yml (1)
452-452: LGTM!scripts/afterPack.js (1)
11-20: LGTM!Also applies to: 45-57, 136-224, 237-270, 388-388, 462-462
.github/workflows/pr-checks.yml (1)
655-657: LGTM!Also applies to: 683-684, 693-702, 738-741
.github/workflows/workbench-functional-smoke.yml (1)
19-23: LGTM!Also applies to: 38-38, 115-143, 176-186, 204-204
tests/unit/bootstrap/afterPackPackagingProfile.test.ts (1)
7-21: LGTM!Also applies to: 25-37, 56-73, 83-148, 158-165, 208-312
tests/unit/bootstrap/buildWithBuilder.test.ts (1)
110-118: LGTM!Also applies to: 132-142, 187-190
697783e to
a0fcc3c
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
scripts/prepareEvaosDesktopBridgeResource.js (2)
55-60: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winRequire immutable commit refs for proof lanes.
isMutableBridgeSourceRef()still accepts mutable branch-like refs such asrefs/heads/main,origin/main, ordevelop, so functional-smoke/release proof can resolve a moving target instead of the pinned Bridge commit.Proposed fix
+const COMMIT_SHA_RE = /^[0-9a-f]{40}$/i; + /** Returns true for source refs that are obviously mutable and invalid for proof/release lanes. */ function isMutableBridgeSourceRef(ref) { const normalized = String(ref || '') .trim() .toLowerCase(); - return !normalized || normalized === 'main' || normalized === 'master' || normalized === 'head'; + return !COMMIT_SHA_RE.test(normalized); }🤖 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 `@scripts/prepareEvaosDesktopBridgeResource.js` around lines 55 - 60, The ref check in isMutableBridgeSourceRef() is too narrow and still lets branch-like sources such as refs/heads/main, origin/main, and develop pass through. Update the normalization and validation logic in isMutableBridgeSourceRef() so proof/release lanes reject any mutable branch/reference form and only allow pinned commit-style refs, using this helper where the Bridge source ref is validated in prepareEvaosDesktopBridgeResource.js.
167-170: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winReset cached Git remotes to a credential-free URL.
resetBridgeSourceOrigin(repo)writes the rawrepovalue back into.git/config; ifEVAOS_DESKTOP_BRIDGE_SOURCE_REPOincludes credentials, the cache still persists the token.Proposed fix
function resetBridgeSourceOrigin(repo) { if (!fs.existsSync(path.join(bridgeSourceCacheDir, '.git'))) return; - runGit(['remote', 'set-url', 'origin', repo], bridgeSourceCacheDir, repo); + runGit(['remote', 'set-url', 'origin', sanitizeRepoForLog(repo)], bridgeSourceCacheDir, repo); }🤖 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 `@scripts/prepareEvaosDesktopBridgeResource.js` around lines 167 - 170, The resetBridgeSourceOrigin(repo) helper is persisting whatever repo URL it receives back into the cached .git config, which can retain credentials. Update resetBridgeSourceOrigin and its callers in prepareEvaosDesktopBridgeResource.js so the origin URL is sanitized to a credential-free form before passing it to runGit(..., ['remote', 'set-url', 'origin', ...]). Ensure the cache always stores a clean repository URL even when EVAOS_DESKTOP_BRIDGE_SOURCE_REPO contains embedded auth.
🤖 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 `@scripts/afterPack.js`:
- Around line 262-265: The native Bridge validation in shouldRequireRealBridge()
is only checking peekaboo and evaos-connector-helper, so functional-smoke can
still pass with non-native helper binaries when the manifest is not a
placeholder. Update the afterPack.js flow so the functional-smoke path also
enforces the same target-arch Mach-O checks for the Bridge-related binaries,
using requireMachOExecutableForArch alongside the existing peekaboo and helper
validations.
---
Duplicate comments:
In `@scripts/prepareEvaosDesktopBridgeResource.js`:
- Around line 55-60: The ref check in isMutableBridgeSourceRef() is too narrow
and still lets branch-like sources such as refs/heads/main, origin/main, and
develop pass through. Update the normalization and validation logic in
isMutableBridgeSourceRef() so proof/release lanes reject any mutable
branch/reference form and only allow pinned commit-style refs, using this helper
where the Bridge source ref is validated in
prepareEvaosDesktopBridgeResource.js.
- Around line 167-170: The resetBridgeSourceOrigin(repo) helper is persisting
whatever repo URL it receives back into the cached .git config, which can retain
credentials. Update resetBridgeSourceOrigin and its callers in
prepareEvaosDesktopBridgeResource.js so the origin URL is sanitized to a
credential-free form before passing it to runGit(..., ['remote', 'set-url',
'origin', ...]). Ensure the cache always stores a clean repository URL even when
EVAOS_DESKTOP_BRIDGE_SOURCE_REPO contains embedded auth.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 6ef5db42-7896-4f92-8acd-d8215bf9bd26
📒 Files selected for processing (11)
.github/workflows/_build-reusable.yml.github/workflows/pr-checks.yml.github/workflows/workbench-functional-smoke.yml.gitignorepackages/desktop/electron-builder.ymlscripts/afterPack.jsscripts/build-with-builder.jsscripts/prepareEvaosDesktopBridgeResource.jstests/unit/bootstrap/afterPackPackagingProfile.test.tstests/unit/bootstrap/buildWithBuilder.test.tstests/unit/bootstrap/prepareEvaosDesktopBridgeResource.test.ts
📜 Review details
⚠️ CI failures not shown inline (9)
GitHub Actions: Workbench Functional Smoke / 0_macOS arm64 unpacked app.txt: Workbench Functional Smoke
Conclusion: failure
##[group]Run set -euo pipefail
�[36;1mset -euo pipefail�[0m
�[36;1mif [ -z "${BRIDGE_SOURCE_***REDACTED*** ]; then�[0m
�[36;1m echo "::error::A private bridge source token is required to fetch electricsheephq/evaos-desktop-bridge for functional smoke."�[0m
GitHub Actions: Workbench Functional Smoke / macOS arm64 unpacked app: Workbench Functional Smoke
Conclusion: failure
##[group]Run set -euo pipefail
�[36;1mset -euo pipefail�[0m
�[36;1mAPP_PATH="$(find out -type d -name '*.app' -print -quit)"�[0m
�[36;1mif [ -z "$APP_PATH" ]; then�[0m
�[36;1m echo "::error::No .app produced under out/"�[0m
GitHub Actions: Workbench Functional Smoke / macOS arm64 unpacked app: Workbench Functional Smoke
Conclusion: failure
##[group]Run set -euo pipefail
�[36;1mset -euo pipefail�[0m
�[36;1mif [ -z "${BRIDGE_SOURCE_***REDACTED*** ]; then�[0m
�[36;1m echo "::error::A private bridge source token is required to fetch electricsheephq/evaos-desktop-bridge for functional smoke."�[0m
GitHub Actions: PR Checks / Thin App Smoke (macos-arm64): Restore Workbench Bridge resource proof
Conclusion: failure
##[group]Run set -euo pipefail
�[36;1mset -euo pipefail�[0m
�[36;1mecho "=========================================="�[0m
�[36;1mecho "VERIFY UNPACKED APP RESOURCE SHAPE"�[0m
�[36;1mecho "=========================================="�[0m
�[36;1mAPP_PATH="$(find out -type d -name '*.app' -print -quit)"�[0m
�[36;1mif [ -z "$APP_PATH" ]; then�[0m
�[36;1m echo "::error::No .app produced under out/"�[0m
GitHub Actions: PR Checks / 0_Thin App Smoke (macos-arm64).txt: Restore Workbench Bridge resource proof
Conclusion: failure
##[group]Run set -euo pipefail
�[36;1mset -euo pipefail�[0m
�[36;1mecho "=========================================="�[0m
�[36;1mecho "VERIFY UNPACKED APP RESOURCE SHAPE"�[0m
�[36;1mecho "=========================================="�[0m
�[36;1mAPP_PATH="$(find out -type d -name '*.app' -print -quit)"�[0m
�[36;1mif [ -z "$APP_PATH" ]; then�[0m
�[36;1m echo "::error::No .app produced under out/"�[0m
GitHub Actions: PR Checks / Unit Tests (macos-14): Restore Workbench Bridge resource proof
Conclusion: failure
mms�[22m�[39m
�[90mstdout�[2m | tests/unit/feedback/MessageToolGroupFeedback.dom.test.tsx
�[22m�[39mbuild.buildStorage global
build.buildStorage system
�[32m✓�[39m �[30m�[45m dom �[49m�[39m tests/unit/feedback/MessageToolGroupFeedback.dom.test.tsx �[2m(�[22m�[2m4 tests�[22m�[2m)�[22m�[32m 142�[2mms�[22m�[39m
�[32m✓�[39m �[30m�[45m dom �[49m�[39m tests/unit/settings/SystemSettings.dom.test.tsx �[2m(�[22m�[2m5 tests�[22m�[2m)�[22m�[32m 22�[2mms�[22m�[39m
�[32m✓�[39m �[30m�[45m dom �[49m�[39m tests/unit/feedback/FeedbackButton.dom.test.tsx �[2m(�[22m�[2m5 tests�[22m�[2m)�[22m�[32m 200�[2mms�[22m�[39m
�[32m✓�[39m �[30m�[45m dom �[49m�[39m tests/unit/feedback/McpServerHeaderFeedback.dom.test.tsx �[2m(�[22m�[2m4 tests�[22m�[2m)�[22m�[32m 137�[2mms�[22m�[39m
�[32m✓�[39m �[30m�[45m dom �[49m�[39m tests/unit/previews/MarkdownViewer.dom.test.tsx �[2m(�[22m�[2m3 tests�[22m�[2m)�[22m�[32m 52�[2mms�[22m�[39m
�[32m✓�[39m �[30m�[45m dom �[49m�[39m tests/unit/renderer/messageToolGroupSummary.dom.test.tsx �[2m(�[22m�[2m1 test�[22m�[2m)�[22m�[32m 42�[2mms�[22m�[39m
�[90mstdout�[2m | tests/unit/feedback/MessageAgentStatusFeedback.dom.test.tsx
�[22m�[39mbuild.buildStorage global
build.buildStorage system
�[90mstdout�[2m | tests/unit/feedback/MessageAgentStatusFeedback.dom.test.tsx�[2m > �[22m�[2mMessageAgentStatus — FeedbackButton wiring�[2m > �[22m�[2mdoes not render FeedbackButton on successful statuses
�[22m�[39m[httpBridge] GET /api/agents (no body)
[httpBridge] GET /api/assistants (no body)
�[90mstderr�[2m | tests/unit/feedback/MessageAgentStatusFeedback.dom.test.tsx�[2m > �[22m�[2mMessageAgentStatus — FeedbackButton wiring�[2m > �[22m�[2mdoes not render FeedbackButton on successful statuses
�[22m�[39mFailed to load assistants for conversation selector: TypeError: Failed to parse URL from /api/assistants
�[90m at node:internal/deps/undici/undici:14976:13�[39m
�[90m at processTicksAndRejections (node:internal/process/task_queues:103:5)�[39m
at...
GitHub Actions: PR Checks / 4_Coverage Test.txt: Restore Workbench Bridge resource proof
Conclusion: failure
load.ts | 0 | 0 | 0 | 0 | 20-113
...leSelector.ts | 0 | 0 | 0 | 0 | 21-48
...steService.ts | 0 | 0 | 0 | 0 | 23-92
...ewLauncher.ts | 5.08 | 0 | 0 | 5.17 | 25-29,71-204
...ploadState.ts | 74.71 | 67.39 | 45 | 76.92 | ...49-250,255-276
...ceSelector.ts | 0 | 0 | 0 | 0 | 21-59
...erer/hooks/mcp | 0 | 0 | 0 | 0 |
catalog.ts | 0 | 0 | 0 | 0 | 16-117
index.ts | 0 | 0 | 0 | 0 |
messageQueue.ts | 0 | 0 | 0 | 0 | 15-77
...Connection.ts | 0 | 0 | 0 | 0 | 14-266
useMcpModal.ts | 0 | 100 | 0 | 0 | 8-50
useMcpOAuth.ts | 0 | 0 | 0 | 0 | 16-181
...ServerCRUD.ts | 0 | 0 | 0 | 0 | 9-148
useMcpServers.ts | 0 | 0 | 0 | 0 | 11-76
...r/hooks/system | 7.95 | 3.64 | 3.65 | 8.01 |
...lesEnabled.ts | 44.44 | 0 | 0 | 50 | 7,10,16-17
useDeepLink.ts | 0 | 0 | 0 | 0 | 27-91
useExtI18n.ts | 0 | 0 | 0 | 0 | 18-67
...ttingsTabs.ts | 12.12 | 0 | 0 | 13.33 | 21-78
...ationClick.ts | 0 | 0 | 0 | 0 | 15-34
...lDetection.ts | 0 | 0 | 0 | 0 | 59-186
usePwaMode.ts | 0 | 0 | 0 | 0 | 15-30
...peechInput.ts | 0 | 0 | 0 | 0 | 40-423
useTheme.ts | 58.69 | 70 | 33.33 | 57.77 | 53-95
...derer/hooks/ui | 12.11 | 3.24 | 4.28 | 12.1 |
...olorScheme.ts | 57.77 | 62.5 | 33.33 | 56.81 | 65-116
...nShortcuts.ts | 0 | 0 | 0 | 0 | 11-74
useDebounce.ts | 0 | 0 | 0 | 0 | 11-38
...
GitHub Actions: PR Checks / 6_Unit Tests (macos-14).txt: Restore Workbench Bridge resource proof
Conclusion: failure
p/src/common/adapter/httpBridge.ts:191:26�[90m)�[39m
at Object.invoke �[90m(/Users/runner/work/evaOS-GUI/evaOS-GUI/�[39mpackages/desktop/src/common/adapter/httpBridge.ts:257:14�[90m)�[39m
at �[90m/Users/runner/work/evaOS-GUI/evaOS-GUI/�[39mpackages/desktop/src/renderer/pages/conversation/hooks/useConversationAgents.ts:42:52
at fetcher �[90m(file:///Users/runner/work/evaOS-GUI/evaOS-GUI/�[39mnode_modules/�[4m.bun�[24m/swr@2.4.1+b1ab299f0a400331/node_modules/�[4mswr�[24m/dist/_internal/index.mjs:64:42�[90m)�[39m {
code: �[32m'ERR_INVALID_URL'�[39m,
input: �[32m'/api/assistants'�[39m
}
}
An update to MessageAgentStatus inside a test was not wrapped in act(...).
When testing, code that causes React state updates should be wrapped into act(...):
act(() => {
/* fire events that update state */
});
/* assert on the output */
This ensures that you're testing the behavior the user would see in the browser. Learn more at https://react.dev/link/wrap-tests-with-act
An update to MessageAgentStatus inside a test was not wrapped in act(...).
When testing, code that causes React state updates should be wrapped into act(...):
act(() => {
/* fire events that update state */
});
/* assert on the output */
This ensures that you're testing the behavior the user would see in the browser. Learn more at https://react.dev/link/wrap-tests-with-act
�[32m✓�[39m �[30m�[45m dom �[49m�[39m tests/unit/feedback/MessageAgentStatusFeedback.dom.test.tsx �[2m(�[22m�[2m3 tests�[22m�[2m)�[22m�[32m 160�[2mms�[22m�[39m
�[32m✓�[39m �[30m�[45m dom �[49m�[39m tests/unit/previews/ExcelViewer.dom.test.tsx �[2m(�[22m�[2m3 tests�[22m�[2m)�[22m�[32m 19�[2mms�[22m�[39m
�[32m✓�[39m �[30m�[45m dom �[49m�[39m tests/unit/cron/CronStatusTag.dom.test.tsx �[2m(�[22m�[2m6 tests�[22m�[2m)�[22m�[32m 32�[2mms�[22m�[39m
�[32m✓�[39m �[30m�[45m dom �[49m�[39m tests/unit/renderer/useSiderTeamBadges.dom.test.ts �[2m(�[22m�[2m1 test�[22m�[2m)�[22m�[32m 15�[2mms�[22m�[3...
GitHub Actions: PR Checks / Coverage Test: Restore Workbench Bridge resource proof
Conclusion: failure
m
at �[90m/home/runner/work/evaOS-GUI/evaOS-GUI/�[39mtests/unit/assets/presetAssistantResources.test.ts:44:21
at �[90mfile:///home/runner/work/evaOS-GUI/evaOS-GUI/�[39mnode_modules/�[4m.bun�[24m/@vitest+runner@4.1.0/node_modules/�[4m@vitest/runner�[24m/dist/index.js:1258:20 {
[cause]: Error: connect ECONNREFUSED 127.0.0.1:13400
�[90m at TCPConnectWrap.afterConnect [as oncomplete] (node:net:1638:16)�[39m {
errno: �[33m-111�[39m,
code: �[32m'ECONNREFUSED'�[39m,
syscall: �[32m'connect'�[39m,
address: �[32m'127.0.0.1'�[39m,
port: �[33m13400�[39m
}
}
�[32m✓�[39m �[30m�[43m node �[49m�[39m tests/unit/assets/presetAssistantResources.test.ts �[2m(�[22m�[2m6 tests�[22m�[2m)�[22m�[32m 60�[2mms�[22m�[39m
�[32m✓�[39m �[30m�[43m node �[49m�[39m tests/unit/renderer/warmupConversation.test.ts �[2m(�[22m�[2m2 tests�[22m�[2m)�[22m�[32m 11�[2mms�[22m�[39m
�[32m✓�[39m �[30m�[43m node �[49m�[39m tests/unit/common/OpenAI2GeminiConverter.test.ts �[2m(�[22m�[2m2 tests�[22m�[2m)�[22m�[32m 8�[2mms�[22m�[39m
�[32m✓�[39m �[30m�[43m node �[49m�[39m tests/unit/common/platformConstants.test.ts �[2m(�[22m�[2m6 tests�[22m�[2m)�[22m�[32m 5�[2mms�[22m�[39m
�[32m✓�[39m �[30m�[43m node �[49m�[39m tests/unit/common-adapter/workspaceMapper.test.ts �[2m(�[22m�[2m2 tests�[22m�[2m)�[22m�[32m 5�[2mms�[22m�[39m
�[32m✓�[39m �[30m�[43m node �[49m�[39m tests/unit/extension/extensionMapperIntegration.test.ts �[2m(�[22m�[2m3 tests�[22m�[2m)�[22m�[32m 8�[2mms�[22m�[39m
�[32m✓�[39m �[30m�[43m node �[49m�[39m tests/unit/common/normalizeToolCall.test.ts �[2m(�[22m�[2m1 test�[22m�[2m)�[22m�[32m 8�[2mms�[22m�[39m
�[32m✓�[39m �[30m�[43m node �[49m�[39m tests/unit/cron/cronUtils.test.ts �[2m(�[22m�[2m2 tests�[22m�[2m)�[22m�[32m 11�[2mms�[22m�[39m
�[32m✓�[39m �[30m�[43m node �[49m�[39m tests/unit/chat/atFileQuery.test.ts �[2m(�[22m�[2m3 tests�[22m�[2m)�[22m�[32m 5�[2mms�[22m�[39m
�[90mstdout�[2m | tests/unit/chat/toolCallLogging.test.ts
�[22m�[39mbuild...
🧰 Additional context used
📓 Path-based instructions (4)
**/*[!.module].ts
📄 CodeRabbit inference engine (AGENTS.md)
Utility files must use camelCase naming (e.g., formatDate.ts)
Files:
tests/unit/bootstrap/buildWithBuilder.test.tstests/unit/bootstrap/afterPackPackagingProfile.test.tstests/unit/bootstrap/prepareEvaosDesktopBridgeResource.test.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Unused function parameters must be prefixed with underscore (_)
Use single-element arrays inline if they fit on one line; require trailing commas in multi-line arrays/objects; use single quotes for strings
Use English for code comments; provide JSDoc for public functions
Files:
tests/unit/bootstrap/buildWithBuilder.test.tsscripts/afterPack.jstests/unit/bootstrap/afterPackPackagingProfile.test.tsscripts/prepareEvaosDesktopBridgeResource.jstests/unit/bootstrap/prepareEvaosDesktopBridgeResource.test.tsscripts/build-with-builder.js
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use@icon-park/reactfor icons
TypeScript strict mode must be enabled; no 'any' types, no implicit returns
Use path aliases:@/,@process/,@renderer/* for imports
Prefer 'type' over 'interface' for type definitions (per Oxlint config)
Files:
tests/unit/bootstrap/buildWithBuilder.test.tstests/unit/bootstrap/afterPackPackagingProfile.test.tstests/unit/bootstrap/prepareEvaosDesktopBridgeResource.test.ts
**/*.{test,spec}.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Vitest 4 for testing with a coverage target of at least 80%
Files:
tests/unit/bootstrap/buildWithBuilder.test.tstests/unit/bootstrap/afterPackPackagingProfile.test.tstests/unit/bootstrap/prepareEvaosDesktopBridgeResource.test.ts
🪛 ast-grep (0.44.0)
scripts/afterPack.js
[warning] 136-136: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.readFileSync(filePath, 'utf8')
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
[warning] 167-167: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.readFileSync(filePath, { encoding: null, flag: 'r' })
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
[warning] 175-175: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.readFileSync(filePath, { encoding: null, flag: 'r' })
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
scripts/prepareEvaosDesktopBridgeResource.js
[warning] 4-4: Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: require('child_process')
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process)
[warning] 243-243: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.readFileSync(filePath, { encoding: null, flag: 'r' })
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
[warning] 259-278: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.writeFileSync(
helperPath,
`#!/bin/sh
set -eu
HELPER_DIR="$(CDPATH= cd -- "$(dirname -- "$0")" && pwd)"
if [ -x "$HELPER_DIR/peekaboo" ]; then
exec "$HELPER_DIR/peekaboo" "$@"
fi
for candidate in /opt/homebrew/bin/peekaboo /usr/local/bin/peekaboo; do
if [ -x "$candidate" ]; then
exec "$candidate" "$@"
fi
done
echo "evaos-connector-helper: bundled peekaboo was not found. Rebuild evaOS Workbench with Bridge/bin/peekaboo or install the Workbench connector package." >&2
exit 127
`
)
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
[warning] 285-299: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.writeFileSync(
peekabooPath,
`#!/bin/sh
set -eu
for candidate in /opt/homebrew/bin/peekaboo /usr/local/bin/peekaboo; do
if [ -x "$candidate" ]; then
exec "$candidate" "$@"
fi
done
echo "peekaboo: bundled helper binary was not found. Rebuild evaOS Workbench with Bridge/bin/peekaboo or install the Workbench connector package." >&2
exit 127
`
)
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
[warning] 360-360: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.writeFileSync(wrapperPath, bridgeWrapperScript())
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
[warning] 380-394: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.writeFileSync(
wrapperPath,
`#!/bin/sh
set -eu
if [ "${1:-}" = "--version" ] || [ "${1:-}" = "version" ]; then
echo "evaos-desktop-bridge diagnostic-placeholder"
exit 0
fi
echo "evaos-desktop-bridge diagnostic placeholder: ${escapeForShellDoubleQuotes(reason)}" >&2
echo "This PR/build artifact is not valid for Mac pairing release proof. Configure EVAOS_DESKTOP_BRIDGE_SOURCE_DIR or EVAOS_DESKTOP_BRIDGE_SOURCE_TOKEN for a real release build." >&2
exit 78
`
)
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
[warning] 399-399: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.writeFileSync(path.join(bridgeResourceDir, 'manifest.json'), ${JSON.stringify(manifest, null, 2)}\n)
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
[error] 436-436: An archive entry path (e.g. entry.path / entry.fileName / header.name) is joined to an output directory without validating that the resolved path stays inside that directory. A malicious archive can use "../" sequences to escape the extraction directory and overwrite arbitrary files (Zip Slip). Resolve the path and verify it starts with the normalized output directory, or strip traversal with path.basename, before writing the entry.
Context: path.join(dir, entry.name)
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(zip-slip-archive-extraction-javascript)
tests/unit/bootstrap/prepareEvaosDesktopBridgeResource.test.ts
[warning] 6-6: Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import { spawnSync } from 'node:child_process';
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process-typescript)
🪛 GitHub Actions: PR Checks / Unit Tests (macos-14)
tests/unit/bootstrap/afterPackPackagingProfile.test.ts
[warning] 1-1: Packaging profile guard logged warning:
🔇 Additional comments (13)
tests/unit/bootstrap/prepareEvaosDesktopBridgeResource.test.ts (2)
167-179: Expand the mutable-ref coverage.This still only exercises exact
'main', so alias forms like'refs/heads/main'and'origin/main'can regress unnoticed while proof lanes are still expected to reject mutable refs.
14-165: LGTM!Also applies to: 181-251
tests/unit/bootstrap/afterPackPackagingProfile.test.ts (1)
22-37: LGTM!Also applies to: 56-73, 83-148, 158-165, 208-331
tests/unit/bootstrap/buildWithBuilder.test.ts (1)
110-118: LGTM!Also applies to: 133-142, 187-190
.gitignore (1)
207-207: LGTM!packages/desktop/electron-builder.yml (1)
136-138: LGTM!scripts/build-with-builder.js (1)
126-154: LGTM!Also applies to: 274-279, 572-572, 599-605
.github/workflows/_build-reusable.yml (1)
452-452: LGTM!scripts/prepareEvaosDesktopBridgeResource.js (1)
1-54: LGTM!Also applies to: 63-166, 172-514
.github/workflows/pr-checks.yml (2)
728-728: Previously flagged: assert the PR smoke bundles the diagnostic placeholder.Line 728 still only reports
manifest.placeholder; it does not fail when the manifest is a real Bridge bundle, so this lane can stop proving the secret-free placeholder contract.Suggested fix
- BRIDGE_SUMMARY="$(node -e "const fs=require('fs'); const m=JSON.parse(fs.readFileSync(process.argv[1], 'utf8')); console.log(m.placeholder ? 'diagnostic-placeholder' : (m.sourceCommit || m.commit || m.version || 'unknown'))" "$BRIDGE_MANIFEST")" + BRIDGE_SUMMARY="$(node -e "const fs=require('fs'); const m=JSON.parse(fs.readFileSync(process.argv[1], 'utf8')); if (!m.placeholder) { console.error('::error::Expected thin-app-smoke to bundle the diagnostic Bridge placeholder'); process.exit(1); } console.log('diagnostic-placeholder')" "$BRIDGE_MANIFEST")"
655-657: LGTM!Also applies to: 683-702, 738-741
scripts/afterPack.js (1)
11-19: LGTM!Also applies to: 44-55, 135-222, 236-260, 267-268, 387-387, 461-461
.github/workflows/workbench-functional-smoke.yml (1)
19-23: LGTM!Also applies to: 38-38, 115-143, 176-186, 204-204
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
scripts/build-with-builder.js (1)
122-190: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winRemove empty
extraResourcesblocks after filtering.For the new mac block,
resources/Bridgeis the only child. This function emits theextraResources:header before filtering, so thin-shell config can be left with an emptymac.extraResources:value. Buffer the block header and only write it when at least one child resource remains.🤖 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 `@scripts/build-with-builder.js` around lines 122 - 190, The removeThinShellExtraResources function currently writes the extraResources header immediately, which can leave an empty mac.extraResources block after filtering out resources/Bridge. Update removeThinShellExtraResources to buffer the extraResources header and only emit it from the main loop when at least one child resource under that block remains, using the existing symbols removeThinShellExtraResources, extraResourcesMatch, and THIN_SHELL_REMOVED_RESOURCES to locate the 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/workbench-functional-smoke.yml:
- Around line 176-186: The smoke workflow validation is too permissive because
BRIDGE_VERSION can fall back to version and the only strict check compares
requestedSourceRef, which can let stale or malformed manifests pass. Update the
Bridge manifest checks to require a 40-character resolved commit from
sourceCommit or commit, and compare that resolved commit directly against
WORKBENCH_SMOKE_BRIDGE_REF in the same validation block. Keep the existing
placeholder handling, but remove reliance on version/requestedSourceRef for
proving the bundled Bridge revision.
In `@scripts/prepareEvaosDesktopBridgeResource.js`:
- Around line 140-155: The managed clone/fetch path in
prepareEvaosDesktopBridgeResource currently returns the cache after checkout
without the same pinned-source verification used by assertPinnedSourceDir().
After the runGit checkout in the fallback flow, add the final immutable HEAD
assertion before returning bridgeSourceCacheDir so the fetched source is
validated against the requested ref/commit just like the local candidate path.
Use the existing runGit, resetBridgeSourceOrigin, and assertPinnedSourceDir
symbols to place the check in the fallback branch.
In `@tests/unit/bootstrap/buildWithBuilder.test.ts`:
- Around line 117-118: The build matrix test is leaking the ambient
EVAOS_PACKAGING_PROFILE from process.env, which can change whether the macOS
path runs prepareEvaosDesktopBridgeResource.js. Update the test helper/callsite
used by runBuildWithHook in buildWithBuilder.test.ts to force a neutral
packaging profile before each case, and keep the assertion around
expectedBridgePrep tied to the controlled environment rather than inherited
state.
In `@tests/unit/bootstrap/prepareEvaosDesktopBridgeResource.test.ts`:
- Around line 113-115: The execute-bit checks in
prepareEvaosDesktopBridgeResource.test are POSIX-only, so guard the
statSync(...).mode & 0o111 assertions behind a process.platform !== 'win32'
condition. Keep the existing mode verification for evaos-desktop-bridge,
bin/peekaboo, and bin/evaos-connector-helper on non-Windows platforms, and use a
Windows-safe assertion path in the same test block.
---
Outside diff comments:
In `@scripts/build-with-builder.js`:
- Around line 122-190: The removeThinShellExtraResources function currently
writes the extraResources header immediately, which can leave an empty
mac.extraResources block after filtering out resources/Bridge. Update
removeThinShellExtraResources to buffer the extraResources header and only emit
it from the main loop when at least one child resource under that block remains,
using the existing symbols removeThinShellExtraResources, extraResourcesMatch,
and THIN_SHELL_REMOVED_RESOURCES to locate the 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: ASSERTIVE
Plan: Pro
Run ID: 768edd33-aa4e-41e2-b59d-7457613c6597
📒 Files selected for processing (11)
.github/workflows/_build-reusable.yml.github/workflows/pr-checks.yml.github/workflows/workbench-functional-smoke.yml.gitignorepackages/desktop/electron-builder.ymlscripts/afterPack.jsscripts/build-with-builder.jsscripts/prepareEvaosDesktopBridgeResource.jstests/unit/bootstrap/afterPackPackagingProfile.test.tstests/unit/bootstrap/buildWithBuilder.test.tstests/unit/bootstrap/prepareEvaosDesktopBridgeResource.test.ts
📜 Review details
⚠️ CI failures not shown inline (9)
GitHub Actions: Workbench Functional Smoke / macOS arm64 unpacked app: Workbench Functional Smoke
Conclusion: failure
##[group]Run set -euo pipefail
�[36;1mset -euo pipefail�[0m
�[36;1mif [ -z "${BRIDGE_SOURCE_***REDACTED*** ]; then�[0m
�[36;1m echo "::error::A private bridge source token is required to fetch electricsheephq/evaos-desktop-bridge for functional smoke."�[0m
GitHub Actions: Workbench Functional Smoke / 0_macOS arm64 unpacked app.txt: Workbench Functional Smoke
Conclusion: failure
##[group]Run set -euo pipefail
�[36;1mset -euo pipefail�[0m
�[36;1mif [ -z "${BRIDGE_SOURCE_***REDACTED*** ]; then�[0m
�[36;1m echo "::error::A private bridge source token is required to fetch electricsheephq/evaos-desktop-bridge for functional smoke."�[0m
GitHub Actions: Workbench Functional Smoke / macOS arm64 unpacked app: Workbench Functional Smoke
Conclusion: failure
##[group]Run set -euo pipefail
�[36;1mset -euo pipefail�[0m
�[36;1mAPP_PATH="$(find out -type d -name '*.app' -print -quit)"�[0m
�[36;1mif [ -z "$APP_PATH" ]; then�[0m
�[36;1m echo "::error::No .app produced under out/"�[0m
GitHub Actions: PR Checks / Thin App Smoke (macos-arm64): Restore Workbench Bridge resource proof
Conclusion: failure
##[group]Run set -euo pipefail
�[36;1mset -euo pipefail�[0m
�[36;1mecho "=========================================="�[0m
�[36;1mecho "VERIFY UNPACKED APP RESOURCE SHAPE"�[0m
�[36;1mecho "=========================================="�[0m
�[36;1mAPP_PATH="$(find out -type d -name '*.app' -print -quit)"�[0m
�[36;1mif [ -z "$APP_PATH" ]; then�[0m
�[36;1m echo "::error::No .app produced under out/"�[0m
GitHub Actions: PR Checks / Unit Tests (macos-14): Restore Workbench Bridge resource proof
Conclusion: failure
2m�[2m5 tests�[22m�[2m)�[22m�[32m 36�[2mms�[22m�[39m
�[32m✓�[39m �[30m�[45m dom �[49m�[39m tests/unit/feedback/FeedbackButton.dom.test.tsx �[2m(�[22m�[2m5 tests�[22m�[2m)�[22m�[32m 290�[2mms�[22m�[39m
�[32m✓�[39m �[30m�[45m dom �[49m�[39m tests/unit/feedback/McpServerHeaderFeedback.dom.test.tsx �[2m(�[22m�[2m4 tests�[22m�[2m)�[22m�[32m 274�[2mms�[22m�[39m
�[32m✓�[39m �[30m�[45m dom �[49m�[39m tests/unit/previews/MarkdownViewer.dom.test.tsx �[2m(�[22m�[2m3 tests�[22m�[2m)�[22m�[32m 83�[2mms�[22m�[39m
�[32m✓�[39m �[30m�[45m dom �[49m�[39m tests/unit/renderer/messageToolGroupSummary.dom.test.tsx �[2m(�[22m�[2m1 test�[22m�[2m)�[22m�[32m 51�[2mms�[22m�[39m
�[90mstdout�[2m | tests/unit/feedback/MessageAgentStatusFeedback.dom.test.tsx
�[22m�[39mbuild.buildStorage global
build.buildStorage system
�[90mstdout�[2m | tests/unit/feedback/MessageAgentStatusFeedback.dom.test.tsx�[2m > �[22m�[2mMessageAgentStatus — FeedbackButton wiring�[2m > �[22m�[2mdoes not render FeedbackButton on successful statuses
�[22m�[39m[httpBridge] GET /api/agents (no body)
[httpBridge] GET /api/assistants (no body)
�[90mstderr�[2m | tests/unit/feedback/MessageAgentStatusFeedback.dom.test.tsx�[2m > �[22m�[2mMessageAgentStatus — FeedbackButton wiring�[2m > �[22m�[2mdoes not render FeedbackButton on successful statuses
�[22m�[39mFailed to load assistants for conversation selector: TypeError: Failed to parse URL from /api/assistants
�[90m at node:internal/deps/undici/undici:14976:13�[39m
�[90m at processTicksAndRejections (node:internal/process/task_queues:103:5)�[39m
at httpRequest �[90m(/Users/runner/work/evaOS-GUI/evaOS-GUI/�[39mpackages/desktop/src/common/adapter/httpBridge.ts:191:20�[90m)�[39m {
[cause]: TypeError: Invalid URL
�[90m at new URL (node:internal/url:818:25)�[39m
at new URL �[90m(file:///Users/runner/work/evaOS-GUI/evaOS-GUI/�[39mnode_modules/�[4m.bun�[24m/vitest@4.1.0+634674f7a4778bc9/node_modules/�[4mvitest�[24m/dist/chunks/index.EY6TCHpo....
GitHub Actions: PR Checks / 0_Thin App Smoke (macos-arm64).txt: Restore Workbench Bridge resource proof
Conclusion: failure
##[group]Run set -euo pipefail
�[36;1mset -euo pipefail�[0m
�[36;1mecho "=========================================="�[0m
�[36;1mecho "VERIFY UNPACKED APP RESOURCE SHAPE"�[0m
�[36;1mecho "=========================================="�[0m
�[36;1mAPP_PATH="$(find out -type d -name '*.app' -print -quit)"�[0m
�[36;1mif [ -z "$APP_PATH" ]; then�[0m
�[36;1m echo "::error::No .app produced under out/"�[0m
GitHub Actions: PR Checks / Coverage Test: Restore Workbench Bridge resource proof
Conclusion: failure
9m
at �[90m/home/runner/work/evaOS-GUI/evaOS-GUI/�[39mtests/unit/assets/presetAssistantResources.test.ts:44:21
at �[90mfile:///home/runner/work/evaOS-GUI/evaOS-GUI/�[39mnode_modules/�[4m.bun�[24m/@vitest+runner@4.1.0/node_modules/�[4m@vitest/runner�[24m/dist/index.js:1258:20 {
[cause]: Error: connect ECONNREFUSED 127.0.0.1:13400
�[90m at TCPConnectWrap.afterConnect [as oncomplete] (node:net:1638:16)�[39m {
errno: �[33m-111�[39m,
code: �[32m'ECONNREFUSED'�[39m,
syscall: �[32m'connect'�[39m,
address: �[32m'127.0.0.1'�[39m,
port: �[33m13400�[39m
}
}
�[32m✓�[39m �[30m�[43m node �[49m�[39m tests/unit/assets/presetAssistantResources.test.ts �[2m(�[22m�[2m6 tests�[22m�[2m)�[22m�[32m 58�[2mms�[22m�[39m
�[32m✓�[39m �[30m�[43m node �[49m�[39m tests/unit/common/OpenAI2GeminiConverter.test.ts �[2m(�[22m�[2m2 tests�[22m�[2m)�[22m�[32m 8�[2mms�[22m�[39m
�[32m✓�[39m �[30m�[43m node �[49m�[39m tests/unit/renderer/warmupConversation.test.ts �[2m(�[22m�[2m2 tests�[22m�[2m)�[22m�[32m 13�[2mms�[22m�[39m
�[32m✓�[39m �[30m�[43m node �[49m�[39m tests/unit/extension/extensionMapperIntegration.test.ts �[2m(�[22m�[2m3 tests�[22m�[2m)�[22m�[32m 8�[2mms�[22m�[39m
�[32m✓�[39m �[30m�[43m node �[49m�[39m tests/unit/common/platformConstants.test.ts �[2m(�[22m�[2m6 tests�[22m�[2m)�[22m�[32m 6�[2mms�[22m�[39m
�[32m✓�[39m �[30m�[43m node �[49m�[39m tests/unit/common-adapter/workspaceMapper.test.ts �[2m(�[22m�[2m2 tests�[22m�[2m)�[22m�[32m 8�[2mms�[22m�[39m
�[32m✓�[39m �[30m�[43m node �[49m�[39m tests/unit/common/normalizeToolCall.test.ts �[2m(�[22m�[2m1 test�[22m�[2m)�[22m�[32m 5�[2mms�[22m�[39m
�[32m✓�[39m �[30m�[43m node �[49m�[39m tests/unit/cron/cronUtils.test.ts �[2m(�[22m�[2m2 tests�[22m�[2m)�[22m�[32m 9�[2mms�[22m�[39m
�[32m✓�[39m �[30m�[43m node �[49m�[39m tests/unit/chat/atFileQuery.test.ts �[2m(�[22m�[2m3 tests�[22m�[2m)�[22m�[32m 4�[2mms�[22m�[39m
�[90mstdout�[2m | tests/unit/chat/toolCallLogging.test.ts
�[22m�[39mbuild...
GitHub Actions: PR Checks / 6_Unit Tests (macos-14).txt: Restore Workbench Bridge resource proof
Conclusion: failure
9mnode_modules/�[4m.bun�[24m/swr@2.4.1+b1ab299f0a400331/node_modules/�[4mswr�[24m/dist/_internal/index.mjs:64:42�[90m)�[39m {
code: �[32m'ERR_INVALID_URL'�[39m,
input: �[32m'/api/assistants'�[39m
}
}
An update to MessageAgentStatus inside a test was not wrapped in act(...).
When testing, code that causes React state updates should be wrapped into act(...):
act(() => {
/* fire events that update state */
});
/* assert on the output */
This ensures that you're testing the behavior the user would see in the browser. Learn more at https://react.dev/link/wrap-tests-with-act
An update to MessageAgentStatus inside a test was not wrapped in act(...).
When testing, code that causes React state updates should be wrapped into act(...):
act(() => {
/* fire events that update state */
});
/* assert on the output */
This ensures that you're testing the behavior the user would see in the browser. Learn more at https://react.dev/link/wrap-tests-with-act
�[32m✓�[39m �[30m�[45m dom �[49m�[39m tests/unit/feedback/MessageAgentStatusFeedback.dom.test.tsx �[2m(�[22m�[2m3 tests�[22m�[2m)�[22m�[32m 224�[2mms�[22m�[39m
�[32m✓�[39m �[30m�[45m dom �[49m�[39m tests/unit/cron/CronStatusTag.dom.test.tsx �[2m(�[22m�[2m6 tests�[22m�[2m)�[22m�[32m 58�[2mms�[22m�[39m
�[32m✓�[39m �[30m�[45m dom �[49m�[39m tests/unit/previews/ExcelViewer.dom.test.tsx �[2m(�[22m�[2m3 tests�[22m�[2m)�[22m�[32m 39�[2mms�[22m�[39m
�[32m✓�[39m �[30m�[45m dom �[49m�[39m tests/unit/renderer/useSiderTeamBadges.dom.test.ts �[2m(�[22m�[2m1 test�[22m�[2m)�[22m�[32m 26�[2mms�[22m�[39m
�[32m✓�[39m �[30m�[45m dom �[49m�[39m tests/unit/renderer/useGuidInput.dom.test.ts �[2m(�[22m�[2m2 tests�[22m�[2m)�[22m�[32m 28�[2mms�[22m�[39m
�[90mstdout�[2m | tests/unit/previews/OfficeWatchViewer.dom.test.tsx�[2m > �[22m�[2mOfficeWatchViewer module shape�[2m > �[22m�[2mmodule loads and exposes a default export
�[22m�[39mbuild.buildStorage global
build.buildStorage system
�[32m✓�[39m �[30m�[45m do...
GitHub Actions: PR Checks / 3_Coverage Test.txt: Restore Workbench Bridge resource proof
Conclusion: failure
load.ts | 0 | 0 | 0 | 0 | 20-113
...leSelector.ts | 0 | 0 | 0 | 0 | 21-48
...steService.ts | 0 | 0 | 0 | 0 | 23-92
...ewLauncher.ts | 5.08 | 0 | 0 | 5.17 | 25-29,71-204
...ploadState.ts | 74.71 | 67.39 | 45 | 76.92 | ...49-250,255-276
...ceSelector.ts | 0 | 0 | 0 | 0 | 21-59
...erer/hooks/mcp | 0 | 0 | 0 | 0 |
catalog.ts | 0 | 0 | 0 | 0 | 16-117
index.ts | 0 | 0 | 0 | 0 |
messageQueue.ts | 0 | 0 | 0 | 0 | 15-77
...Connection.ts | 0 | 0 | 0 | 0 | 14-266
useMcpModal.ts | 0 | 100 | 0 | 0 | 8-50
useMcpOAuth.ts | 0 | 0 | 0 | 0 | 16-181
...ServerCRUD.ts | 0 | 0 | 0 | 0 | 9-148
useMcpServers.ts | 0 | 0 | 0 | 0 | 11-76
...r/hooks/system | 7.95 | 3.64 | 3.65 | 8.01 |
...lesEnabled.ts | 44.44 | 0 | 0 | 50 | 7,10,16-17
useDeepLink.ts | 0 | 0 | 0 | 0 | 27-91
useExtI18n.ts | 0 | 0 | 0 | 0 | 18-67
...ttingsTabs.ts | 12.12 | 0 | 0 | 13.33 | 21-78
...ationClick.ts | 0 | 0 | 0 | 0 | 15-34
...lDetection.ts | 0 | 0 | 0 | 0 | 59-186
usePwaMode.ts | 0 | 0 | 0 | 0 | 15-30
...peechInput.ts | 0 | 0 | 0 | 0 | 40-423
useTheme.ts | 58.69 | 70 | 33.33 | 57.77 | 53-95
...derer/hooks/ui | 12.11 | 3.24 | 4.28 | 12.1 |
...olorScheme.ts | 57.77 | 62.5 | 33.33 | 56.81 | 65-116
...nShortcuts.ts | 0 | 0 | 0 | 0 | 11-74
useDebounce.ts | 0 | 0 | 0 | 0 | 11-38
...
🧰 Additional context used
📓 Path-based instructions (4)
**/*[!.module].ts
📄 CodeRabbit inference engine (AGENTS.md)
Utility files must use camelCase naming (e.g., formatDate.ts)
Files:
tests/unit/bootstrap/buildWithBuilder.test.tstests/unit/bootstrap/afterPackPackagingProfile.test.tstests/unit/bootstrap/prepareEvaosDesktopBridgeResource.test.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Unused function parameters must be prefixed with underscore (_)
Use single-element arrays inline if they fit on one line; require trailing commas in multi-line arrays/objects; use single quotes for strings
Use English for code comments; provide JSDoc for public functions
Files:
tests/unit/bootstrap/buildWithBuilder.test.tsscripts/afterPack.jstests/unit/bootstrap/afterPackPackagingProfile.test.tsscripts/build-with-builder.jstests/unit/bootstrap/prepareEvaosDesktopBridgeResource.test.tsscripts/prepareEvaosDesktopBridgeResource.js
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use@icon-park/reactfor icons
TypeScript strict mode must be enabled; no 'any' types, no implicit returns
Use path aliases:@/,@process/,@renderer/* for imports
Prefer 'type' over 'interface' for type definitions (per Oxlint config)
Files:
tests/unit/bootstrap/buildWithBuilder.test.tstests/unit/bootstrap/afterPackPackagingProfile.test.tstests/unit/bootstrap/prepareEvaosDesktopBridgeResource.test.ts
**/*.{test,spec}.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Vitest 4 for testing with a coverage target of at least 80%
Files:
tests/unit/bootstrap/buildWithBuilder.test.tstests/unit/bootstrap/afterPackPackagingProfile.test.tstests/unit/bootstrap/prepareEvaosDesktopBridgeResource.test.ts
🪛 ast-grep (0.44.0)
scripts/afterPack.js
[warning] 136-136: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.readFileSync(filePath, 'utf8')
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
[warning] 167-167: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.readFileSync(filePath, { encoding: null, flag: 'r' })
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
[warning] 175-175: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.readFileSync(filePath, { encoding: null, flag: 'r' })
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
tests/unit/bootstrap/prepareEvaosDesktopBridgeResource.test.ts
[warning] 6-6: Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import { spawnSync } from 'node:child_process';
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process-typescript)
scripts/prepareEvaosDesktopBridgeResource.js
[warning] 243-243: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.readFileSync(filePath, { encoding: null, flag: 'r' })
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
[warning] 259-278: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.writeFileSync(
helperPath,
`#!/bin/sh
set -eu
HELPER_DIR="$(CDPATH= cd -- "$(dirname -- "$0")" && pwd)"
if [ -x "$HELPER_DIR/peekaboo" ]; then
exec "$HELPER_DIR/peekaboo" "$@"
fi
for candidate in /opt/homebrew/bin/peekaboo /usr/local/bin/peekaboo; do
if [ -x "$candidate" ]; then
exec "$candidate" "$@"
fi
done
echo "evaos-connector-helper: bundled peekaboo was not found. Rebuild evaOS Workbench with Bridge/bin/peekaboo or install the Workbench connector package." >&2
exit 127
`
)
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
[warning] 285-299: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.writeFileSync(
peekabooPath,
`#!/bin/sh
set -eu
for candidate in /opt/homebrew/bin/peekaboo /usr/local/bin/peekaboo; do
if [ -x "$candidate" ]; then
exec "$candidate" "$@"
fi
done
echo "peekaboo: bundled helper binary was not found. Rebuild evaOS Workbench with Bridge/bin/peekaboo or install the Workbench connector package." >&2
exit 127
`
)
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
[warning] 360-360: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.writeFileSync(wrapperPath, bridgeWrapperScript())
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
[warning] 380-394: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.writeFileSync(
wrapperPath,
`#!/bin/sh
set -eu
if [ "${1:-}" = "--version" ] || [ "${1:-}" = "version" ]; then
echo "evaos-desktop-bridge diagnostic-placeholder"
exit 0
fi
echo "evaos-desktop-bridge diagnostic placeholder: ${escapeForShellDoubleQuotes(reason)}" >&2
echo "This PR/build artifact is not valid for Mac pairing release proof. Configure EVAOS_DESKTOP_BRIDGE_SOURCE_DIR or EVAOS_DESKTOP_BRIDGE_SOURCE_TOKEN for a real release build." >&2
exit 78
`
)
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
[warning] 399-399: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.writeFileSync(path.join(bridgeResourceDir, 'manifest.json'), ${JSON.stringify(manifest, null, 2)}\n)
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
[error] 436-436: An archive entry path (e.g. entry.path / entry.fileName / header.name) is joined to an output directory without validating that the resolved path stays inside that directory. A malicious archive can use "../" sequences to escape the extraction directory and overwrite arbitrary files (Zip Slip). Resolve the path and verify it starts with the normalized output directory, or strip traversal with path.basename, before writing the entry.
Context: path.join(dir, entry.name)
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(zip-slip-archive-extraction-javascript)
[warning] 4-4: Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: require('child_process')
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process)
🪛 GitHub Actions: PR Checks / 6_Unit Tests (macos-14).txt
tests/unit/bootstrap/afterPackPackagingProfile.test.ts
[warning] 1-1: afterPack packaging profile guard: app.asar.unpacked not found (logged as warning '
🪛 GitHub Actions: PR Checks / Unit Tests (macos-14)
tests/unit/bootstrap/afterPackPackagingProfile.test.ts
[warning] 1-1: Packaging guard warning: 'app.asar.unpacked not found' (printed multiple times during profile guard tests).
🔇 Additional comments (15)
scripts/afterPack.js (6)
262-265:functional-smokestill skips native Bridge arch checks.Line 256 rejects placeholder manifests for
functional-smoke, but the Mach-O/arch enforcement on Line 262 only runs whenshouldRequireRealBridge()is true. A non-placeholder manifest paired with wrong-arch or non-nativepeekaboo/evaos-connector-helperbinaries still passes the functional-smoke proof. Gate the arch checks onshouldRejectPlaceholderBridge(packagingProfile)(or add thefunctional-smokecondition) so the proof lane enforces native helpers.Suggested fix
- if (shouldRequireRealBridge()) { + if (shouldRejectPlaceholderBridge(packagingProfile)) { requireMachOExecutableForArch(path.join(resourcesDir, peekabooRelativePath), peekabooRelativePath, targetArch); requireMachOExecutableForArch(path.join(resourcesDir, helperRelativePath), helperRelativePath, targetArch); }
11-19: LGTM!
44-56: LGTM!
135-223: LGTM!
387-387: LGTM!
461-461: LGTM!.github/workflows/pr-checks.yml (4)
728-728: Bridge manifest is reported but not enforced.Line 728 still only derives
BRIDGE_SUMMARY; it never assertsm.placeholder === true. This was previously flagged (marked addressed inac6554f) but the current code is back to report-only, so this PR lane can silently pass with a real Bridge bundle and stop proving the secret-free placeholder contract. Re-add them.placeholderassertion before computing the summary.
655-657: LGTM!
683-702: LGTM!
738-741: LGTM!.gitignore (1)
207-207: LGTM!packages/desktop/electron-builder.yml (1)
136-138: LGTM!scripts/build-with-builder.js (1)
274-278: LGTM!Also applies to: 572-572, 599-605
.github/workflows/_build-reusable.yml (1)
452-452: LGTM!.github/workflows/workbench-functional-smoke.yml (1)
19-23: LGTM!Also applies to: 38-38, 115-143, 204-204
a0fcc3c to
16d3632
Compare
16d3632 to
22bcede
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/workbench-functional-smoke.yml (1)
169-170: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winRequire the Bridge CLI to be a file before accepting the artifact.
test -x "$BRIDGE"can pass for an executable directory. Use an explicit file check so the functional-smoke proof validates the expected packaged CLI shape.Proposed fix
- test -x "$BRIDGE" + test -f "$BRIDGE" + test -x "$BRIDGE" test -f "$BRIDGE_MANIFEST"🤖 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/workbench-functional-smoke.yml around lines 169 - 170, The Bridge artifact validation in the functional-smoke workflow is too loose because test -x on the BRIDGE check can succeed for an executable directory instead of a packaged CLI file. Update the artifact verification logic in the workflow step that checks BRIDGE and BRIDGE_MANIFEST to require BRIDGE to be an explicit regular file before proceeding, keeping the same validation flow but tightening the check to match the expected CLI shape.
🤖 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/pr-checks.yml:
- Around line 693-697: The Bridge CLI existence check in the workflow is too
loose because the current `[ -x "$BRIDGE" ]` test can pass for directories on
macOS. Update the Bridge validation in the thin-smoke resource-shape step to
require both a regular file and executable status, using the existing `BRIDGE`
path check before the `find`/`exit 1` block so only a real bundled CLI file is
accepted.
In `@scripts/afterPack.js`:
- Around line 44-54: `requireExecutableResource` currently only checks
`fs.accessSync(..., X_OK)`, which can incorrectly accept executable directories.
Update this helper to also verify the resolved path is a file by using
`fs.statSync(...).isFile()` before the executable-bit check, and keep the
existing missing/resource error handling in place for `resourcesDir`,
`absolutePath`, and `missing`.
In `@scripts/prepareEvaosDesktopBridgeResource.js`:
- Around line 99-100: The EVAOS_DESKTOP_BRIDGE_DISABLE_DEFAULT_CANDIDATES check
in prepareEvaosDesktopBridgeResource is too strict because it only treats '1' as
enabled; update that guard to use the shared truthy(...) parser so values like
'true' are handled consistently with the other boolean env flags in this script.
Keep the logic in the same candidate-selection path and adjust the condition
around the existing return [] behavior.
---
Outside diff comments:
In @.github/workflows/workbench-functional-smoke.yml:
- Around line 169-170: The Bridge artifact validation in the functional-smoke
workflow is too loose because test -x on the BRIDGE check can succeed for an
executable directory instead of a packaged CLI file. Update the artifact
verification logic in the workflow step that checks BRIDGE and BRIDGE_MANIFEST
to require BRIDGE to be an explicit regular file before proceeding, keeping the
same validation flow but tightening the check to match the expected CLI shape.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 068d4775-7f3a-4809-a39b-04f9cac75726
📒 Files selected for processing (11)
.github/workflows/_build-reusable.yml.github/workflows/pr-checks.yml.github/workflows/workbench-functional-smoke.yml.gitignorepackages/desktop/electron-builder.ymlscripts/afterPack.jsscripts/build-with-builder.jsscripts/prepareEvaosDesktopBridgeResource.jstests/unit/bootstrap/afterPackPackagingProfile.test.tstests/unit/bootstrap/buildWithBuilder.test.tstests/unit/bootstrap/prepareEvaosDesktopBridgeResource.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (3)
- GitHub Check: macOS arm64 unpacked app
- GitHub Check: PR Check Plan
- GitHub Check: Coverage Test
🧰 Additional context used
📓 Path-based instructions (4)
**/*[!.module].ts
📄 CodeRabbit inference engine (AGENTS.md)
Utility files must use camelCase naming (e.g., formatDate.ts)
Files:
tests/unit/bootstrap/buildWithBuilder.test.tstests/unit/bootstrap/afterPackPackagingProfile.test.tstests/unit/bootstrap/prepareEvaosDesktopBridgeResource.test.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Unused function parameters must be prefixed with underscore (_)
Use single-element arrays inline if they fit on one line; require trailing commas in multi-line arrays/objects; use single quotes for strings
Use English for code comments; provide JSDoc for public functions
Files:
tests/unit/bootstrap/buildWithBuilder.test.tstests/unit/bootstrap/afterPackPackagingProfile.test.tsscripts/build-with-builder.jsscripts/afterPack.jsscripts/prepareEvaosDesktopBridgeResource.jstests/unit/bootstrap/prepareEvaosDesktopBridgeResource.test.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use@icon-park/reactfor icons
TypeScript strict mode must be enabled; no 'any' types, no implicit returns
Use path aliases:@/,@process/,@renderer/* for imports
Prefer 'type' over 'interface' for type definitions (per Oxlint config)
Files:
tests/unit/bootstrap/buildWithBuilder.test.tstests/unit/bootstrap/afterPackPackagingProfile.test.tstests/unit/bootstrap/prepareEvaosDesktopBridgeResource.test.ts
**/*.{test,spec}.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Vitest 4 for testing with a coverage target of at least 80%
Files:
tests/unit/bootstrap/buildWithBuilder.test.tstests/unit/bootstrap/afterPackPackagingProfile.test.tstests/unit/bootstrap/prepareEvaosDesktopBridgeResource.test.ts
🪛 ast-grep (0.44.0)
scripts/afterPack.js
[warning] 136-136: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.readFileSync(filePath, 'utf8')
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
[warning] 167-167: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.readFileSync(filePath, { encoding: null, flag: 'r' })
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
[warning] 175-175: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.readFileSync(filePath, { encoding: null, flag: 'r' })
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
scripts/prepareEvaosDesktopBridgeResource.js
[warning] 4-4: Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: require('child_process')
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process)
[warning] 244-244: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.readFileSync(filePath, { encoding: null, flag: 'r' })
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
[warning] 260-279: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.writeFileSync(
helperPath,
`#!/bin/sh
set -eu
HELPER_DIR="$(CDPATH= cd -- "$(dirname -- "$0")" && pwd)"
if [ -x "$HELPER_DIR/peekaboo" ]; then
exec "$HELPER_DIR/peekaboo" "$@"
fi
for candidate in /opt/homebrew/bin/peekaboo /usr/local/bin/peekaboo; do
if [ -x "$candidate" ]; then
exec "$candidate" "$@"
fi
done
echo "evaos-connector-helper: bundled peekaboo was not found. Rebuild evaOS Workbench with Bridge/bin/peekaboo or install the Workbench connector package." >&2
exit 127
`
)
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
[warning] 286-300: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.writeFileSync(
peekabooPath,
`#!/bin/sh
set -eu
for candidate in /opt/homebrew/bin/peekaboo /usr/local/bin/peekaboo; do
if [ -x "$candidate" ]; then
exec "$candidate" "$@"
fi
done
echo "peekaboo: bundled helper binary was not found. Rebuild evaOS Workbench with Bridge/bin/peekaboo or install the Workbench connector package." >&2
exit 127
`
)
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
[warning] 361-361: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.writeFileSync(wrapperPath, bridgeWrapperScript())
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
[warning] 381-395: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.writeFileSync(
wrapperPath,
`#!/bin/sh
set -eu
if [ "${1:-}" = "--version" ] || [ "${1:-}" = "version" ]; then
echo "evaos-desktop-bridge diagnostic-placeholder"
exit 0
fi
echo "evaos-desktop-bridge diagnostic placeholder: ${escapeForShellDoubleQuotes(reason)}" >&2
echo "This PR/build artifact is not valid for Mac pairing release proof. Configure EVAOS_DESKTOP_BRIDGE_SOURCE_DIR or EVAOS_DESKTOP_BRIDGE_SOURCE_TOKEN for a real release build." >&2
exit 78
`
)
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
[warning] 400-400: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.writeFileSync(path.join(bridgeResourceDir, 'manifest.json'), ${JSON.stringify(manifest, null, 2)}\n)
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
[error] 437-437: An archive entry path (e.g. entry.path / entry.fileName / header.name) is joined to an output directory without validating that the resolved path stays inside that directory. A malicious archive can use "../" sequences to escape the extraction directory and overwrite arbitrary files (Zip Slip). Resolve the path and verify it starts with the normalized output directory, or strip traversal with path.basename, before writing the entry.
Context: path.join(dir, entry.name)
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(zip-slip-archive-extraction-javascript)
tests/unit/bootstrap/prepareEvaosDesktopBridgeResource.test.ts
[warning] 6-6: Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import { spawnSync } from 'node:child_process';
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process-typescript)
🔇 Additional comments (12)
tests/unit/bootstrap/prepareEvaosDesktopBridgeResource.test.ts (1)
1-261: LGTM!tests/unit/bootstrap/afterPackPackagingProfile.test.ts (1)
7-37: LGTM!Also applies to: 56-73, 83-148, 158-165, 208-331
tests/unit/bootstrap/buildWithBuilder.test.ts (1)
15-37: LGTM!Also applies to: 112-112, 127-159, 204-207
.gitignore (1)
207-207: LGTM!packages/desktop/electron-builder.yml (1)
136-138: LGTM!scripts/build-with-builder.js (1)
126-154: LGTM!Also applies to: 274-279, 572-572, 599-605
.github/workflows/_build-reusable.yml (1)
452-452: LGTM!scripts/prepareEvaosDesktopBridgeResource.js (1)
1-98: LGTM!Also applies to: 101-515
.github/workflows/pr-checks.yml (2)
728-728: Still enforce the diagnostic placeholder contract here.This line only reports
manifest.placeholder; it does not fail if the thin-smoke artifact unexpectedly bundles a real Bridge. This appears to be the same invariant previously flagged for this workflow.
655-657: LGTM!Also applies to: 683-684, 698-702, 738-741
scripts/afterPack.js (1)
11-19: LGTM!Also applies to: 135-223, 236-269, 387-387, 461-461
.github/workflows/workbench-functional-smoke.yml (1)
19-23: LGTM!Also applies to: 38-38, 115-143, 176-194, 212-212
22bcede to
861bf2d
Compare
861bf2d to
e3a4c14
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@scripts/afterPack.js`:
- Around line 255-260: Update the bridge manifest validation in afterPack so it
also checks manifest.sourceCommit against EVAOS_DESKTOP_BRIDGE_SOURCE_REF,
alongside the existing placeholder guard in the manifest handling block that
uses readJsonFile and shouldRejectPlaceholderBridge. If the packaged manifest is
non-placeholder but the sourceCommit does not match the expected ref, throw the
same kind of build-failing error with clear context so stale bridges are
rejected in functional-smoke and real-bridge builds.
In `@scripts/prepareEvaosDesktopBridgeResource.js`:
- Around line 415-430: The placeholder artifact generation is still persisting
raw exception text through placeholderReason, which can leak absolute checkout
paths from assertPinnedSourceDir(). Update prepareEvaosDesktopBridgeResource.js
so the error handling path converts error?.message into a coarse error code or
fully path-scrubbed message before passing it to sanitizeCommandText,
writePlaceholderWrapper, and writeManifest, keeping the placeholderReason safe
for packaged scripts and manifests.
In `@tests/unit/bootstrap/afterPackPackagingProfile.test.ts`:
- Around line 208-331: The Darwin Bridge fixture tests in
afterPackPackagingProfile.test.ts should be skipped on win32 because they rely
on executable permission bits and Mach-O validation that are not reliable there.
Update the relevant cases around createDarwinContext,
createCompleteBridgeResources, and afterPack so they only run on macOS/Darwin
(or return early on Windows), keeping the placeholder/native-architecture
assertions intact for supported platforms.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 50f15e05-b2eb-4628-a754-91896e56b741
📒 Files selected for processing (11)
.github/workflows/_build-reusable.yml.github/workflows/pr-checks.yml.github/workflows/workbench-functional-smoke.yml.gitignorepackages/desktop/electron-builder.ymlscripts/afterPack.jsscripts/build-with-builder.jsscripts/prepareEvaosDesktopBridgeResource.jstests/unit/bootstrap/afterPackPackagingProfile.test.tstests/unit/bootstrap/buildWithBuilder.test.tstests/unit/bootstrap/prepareEvaosDesktopBridgeResource.test.ts
📜 Review details
⚠️ CI failures not shown inline (9)
GitHub Actions: Workbench Functional Smoke / 0_macOS arm64 unpacked app.txt: Workbench Functional Smoke
Conclusion: failure
##[group]Run set -euo pipefail
�[36;1mset -euo pipefail�[0m
�[36;1mif [ -z "${BRIDGE_SOURCE_***REDACTED*** ]; then�[0m
�[36;1m echo "::error::A private bridge source token is required to fetch electricsheephq/evaos-desktop-bridge for functional smoke."�[0m
GitHub Actions: Workbench Functional Smoke / macOS arm64 unpacked app: Workbench Functional Smoke
Conclusion: failure
##[group]Run set -euo pipefail
�[36;1mset -euo pipefail�[0m
�[36;1mif [ -z "${BRIDGE_SOURCE_***REDACTED*** ]; then�[0m
�[36;1m echo "::error::A private bridge source token is required to fetch electricsheephq/evaos-desktop-bridge for functional smoke."�[0m
GitHub Actions: Workbench Functional Smoke / macOS arm64 unpacked app: Workbench Functional Smoke
Conclusion: failure
##[group]Run set -euo pipefail
�[36;1mset -euo pipefail�[0m
�[36;1mAPP_PATH="$(find out -type d -name '*.app' -print -quit)"�[0m
�[36;1mif [ -z "$APP_PATH" ]; then�[0m
�[36;1m echo "::error::No .app produced under out/"�[0m
GitHub Actions: PR Checks / Thin App Smoke (macos-arm64): Restore Workbench Bridge resource proof
Conclusion: failure
##[group]Run set -euo pipefail
�[36;1mset -euo pipefail�[0m
�[36;1mecho "=========================================="�[0m
�[36;1mecho "VERIFY UNPACKED APP RESOURCE SHAPE"�[0m
�[36;1mecho "=========================================="�[0m
�[36;1mAPP_PATH="$(find out -type d -name '*.app' -print -quit)"�[0m
�[36;1mif [ -z "$APP_PATH" ]; then�[0m
�[36;1m echo "::error::No .app produced under out/"�[0m
GitHub Actions: PR Checks / Unit Tests (macos-14): Restore Workbench Bridge resource proof
Conclusion: failure
�[32m✓�[39m �[30m�[45m dom �[49m�[39m tests/unit/feedback/MessageToolGroupFeedback.dom.test.tsx �[2m(�[22m�[2m4 tests�[22m�[2m)�[22m�[32m 218�[2mms�[22m�[39m
�[32m✓�[39m �[30m�[45m dom �[49m�[39m tests/unit/previews/PreviewContext.dom.test.tsx �[2m(�[22m�[2m5 tests�[22m�[2m)�[22m�[32m 22�[2mms�[22m�[39m
�[32m✓�[39m �[30m�[45m dom �[49m�[39m tests/unit/settings/SystemSettings.dom.test.tsx �[2m(�[22m�[2m5 tests�[22m�[2m)�[22m�[32m 27�[2mms�[22m�[39m
�[32m✓�[39m �[30m�[45m dom �[49m�[39m tests/unit/feedback/FeedbackButton.dom.test.tsx �[2m(�[22m�[2m5 tests�[22m�[2m)�[22m�[32m 250�[2mms�[22m�[39m
�[32m✓�[39m �[30m�[45m dom �[49m�[39m tests/unit/feedback/McpServerHeaderFeedback.dom.test.tsx �[2m(�[22m�[2m4 tests�[22m�[2m)�[22m�[32m 169�[2mms�[22m�[39m
�[32m✓�[39m �[30m�[45m dom �[49m�[39m tests/unit/previews/MarkdownViewer.dom.test.tsx �[2m(�[22m�[2m3 tests�[22m�[2m)�[22m�[32m 76�[2mms�[22m�[39m
�[32m✓�[39m �[30m�[45m dom �[49m�[39m tests/unit/renderer/messageToolGroupSummary.dom.test.tsx �[2m(�[22m�[2m1 test�[22m�[2m)�[22m�[32m 79�[2mms�[22m�[39m
�[90mstdout�[2m | tests/unit/feedback/MessageAgentStatusFeedback.dom.test.tsx
�[22m�[39mbuild.buildStorage global
build.buildStorage system
�[90mstdout�[2m | tests/unit/feedback/MessageAgentStatusFeedback.dom.test.tsx�[2m > �[22m�[2mMessageAgentStatus — FeedbackButton wiring�[2m > �[22m�[2mdoes not render FeedbackButton on successful statuses
�[22m�[39m[httpBridge] GET /api/agents (no body)
[httpBridge] GET /api/assistants (no body)
�[90mstderr�[2m | tests/unit/feedback/MessageAgentStatusFeedback.dom.test.tsx�[2m > �[22m�[2mMessageAgentStatus — FeedbackButton wiring�[2m > �[22m�[2mdoes not render FeedbackButton on successful statuses
�[22m�[39mFailed to load assistants for conversation selector: TypeError: Failed to parse URL from /api/assistants
�[90m at node:internal/deps/undici/undici:14976:13�[39m
�[90m at processTicksAndRejections (node:internal/process/task_queues:103:5)�[39m
at httpR...
GitHub Actions: PR Checks / 0_Thin App Smoke (macos-arm64).txt: Restore Workbench Bridge resource proof
Conclusion: failure
##[group]Run set -euo pipefail
�[36;1mset -euo pipefail�[0m
�[36;1mecho "=========================================="�[0m
�[36;1mecho "VERIFY UNPACKED APP RESOURCE SHAPE"�[0m
�[36;1mecho "=========================================="�[0m
�[36;1mAPP_PATH="$(find out -type d -name '*.app' -print -quit)"�[0m
�[36;1mif [ -z "$APP_PATH" ]; then�[0m
�[36;1m echo "::error::No .app produced under out/"�[0m
GitHub Actions: PR Checks / Coverage Test: Restore Workbench Bridge resource proof
Conclusion: failure
9m
at �[90m/home/runner/work/evaOS-GUI/evaOS-GUI/�[39mtests/unit/assets/presetAssistantResources.test.ts:44:21
at �[90mfile:///home/runner/work/evaOS-GUI/evaOS-GUI/�[39mnode_modules/�[4m.bun�[24m/@vitest+runner@4.1.0/node_modules/�[4m@vitest/runner�[24m/dist/index.js:1258:20 {
[cause]: Error: connect ECONNREFUSED 127.0.0.1:13400
�[90m at TCPConnectWrap.afterConnect [as oncomplete] (node:net:1638:16)�[39m {
errno: �[33m-111�[39m,
code: �[32m'ECONNREFUSED'�[39m,
syscall: �[32m'connect'�[39m,
address: �[32m'127.0.0.1'�[39m,
port: �[33m13400�[39m
}
}
�[32m✓�[39m �[30m�[43m node �[49m�[39m tests/unit/assets/presetAssistantResources.test.ts �[2m(�[22m�[2m6 tests�[22m�[2m)�[22m�[32m 53�[2mms�[22m�[39m
�[32m✓�[39m �[30m�[43m node �[49m�[39m tests/unit/common/OpenAI2GeminiConverter.test.ts �[2m(�[22m�[2m2 tests�[22m�[2m)�[22m�[32m 7�[2mms�[22m�[39m
�[32m✓�[39m �[30m�[43m node �[49m�[39m tests/unit/renderer/warmupConversation.test.ts �[2m(�[22m�[2m2 tests�[22m�[2m)�[22m�[32m 13�[2mms�[22m�[39m
�[32m✓�[39m �[30m�[43m node �[49m�[39m tests/unit/common/platformConstants.test.ts �[2m(�[22m�[2m6 tests�[22m�[2m)�[22m�[32m 7�[2mms�[22m�[39m
�[32m✓�[39m �[30m�[43m node �[49m�[39m tests/unit/extension/extensionMapperIntegration.test.ts �[2m(�[22m�[2m3 tests�[22m�[2m)�[22m�[32m 7�[2mms�[22m�[39m
�[32m✓�[39m �[30m�[43m node �[49m�[39m tests/unit/common-adapter/workspaceMapper.test.ts �[2m(�[22m�[2m2 tests�[22m�[2m)�[22m�[32m 7�[2mms�[22m�[39m
�[32m✓�[39m �[30m�[43m node �[49m�[39m tests/unit/common/normalizeToolCall.test.ts �[2m(�[22m�[2m1 test�[22m�[2m)�[22m�[32m 8�[2mms�[22m�[39m
�[32m✓�[39m �[30m�[43m node �[49m�[39m tests/unit/cron/cronUtils.test.ts �[2m(�[22m�[2m2 tests�[22m�[2m)�[22m�[32m 9�[2mms�[22m�[39m
�[90mstdout�[2m | tests/unit/chat/toolCallLogging.test.ts
�[22m�[39mbuild.buildStorage global
build.buildStorage system
�[32m✓�[39m �[30m�[43m node �[49m�[39m tests/unit/chat/atFileQuery.test.ts �[2m(�[22m�[...
GitHub Actions: PR Checks / 2_Coverage Test.txt: Restore Workbench Bridge resource proof
Conclusion: failure
load.ts | 0 | 0 | 0 | 0 | 20-113
...leSelector.ts | 0 | 0 | 0 | 0 | 21-48
...steService.ts | 0 | 0 | 0 | 0 | 23-92
...ewLauncher.ts | 5.08 | 0 | 0 | 5.17 | 25-29,71-204
...ploadState.ts | 74.71 | 67.39 | 45 | 76.92 | ...49-250,255-276
...ceSelector.ts | 0 | 0 | 0 | 0 | 21-59
...erer/hooks/mcp | 0 | 0 | 0 | 0 |
catalog.ts | 0 | 0 | 0 | 0 | 16-117
index.ts | 0 | 0 | 0 | 0 |
messageQueue.ts | 0 | 0 | 0 | 0 | 15-77
...Connection.ts | 0 | 0 | 0 | 0 | 14-266
useMcpModal.ts | 0 | 100 | 0 | 0 | 8-50
useMcpOAuth.ts | 0 | 0 | 0 | 0 | 16-181
...ServerCRUD.ts | 0 | 0 | 0 | 0 | 9-148
useMcpServers.ts | 0 | 0 | 0 | 0 | 11-76
...r/hooks/system | 7.95 | 3.64 | 3.65 | 8.01 |
...lesEnabled.ts | 44.44 | 0 | 0 | 50 | 7,10,16-17
useDeepLink.ts | 0 | 0 | 0 | 0 | 27-91
useExtI18n.ts | 0 | 0 | 0 | 0 | 18-67
...ttingsTabs.ts | 12.12 | 0 | 0 | 13.33 | 21-78
...ationClick.ts | 0 | 0 | 0 | 0 | 15-34
...lDetection.ts | 0 | 0 | 0 | 0 | 59-186
usePwaMode.ts | 0 | 0 | 0 | 0 | 15-30
...peechInput.ts | 0 | 0 | 0 | 0 | 40-423
useTheme.ts | 58.69 | 70 | 33.33 | 57.77 | 53-95
...derer/hooks/ui | 12.11 | 3.24 | 4.28 | 12.1 |
...olorScheme.ts | 57.77 | 62.5 | 33.33 | 56.81 | 65-116
...nShortcuts.ts | 0 | 0 | 0 | 0 | 11-74
useDebounce.ts | 0 | 0 | 0 | 0 | 11-38
...
GitHub Actions: PR Checks / 5_Unit Tests (macos-14).txt: Restore Workbench Bridge resource proof
Conclusion: failure
mmon/adapter/httpBridge.ts:191:26�[90m)�[39m
at Object.invoke �[90m(/Users/runner/work/evaOS-GUI/evaOS-GUI/�[39mpackages/desktop/src/common/adapter/httpBridge.ts:257:14�[90m)�[39m
at �[90m/Users/runner/work/evaOS-GUI/evaOS-GUI/�[39mpackages/desktop/src/renderer/pages/conversation/hooks/useConversationAgents.ts:42:52
at fetcher �[90m(file:///Users/runner/work/evaOS-GUI/evaOS-GUI/�[39mnode_modules/�[4m.bun�[24m/swr@2.4.1+b1ab299f0a400331/node_modules/�[4mswr�[24m/dist/_internal/index.mjs:64:42�[90m)�[39m {
code: �[32m'ERR_INVALID_URL'�[39m,
input: �[32m'/api/assistants'�[39m
}
}
An update to MessageAgentStatus inside a test was not wrapped in act(...).
When testing, code that causes React state updates should be wrapped into act(...):
act(() => {
/* fire events that update state */
});
/* assert on the output */
This ensures that you're testing the behavior the user would see in the browser. Learn more at https://react.dev/link/wrap-tests-with-act
An update to MessageAgentStatus inside a test was not wrapped in act(...).
When testing, code that causes React state updates should be wrapped into act(...):
act(() => {
/* fire events that update state */
});
/* assert on the output */
This ensures that you're testing the behavior the user would see in the browser. Learn more at https://react.dev/link/wrap-tests-with-act
�[32m✓�[39m �[30m�[45m dom �[49m�[39m tests/unit/feedback/MessageAgentStatusFeedback.dom.test.tsx �[2m(�[22m�[2m3 tests�[22m�[2m)�[22m�[32m 201�[2mms�[22m�[39m
�[32m✓�[39m �[30m�[45m dom �[49m�[39m tests/unit/cron/CronStatusTag.dom.test.tsx �[2m(�[22m�[2m6 tests�[22m�[2m)�[22m�[32m 30�[2mms�[22m�[39m
�[32m✓�[39m �[30m�[45m dom �[49m�[39m tests/unit/previews/ExcelViewer.dom.test.tsx �[2m(�[22m�[2m3 tests�[22m�[2m)�[22m�[32m 19�[2mms�[22m�[39m
�[32m✓�[39m �[30m�[45m dom �[49m�[39m tests/unit/renderer/useSiderTeamBadges.dom.test.ts �[2m(�[22m�[2m1 test�[22m�[2m)�[22m�[32m 21�[2mms�[22m�[39m
�[3...
🧰 Additional context used
📓 Path-based instructions (4)
**/*[!.module].ts
📄 CodeRabbit inference engine (AGENTS.md)
Utility files must use camelCase naming (e.g., formatDate.ts)
Files:
tests/unit/bootstrap/buildWithBuilder.test.tstests/unit/bootstrap/prepareEvaosDesktopBridgeResource.test.tstests/unit/bootstrap/afterPackPackagingProfile.test.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Unused function parameters must be prefixed with underscore (_)
Use single-element arrays inline if they fit on one line; require trailing commas in multi-line arrays/objects; use single quotes for strings
Use English for code comments; provide JSDoc for public functions
Files:
tests/unit/bootstrap/buildWithBuilder.test.tsscripts/build-with-builder.jsscripts/afterPack.jstests/unit/bootstrap/prepareEvaosDesktopBridgeResource.test.tstests/unit/bootstrap/afterPackPackagingProfile.test.tsscripts/prepareEvaosDesktopBridgeResource.js
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use@icon-park/reactfor icons
TypeScript strict mode must be enabled; no 'any' types, no implicit returns
Use path aliases:@/,@process/,@renderer/* for imports
Prefer 'type' over 'interface' for type definitions (per Oxlint config)
Files:
tests/unit/bootstrap/buildWithBuilder.test.tstests/unit/bootstrap/prepareEvaosDesktopBridgeResource.test.tstests/unit/bootstrap/afterPackPackagingProfile.test.ts
**/*.{test,spec}.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Vitest 4 for testing with a coverage target of at least 80%
Files:
tests/unit/bootstrap/buildWithBuilder.test.tstests/unit/bootstrap/prepareEvaosDesktopBridgeResource.test.tstests/unit/bootstrap/afterPackPackagingProfile.test.ts
🪛 ast-grep (0.44.0)
scripts/afterPack.js
[warning] 136-136: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.readFileSync(filePath, 'utf8')
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
[warning] 167-167: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.readFileSync(filePath, { encoding: null, flag: 'r' })
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
[warning] 175-175: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.readFileSync(filePath, { encoding: null, flag: 'r' })
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
tests/unit/bootstrap/prepareEvaosDesktopBridgeResource.test.ts
[warning] 6-6: Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import { spawnSync } from 'node:child_process';
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process-typescript)
scripts/prepareEvaosDesktopBridgeResource.js
[error] 437-437: An archive entry path (e.g. entry.path / entry.fileName / header.name) is joined to an output directory without validating that the resolved path stays inside that directory. A malicious archive can use "../" sequences to escape the extraction directory and overwrite arbitrary files (Zip Slip). Resolve the path and verify it starts with the normalized output directory, or strip traversal with path.basename, before writing the entry.
Context: path.join(dir, entry.name)
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(zip-slip-archive-extraction-javascript)
[warning] 4-4: Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: require('child_process')
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process)
[warning] 244-244: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.readFileSync(filePath, { encoding: null, flag: 'r' })
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
[warning] 260-279: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.writeFileSync(
helperPath,
`#!/bin/sh
set -eu
HELPER_DIR="$(CDPATH= cd -- "$(dirname -- "$0")" && pwd)"
if [ -x "$HELPER_DIR/peekaboo" ]; then
exec "$HELPER_DIR/peekaboo" "$@"
fi
for candidate in /opt/homebrew/bin/peekaboo /usr/local/bin/peekaboo; do
if [ -x "$candidate" ]; then
exec "$candidate" "$@"
fi
done
echo "evaos-connector-helper: bundled peekaboo was not found. Rebuild evaOS Workbench with Bridge/bin/peekaboo or install the Workbench connector package." >&2
exit 127
`
)
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
[warning] 286-300: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.writeFileSync(
peekabooPath,
`#!/bin/sh
set -eu
for candidate in /opt/homebrew/bin/peekaboo /usr/local/bin/peekaboo; do
if [ -x "$candidate" ]; then
exec "$candidate" "$@"
fi
done
echo "peekaboo: bundled helper binary was not found. Rebuild evaOS Workbench with Bridge/bin/peekaboo or install the Workbench connector package." >&2
exit 127
`
)
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
[warning] 361-361: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.writeFileSync(wrapperPath, bridgeWrapperScript())
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
[warning] 381-395: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.writeFileSync(
wrapperPath,
`#!/bin/sh
set -eu
if [ "${1:-}" = "--version" ] || [ "${1:-}" = "version" ]; then
echo "evaos-desktop-bridge diagnostic-placeholder"
exit 0
fi
echo "evaos-desktop-bridge diagnostic placeholder: ${escapeForShellDoubleQuotes(reason)}" >&2
echo "This PR/build artifact is not valid for Mac pairing release proof. Configure EVAOS_DESKTOP_BRIDGE_SOURCE_DIR or EVAOS_DESKTOP_BRIDGE_SOURCE_TOKEN for a real release build." >&2
exit 78
`
)
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
[warning] 400-400: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.writeFileSync(path.join(bridgeResourceDir, 'manifest.json'), ${JSON.stringify(manifest, null, 2)}\n)
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
🪛 GitHub Actions: PR Checks / 5_Unit Tests (macos-14).txt
tests/unit/bootstrap/afterPackPackagingProfile.test.ts
[warning] 1-1: Test logged warning: "app.asar.unpacked not found" (afterPack packaging profile guard).
🪛 GitHub Actions: PR Checks / Unit Tests (macos-14)
tests/unit/bootstrap/afterPackPackagingProfile.test.ts
[warning] 1-1: Packaging warnings during afterPack hook tests: app.asar.unpacked not found (logged as
🔇 Additional comments (17)
scripts/afterPack.js (3)
44-55: Still require executable resources to be regular files.This repeats the prior finding:
fs.accessSync(..., X_OK)can accept executable directories, soBridge/evaos-desktop-bridgeshould also pass anfs.statSync(...).isFile()check before accepting the resource.
11-19: LGTM!Also applies to: 135-223, 387-387, 461-461
236-254: LGTM!Also applies to: 262-269
.github/workflows/pr-checks.yml (3)
693-697: Still require the Bridge CLI path to be a file.This repeats the prior finding:
[ -x "$BRIDGE" ]can pass for directories on macOS. Require both-fand-xbefore accepting the bundled Bridge CLI.
728-728: Still fail thin smoke when the Bridge manifest is not the diagnostic placeholder.This repeats the prior finding: this lane should prove the secret-free placeholder contract, but the current command only summarizes a real Bridge instead of exiting non-zero.
655-657: LGTM!Also applies to: 683-684, 698-702, 738-741
scripts/prepareEvaosDesktopBridgeResource.js (2)
95-100: NormalizeEVAOS_DESKTOP_BRIDGE_DISABLE_DEFAULT_CANDIDATESwithtruthy().
true/yesstill disable nothing here, unlike the other env toggles in this script.
140-148: 🎯 Functional CorrectnessPinned SHA checkout still depends on remote SHA support. If proof lanes pin arbitrary 40-char commits, switch to a flow that fetches the commit by immutable ID first, then keep
assertPinnedSourceDir()as the guard..gitignore (1)
207-207: LGTM!packages/desktop/electron-builder.yml (1)
136-138: LGTM!scripts/build-with-builder.js (2)
122-190: LGTM!Also applies to: 572-572, 599-605
274-277: 🎯 Functional CorrectnessNo issue here.
--allis not an electron-builder CLI flag, and this repo’s build entrypoints don’t pass it, so this macOS branch doesn’t introduce a supported non-mac build path.> Likely an incorrect or invalid review comment..github/workflows/_build-reusable.yml (1)
452-452: LGTM!.github/workflows/workbench-functional-smoke.yml (1)
19-23: LGTM!Also applies to: 38-38, 115-143, 176-215
tests/unit/bootstrap/prepareEvaosDesktopBridgeResource.test.ts (1)
39-260: LGTM!tests/unit/bootstrap/afterPackPackagingProfile.test.ts (1)
7-37: LGTM!Also applies to: 56-165
tests/unit/bootstrap/buildWithBuilder.test.ts (1)
15-41: LGTM!Also applies to: 112-122, 131-163, 208-211
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@tests/unit/bootstrap/buildWithBuilder.test.ts`:
- Around line 18-27: The spawned build environment is still leaking signing
identity signals because sanitizedProcessEnv() only clears the current
ambientBuildEnvKeys set; update the test setup around
ambientBuildEnvKeys/sanitizedProcessEnv() so appleId and APPLE_ID are also
removed from the spawned env. Keep the fix localized to the buildWithBuilder
test helpers so thin/non-release cases cannot be affected by a runner-provided
signing identity.
In `@tests/unit/bootstrap/prepareEvaosDesktopBridgeResource.test.ts`:
- Around line 160-174: Restore all environment variables mutated in this test
case, not just EVAOS_DESKTOP_BRIDGE_DISABLE_DEFAULT_CANDIDATES. In the
prepareEvaosDesktopBridgeResource test around bridgeScript.sourceCandidates(),
save the original values for EVAOS_DESKTOP_BRIDGE_SOURCE_DIR and
EVAOS_DESKTOP_BRIDGE_SOURCE_REF before deleting them, then restore them in the
finally block alongside the disable flag so the test does not leak state into
later cases.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: eada0298-add4-4484-ad35-ce407e892352
📒 Files selected for processing (11)
.github/workflows/_build-reusable.yml.github/workflows/pr-checks.yml.github/workflows/workbench-functional-smoke.yml.gitignorepackages/desktop/electron-builder.ymlscripts/afterPack.jsscripts/build-with-builder.jsscripts/prepareEvaosDesktopBridgeResource.jstests/unit/bootstrap/afterPackPackagingProfile.test.tstests/unit/bootstrap/buildWithBuilder.test.tstests/unit/bootstrap/prepareEvaosDesktopBridgeResource.test.ts
📜 Review details
⚠️ CI failures not shown inline (6)
GitHub Actions: PR Checks / Thin App Smoke (macos-arm64): Restore Workbench Bridge resource proof
Conclusion: failure
##[group]Run set -euo pipefail
�[36;1mset -euo pipefail�[0m
�[36;1mecho "=========================================="�[0m
�[36;1mecho "VERIFY UNPACKED APP RESOURCE SHAPE"�[0m
�[36;1mecho "=========================================="�[0m
�[36;1mAPP_PATH="$(find out -type d -name '*.app' -print -quit)"�[0m
�[36;1mif [ -z "$APP_PATH" ]; then�[0m
�[36;1m echo "::error::No .app produced under out/"�[0m
GitHub Actions: PR Checks / 0_Thin App Smoke (macos-arm64).txt: Restore Workbench Bridge resource proof
Conclusion: failure
##[group]Run set -euo pipefail
�[36;1mset -euo pipefail�[0m
�[36;1mecho "=========================================="�[0m
�[36;1mecho "VERIFY UNPACKED APP RESOURCE SHAPE"�[0m
�[36;1mecho "=========================================="�[0m
�[36;1mAPP_PATH="$(find out -type d -name '*.app' -print -quit)"�[0m
�[36;1mif [ -z "$APP_PATH" ]; then�[0m
�[36;1m echo "::error::No .app produced under out/"�[0m
GitHub Actions: PR Checks / Coverage Test: Restore Workbench Bridge resource proof
Conclusion: failure
m
at �[90m/home/runner/work/evaOS-GUI/evaOS-GUI/�[39mtests/unit/assets/presetAssistantResources.test.ts:44:21
at �[90mfile:///home/runner/work/evaOS-GUI/evaOS-GUI/�[39mnode_modules/�[4m.bun�[24m/@vitest+runner@4.1.0/node_modules/�[4m@vitest/runner�[24m/dist/index.js:1258:20 {
[cause]: Error: connect ECONNREFUSED 127.0.0.1:13400
�[90m at TCPConnectWrap.afterConnect [as oncomplete] (node:net:1638:16)�[39m {
errno: �[33m-111�[39m,
code: �[32m'ECONNREFUSED'�[39m,
syscall: �[32m'connect'�[39m,
address: �[32m'127.0.0.1'�[39m,
port: �[33m13400�[39m
}
}
�[32m✓�[39m �[30m�[43m node �[49m�[39m tests/unit/assets/presetAssistantResources.test.ts �[2m(�[22m�[2m6 tests�[22m�[2m)�[22m�[32m 64�[2mms�[22m�[39m
�[32m✓�[39m �[30m�[43m node �[49m�[39m tests/unit/common/OpenAI2GeminiConverter.test.ts �[2m(�[22m�[2m2 tests�[22m�[2m)�[22m�[32m 5�[2mms�[22m�[39m
�[32m✓�[39m �[30m�[43m node �[49m�[39m tests/unit/renderer/warmupConversation.test.ts �[2m(�[22m�[2m2 tests�[22m�[2m)�[22m�[32m 13�[2mms�[22m�[39m
�[32m✓�[39m �[30m�[43m node �[49m�[39m tests/unit/common/platformConstants.test.ts �[2m(�[22m�[2m6 tests�[22m�[2m)�[22m�[32m 5�[2mms�[22m�[39m
�[32m✓�[39m �[30m�[43m node �[49m�[39m tests/unit/extension/extensionMapperIntegration.test.ts �[2m(�[22m�[2m3 tests�[22m�[2m)�[22m�[32m 9�[2mms�[22m�[39m
�[32m✓�[39m �[30m�[43m node �[49m�[39m tests/unit/common-adapter/workspaceMapper.test.ts �[2m(�[22m�[2m2 tests�[22m�[2m)�[22m�[32m 6�[2mms�[22m�[39m
�[32m✓�[39m �[30m�[43m node �[49m�[39m tests/unit/common/normalizeToolCall.test.ts �[2m(�[22m�[2m1 test�[22m�[2m)�[22m�[32m 5�[2mms�[22m�[39m
�[32m✓�[39m �[30m�[43m node �[49m�[39m tests/unit/cron/cronUtils.test.ts �[2m(�[22m�[2m2 tests�[22m�[2m)�[22m�[32m 11�[2mms�[22m�[39m
�[32m✓�[39m �[30m�[43m node �[49m�[39m tests/unit/chat/atFileQuery.test.ts �[2m(�[22m�[2m3 tests�[22m�[2m)�[22m�[32m 6�[2mms�[22m�[39m
�[90mstdout�[2m | tests/unit/chat/toolCallLogging.test.ts
�[22m�[39mbuild...
GitHub Actions: PR Checks / Unit Tests (macos-14): Restore Workbench Bridge resource proof
Conclusion: failure
�[22m�[2m)�[22m�[32m 76�[2mms�[22m�[39m
�[32m✓�[39m �[30m�[45m dom �[49m�[39m tests/unit/renderer/messageToolGroupSummary.dom.test.tsx �[2m(�[22m�[2m1 test�[22m�[2m)�[22m�[32m 62�[2mms�[22m�[39m
�[90mstdout�[2m | tests/unit/feedback/MessageAgentStatusFeedback.dom.test.tsx
�[22m�[39mbuild.buildStorage global
build.buildStorage system
�[90mstdout�[2m | tests/unit/feedback/MessageAgentStatusFeedback.dom.test.tsx�[2m > �[22m�[2mMessageAgentStatus — FeedbackButton wiring�[2m > �[22m�[2mdoes not render FeedbackButton on successful statuses
�[22m�[39m[httpBridge] GET /api/agents (no body)
[httpBridge] GET /api/assistants (no body)
�[90mstderr�[2m | tests/unit/feedback/MessageAgentStatusFeedback.dom.test.tsx�[2m > �[22m�[2mMessageAgentStatus — FeedbackButton wiring�[2m > �[22m�[2mdoes not render FeedbackButton on successful statuses
�[22m�[39mFailed to load assistants for conversation selector: TypeError: Failed to parse URL from /api/assistants
�[90m at node:internal/deps/undici/undici:14976:13�[39m
�[90m at processTicksAndRejections (node:internal/process/task_queues:103:5)�[39m
at httpRequest �[90m(/Users/runner/work/evaOS-GUI/evaOS-GUI/�[39mpackages/desktop/src/common/adapter/httpBridge.ts:191:20�[90m)�[39m {
[cause]: TypeError: Invalid URL
�[90m at new URL (node:internal/url:818:25)�[39m
at new URL �[90m(file:///Users/runner/work/evaOS-GUI/evaOS-GUI/�[39mnode_modules/�[4m.bun�[24m/vitest@4.1.0+634674f7a4778bc9/node_modules/�[4mvitest�[24m/dist/chunks/index.EY6TCHpo.js:557:2�[90m)�[39m
�[90m at new Request (node:internal/deps/undici/undici:9804:25)�[39m
�[90m at fetch (node:internal/deps/undici/undici:10533:25)�[39m
�[90m at fetch (node:internal/deps/undici/undici:14974:10)�[39m
�[90m at fetch (node:internal/bootstrap/web/exposed-window-or-worker:75:12)�[39m
at httpRequest �[90m(/Users/runner/work/evaOS-GUI/evaOS-GUI/�[39mpackages/desktop/src/common/adapter/httpBridge.ts:191:26�[90m)�[39m
at...
GitHub Actions: PR Checks / 2_Unit Tests (macos-14).txt: Restore Workbench Bridge resource proof
Conclusion: failure
ensures that you're testing the behavior the user would see in the browser. Learn more at https://react.dev/link/wrap-tests-with-act
An update to MessageAgentStatus inside a test was not wrapped in act(...).
When testing, code that causes React state updates should be wrapped into act(...):
act(() => {
/* fire events that update state */
});
/* assert on the output */
This ensures that you're testing the behavior the user would see in the browser. Learn more at https://react.dev/link/wrap-tests-with-act
�[32m✓�[39m �[30m�[45m dom �[49m�[39m tests/unit/feedback/MessageAgentStatusFeedback.dom.test.tsx �[2m(�[22m�[2m3 tests�[22m�[2m)�[22m�[32m 212�[2mms�[22m�[39m
�[32m✓�[39m �[30m�[45m dom �[49m�[39m tests/unit/previews/ExcelViewer.dom.test.tsx �[2m(�[22m�[2m3 tests�[22m�[2m)�[22m�[32m 34�[2mms�[22m�[39m
�[32m✓�[39m �[30m�[45m dom �[49m�[39m tests/unit/cron/CronStatusTag.dom.test.tsx �[2m(�[22m�[2m6 tests�[22m�[2m)�[22m�[32m 44�[2mms�[22m�[39m
�[32m✓�[39m �[30m�[45m dom �[49m�[39m tests/unit/renderer/useSiderTeamBadges.dom.test.ts �[2m(�[22m�[2m1 test�[22m�[2m)�[22m�[32m 14�[2mms�[22m�[39m
�[32m✓�[39m �[30m�[45m dom �[49m�[39m tests/unit/renderer/useGuidInput.dom.test.ts �[2m(�[22m�[2m2 tests�[22m�[2m)�[22m�[32m 18�[2mms�[22m�[39m
�[90mstdout�[2m | tests/unit/previews/OfficeWatchViewer.dom.test.tsx�[2m > �[22m�[2mOfficeWatchViewer module shape�[2m > �[22m�[2mmodule loads and exposes a default export
�[22m�[39mbuild.buildStorage global
build.buildStorage system
�[32m✓�[39m �[30m�[45m dom �[49m�[39m tests/unit/previews/OfficeWatchViewer.dom.test.tsx �[2m(�[22m�[2m3 tests�[22m�[2m)�[22m�[33m 1065�[2mms�[22m�[39m
�[33m�[2m✓�[22m�[39m module loads and exposes a default export �[33m 1062�[2mms�[22m�[39m
�[32m✓�[39m �[30m�[45m dom �[49m�[39m tests/unit/renderer/messageThinking.dom.test.tsx �[2m(�[22m�[2m1 test�[22m�[2m)�[22m�[32m 34�[2mms�[22m�[39m
�[90mstdout�[2m | tests/unit/previews/usePreviewHistory.dom.test.ts�[2m > �[22m�[2musePrevie...
GitHub Actions: PR Checks / 4_Coverage Test.txt: Restore Workbench Bridge resource proof
Conclusion: failure
load.ts | 0 | 0 | 0 | 0 | 20-113
...leSelector.ts | 0 | 0 | 0 | 0 | 21-48
...steService.ts | 0 | 0 | 0 | 0 | 23-92
...ewLauncher.ts | 5.08 | 0 | 0 | 5.17 | 25-29,71-204
...ploadState.ts | 74.71 | 67.39 | 45 | 76.92 | ...49-250,255-276
...ceSelector.ts | 0 | 0 | 0 | 0 | 21-59
...erer/hooks/mcp | 0 | 0 | 0 | 0 |
catalog.ts | 0 | 0 | 0 | 0 | 16-117
index.ts | 0 | 0 | 0 | 0 |
messageQueue.ts | 0 | 0 | 0 | 0 | 15-77
...Connection.ts | 0 | 0 | 0 | 0 | 14-266
useMcpModal.ts | 0 | 100 | 0 | 0 | 8-50
useMcpOAuth.ts | 0 | 0 | 0 | 0 | 16-181
...ServerCRUD.ts | 0 | 0 | 0 | 0 | 9-148
useMcpServers.ts | 0 | 0 | 0 | 0 | 11-76
...r/hooks/system | 7.95 | 3.64 | 3.65 | 8.01 |
...lesEnabled.ts | 44.44 | 0 | 0 | 50 | 7,10,16-17
useDeepLink.ts | 0 | 0 | 0 | 0 | 27-91
useExtI18n.ts | 0 | 0 | 0 | 0 | 18-67
...ttingsTabs.ts | 12.12 | 0 | 0 | 13.33 | 21-78
...ationClick.ts | 0 | 0 | 0 | 0 | 15-34
...lDetection.ts | 0 | 0 | 0 | 0 | 59-186
usePwaMode.ts | 0 | 0 | 0 | 0 | 15-30
...peechInput.ts | 0 | 0 | 0 | 0 | 40-423
useTheme.ts | 58.69 | 70 | 33.33 | 57.77 | 53-95
...derer/hooks/ui | 12.11 | 3.24 | 4.28 | 12.1 |
...olorScheme.ts | 57.77 | 62.5 | 33.33 | 56.81 | 65-116
...nShortcuts.ts | 0 | 0 | 0 | 0 | 11-74
useDebounce.ts | 0 | 0 | 0 | 0 | 11-38
...
🧰 Additional context used
📓 Path-based instructions (4)
**/*[!.module].ts
📄 CodeRabbit inference engine (AGENTS.md)
Utility files must use camelCase naming (e.g., formatDate.ts)
Files:
tests/unit/bootstrap/buildWithBuilder.test.tstests/unit/bootstrap/afterPackPackagingProfile.test.tstests/unit/bootstrap/prepareEvaosDesktopBridgeResource.test.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Unused function parameters must be prefixed with underscore (_)
Use single-element arrays inline if they fit on one line; require trailing commas in multi-line arrays/objects; use single quotes for strings
Use English for code comments; provide JSDoc for public functions
Files:
tests/unit/bootstrap/buildWithBuilder.test.tsscripts/build-with-builder.jstests/unit/bootstrap/afterPackPackagingProfile.test.tstests/unit/bootstrap/prepareEvaosDesktopBridgeResource.test.tsscripts/afterPack.jsscripts/prepareEvaosDesktopBridgeResource.js
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use@icon-park/reactfor icons
TypeScript strict mode must be enabled; no 'any' types, no implicit returns
Use path aliases:@/,@process/,@renderer/* for imports
Prefer 'type' over 'interface' for type definitions (per Oxlint config)
Files:
tests/unit/bootstrap/buildWithBuilder.test.tstests/unit/bootstrap/afterPackPackagingProfile.test.tstests/unit/bootstrap/prepareEvaosDesktopBridgeResource.test.ts
**/*.{test,spec}.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Vitest 4 for testing with a coverage target of at least 80%
Files:
tests/unit/bootstrap/buildWithBuilder.test.tstests/unit/bootstrap/afterPackPackagingProfile.test.tstests/unit/bootstrap/prepareEvaosDesktopBridgeResource.test.ts
🪛 ast-grep (0.44.0)
tests/unit/bootstrap/prepareEvaosDesktopBridgeResource.test.ts
[warning] 6-6: Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import { spawnSync } from 'node:child_process';
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process-typescript)
scripts/afterPack.js
[warning] 139-139: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.readFileSync(filePath, 'utf8')
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
[warning] 171-171: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.readFileSync(filePath, { encoding: null, flag: 'r' })
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
[warning] 179-179: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.readFileSync(filePath, { encoding: null, flag: 'r' })
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
scripts/prepareEvaosDesktopBridgeResource.js
[warning] 4-4: Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: require('child_process')
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process)
[warning] 244-244: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.readFileSync(filePath, { encoding: null, flag: 'r' })
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
[warning] 260-279: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.writeFileSync(
helperPath,
`#!/bin/sh
set -eu
HELPER_DIR="$(CDPATH= cd -- "$(dirname -- "$0")" && pwd)"
if [ -x "$HELPER_DIR/peekaboo" ]; then
exec "$HELPER_DIR/peekaboo" "$@"
fi
for candidate in /opt/homebrew/bin/peekaboo /usr/local/bin/peekaboo; do
if [ -x "$candidate" ]; then
exec "$candidate" "$@"
fi
done
echo "evaos-connector-helper: bundled peekaboo was not found. Rebuild evaOS Workbench with Bridge/bin/peekaboo or install the Workbench connector package." >&2
exit 127
`
)
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
[warning] 286-300: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.writeFileSync(
peekabooPath,
`#!/bin/sh
set -eu
for candidate in /opt/homebrew/bin/peekaboo /usr/local/bin/peekaboo; do
if [ -x "$candidate" ]; then
exec "$candidate" "$@"
fi
done
echo "peekaboo: bundled helper binary was not found. Rebuild evaOS Workbench with Bridge/bin/peekaboo or install the Workbench connector package." >&2
exit 127
`
)
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
[warning] 361-361: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.writeFileSync(wrapperPath, bridgeWrapperScript())
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
[warning] 381-395: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.writeFileSync(
wrapperPath,
`#!/bin/sh
set -eu
if [ "${1:-}" = "--version" ] || [ "${1:-}" = "version" ]; then
echo "evaos-desktop-bridge diagnostic-placeholder"
exit 0
fi
echo "evaos-desktop-bridge diagnostic placeholder: ${escapeForShellDoubleQuotes(reason)}" >&2
echo "This PR/build artifact is not valid for Mac pairing release proof. Configure EVAOS_DESKTOP_BRIDGE_SOURCE_DIR or EVAOS_DESKTOP_BRIDGE_SOURCE_TOKEN for a real release build." >&2
exit 78
`
)
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
[warning] 400-400: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.writeFileSync(path.join(bridgeResourceDir, 'manifest.json'), ${JSON.stringify(manifest, null, 2)}\n)
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
[error] 437-437: An archive entry path (e.g. entry.path / entry.fileName / header.name) is joined to an output directory without validating that the resolved path stays inside that directory. A malicious archive can use "../" sequences to escape the extraction directory and overwrite arbitrary files (Zip Slip). Resolve the path and verify it starts with the normalized output directory, or strip traversal with path.basename, before writing the entry.
Context: path.join(dir, entry.name)
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(zip-slip-archive-extraction-javascript)
🪛 GitHub Actions: PR Checks / 2_Unit Tests (macos-14).txt
tests/unit/bootstrap/afterPackPackagingProfile.test.ts
[warning] 1-1: afterPack packaging profile guard warning: app.asar.unpacked not found.
🪛 GitHub Actions: PR Checks / Unit Tests (macos-14)
tests/unit/bootstrap/afterPackPackagingProfile.test.ts
[warning] 1-1: Packaging guard warning: app.asar.unpacked not found.
🔇 Additional comments (13)
tests/unit/bootstrap/afterPackPackagingProfile.test.ts (2)
208-349: Still skip Darwin executable-path cases on Windows.This matches the existing unresolved review note: these cases rely on executable mode bits and Mach-O fixture validation, which are unreliable on
win32. Apply the prior platform guard/skip to this block.
7-37: LGTM!Also applies to: 56-73, 83-148, 158-165
tests/unit/bootstrap/buildWithBuilder.test.ts (1)
116-116: LGTM!Also applies to: 131-139, 153-163, 208-211
scripts/prepareEvaosDesktopBridgeResource.js (2)
415-430: Duplicate: scrubplaceholderReasonbefore packaging it.The current
reasonstill only token-redactserror.messagebefore writing it into the placeholder wrapper and manifest; the existing review already flags absolute checkout/workspace path leakage here.
1-409: LGTM!Also applies to: 435-515
scripts/afterPack.js (2)
259-264: Duplicate: validate the bundled BridgesourceCommit.This still only rejects placeholder manifests; the existing review already covers comparing
manifest.sourceCommitwithEVAOS_DESKTOP_BRIDGE_SOURCE_REFso stale non-placeholder Bridge bundles fail.
11-19: LGTM!Also applies to: 44-59, 138-225, 240-258, 266-273, 391-391, 465-465
.gitignore (1)
207-207: LGTM!packages/desktop/electron-builder.yml (1)
136-138: LGTM!scripts/build-with-builder.js (1)
126-154: LGTM!Also applies to: 274-278, 572-572, 600-605
.github/workflows/_build-reusable.yml (1)
452-452: LGTM!.github/workflows/workbench-functional-smoke.yml (1)
19-23: LGTM!Also applies to: 38-38, 115-143, 176-194, 212-215
.github/workflows/pr-checks.yml (1)
655-657: LGTM!Also applies to: 683-684, 693-702, 728-728, 738-741
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/unit/bootstrap/afterPackPackagingProfile.test.ts (1)
23-25: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winClear
EVAOS_DESKTOP_BRIDGE_SOURCE_REFin Bridge test env setup.
afterPacknow uses this env var to compute the expected manifest commit, but these tests generate fixtures withDEFAULT_BRIDGE_SOURCE_REF. A pre-set runner value can make the functional-smoke success cases fail nondeterministically.Proposed fix
const BRIDGE_ENV_FLAGS = Array.from( - new Set([...RELEASE_ENV_FLAGS, 'appleId', 'APPLE_ID', 'EVAOS_DESKTOP_BRIDGE_REQUIRE_REAL', 'EVAOS_PACKAGING_PROFILE']) + new Set([ + ...RELEASE_ENV_FLAGS, + 'appleId', + 'APPLE_ID', + 'EVAOS_DESKTOP_BRIDGE_REQUIRE_REAL', + 'EVAOS_DESKTOP_BRIDGE_SOURCE_REF', + 'EVAOS_PACKAGING_PROFILE', + ]) );🤖 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 `@tests/unit/bootstrap/afterPackPackagingProfile.test.ts` around lines 23 - 25, The Bridge test environment setup is missing EVAOS_DESKTOP_BRIDGE_SOURCE_REF, which can leak a runner value into afterPack fixture tests and make the smoke success cases unstable. Update the BRIDGE_ENV_FLAGS list in afterPackPackagingProfile.test.ts so this variable is cleared alongside the other bridge-related env flags, ensuring the tests always use DEFAULT_BRIDGE_SOURCE_REF when exercising afterPack and related fixture generation.
🤖 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.
Outside diff comments:
In `@tests/unit/bootstrap/afterPackPackagingProfile.test.ts`:
- Around line 23-25: The Bridge test environment setup is missing
EVAOS_DESKTOP_BRIDGE_SOURCE_REF, which can leak a runner value into afterPack
fixture tests and make the smoke success cases unstable. Update the
BRIDGE_ENV_FLAGS list in afterPackPackagingProfile.test.ts so this variable is
cleared alongside the other bridge-related env flags, ensuring the tests always
use DEFAULT_BRIDGE_SOURCE_REF when exercising afterPack and related fixture
generation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 06ecbf30-0d25-4e45-814e-f5bdc4104d36
📒 Files selected for processing (4)
scripts/afterPack.jsscripts/prepareEvaosDesktopBridgeResource.jstests/unit/bootstrap/afterPackPackagingProfile.test.tstests/unit/bootstrap/prepareEvaosDesktopBridgeResource.test.ts
📜 Review details
⚠️ CI failures not shown inline (6)
GitHub Actions: PR Checks / Unit Tests (macos-14): Restore Workbench Bridge resource proof
Conclusion: failure
[39mFailed to load assistants for conversation selector: TypeError: Failed to parse URL from /api/assistants
�[90m at node:internal/deps/undici/undici:14976:13�[39m
�[90m at processTicksAndRejections (node:internal/process/task_queues:103:5)�[39m
at httpRequest �[90m(/Users/runner/work/evaOS-GUI/evaOS-GUI/�[39mpackages/desktop/src/common/adapter/httpBridge.ts:191:20�[90m)�[39m {
[cause]: TypeError: Invalid URL
�[90m at new URL (node:internal/url:818:25)�[39m
at new URL �[90m(file:///Users/runner/work/evaOS-GUI/evaOS-GUI/�[39mnode_modules/�[4m.bun�[24m/vitest@4.1.0+634674f7a4778bc9/node_modules/�[4mvitest�[24m/dist/chunks/index.EY6TCHpo.js:557:2�[90m)�[39m
�[90m at new Request (node:internal/deps/undici/undici:9804:25)�[39m
�[90m at fetch (node:internal/deps/undici/undici:10533:25)�[39m
�[90m at fetch (node:internal/deps/undici/undici:14974:10)�[39m
�[90m at fetch (node:internal/bootstrap/web/exposed-window-or-worker:75:12)�[39m
at httpRequest �[90m(/Users/runner/work/evaOS-GUI/evaOS-GUI/�[39mpackages/desktop/src/common/adapter/httpBridge.ts:191:26�[90m)�[39m
at Object.invoke �[90m(/Users/runner/work/evaOS-GUI/evaOS-GUI/�[39mpackages/desktop/src/common/adapter/httpBridge.ts:257:14�[90m)�[39m
at �[90m/Users/runner/work/evaOS-GUI/evaOS-GUI/�[39mpackages/desktop/src/renderer/pages/conversation/hooks/useConversationAgents.ts:42:52
at fetcher �[90m(file:///Users/runner/work/evaOS-GUI/evaOS-GUI/�[39mnode_modules/�[4m.bun�[24m/swr@2.4.1+b1ab299f0a400331/node_modules/�[4mswr�[24m/dist/_internal/index.mjs:64:42�[90m)�[39m {
code: �[32m'ERR_INVALID_URL'�[39m,
input: �[32m'/api/assistants'�[39m
}
}
An update to MessageAgentStatus inside a test was not wrapped in act(...).
When testing, code that causes React state updates should be wrapped into act(...):
act(() => {
/* fire events that update state */
});
/* assert on the output */
This ensures that you're testing the beha...
GitHub Actions: PR Checks / Thin App Smoke (macos-arm64): Restore Workbench Bridge resource proof
Conclusion: failure
##[group]Run set -euo pipefail
�[36;1mset -euo pipefail�[0m
�[36;1mecho "=========================================="�[0m
�[36;1mecho "VERIFY UNPACKED APP RESOURCE SHAPE"�[0m
�[36;1mecho "=========================================="�[0m
�[36;1mAPP_PATH="$(find out -type d -name '*.app' -print -quit)"�[0m
�[36;1mif [ -z "$APP_PATH" ]; then�[0m
�[36;1m echo "::error::No .app produced under out/"�[0m
GitHub Actions: PR Checks / 2_Unit Tests (macos-14).txt: Restore Workbench Bridge resource proof
Conclusion: failure
5m dom �[49m�[39m tests/unit/cron/CronStatusTag.dom.test.tsx �[2m(�[22m�[2m6 tests�[22m�[2m)�[22m�[32m 37�[2mms�[22m�[39m
�[32m✓�[39m �[30m�[45m dom �[49m�[39m tests/unit/renderer/useSiderTeamBadges.dom.test.ts �[2m(�[22m�[2m1 test�[22m�[2m)�[22m�[32m 14�[2mms�[22m�[39m
�[32m✓�[39m �[30m�[45m dom �[49m�[39m tests/unit/renderer/useGuidInput.dom.test.ts �[2m(�[22m�[2m2 tests�[22m�[2m)�[22m�[32m 15�[2mms�[22m�[39m
�[90mstdout�[2m | tests/unit/previews/OfficeWatchViewer.dom.test.tsx�[2m > �[22m�[2mOfficeWatchViewer module shape�[2m > �[22m�[2mmodule loads and exposes a default export
�[22m�[39mbuild.buildStorage global
build.buildStorage system
�[32m✓�[39m �[30m�[45m dom �[49m�[39m tests/unit/previews/OfficeWatchViewer.dom.test.tsx �[2m(�[22m�[2m3 tests�[22m�[2m)�[22m�[33m 740�[2mms�[22m�[39m
�[33m�[2m✓�[22m�[39m module loads and exposes a default export �[33m 739�[2mms�[22m�[39m
�[32m✓�[39m �[30m�[45m dom �[49m�[39m tests/unit/renderer/messageThinking.dom.test.tsx �[2m(�[22m�[2m1 test�[22m�[2m)�[22m�[32m 30�[2mms�[22m�[39m
�[90mstdout�[2m | tests/unit/previews/usePreviewHistory.dom.test.ts�[2m > �[22m�[2musePreviewHistory module shape�[2m > �[22m�[2mmodule loads and exposes usePreviewHistory
�[22m�[39mbuild.buildStorage global
build.buildStorage system
�[32m✓�[39m �[30m�[45m dom �[49m�[39m tests/unit/previews/HTMLViewer.dom.test.tsx �[2m(�[22m�[2m3 tests�[22m�[2m)�[22m�[32m 44�[2mms�[22m�[39m
�[32m✓�[39m �[30m�[45m dom �[49m�[39m tests/unit/previews/usePreviewHistory.dom.test.ts �[2m(�[22m�[2m3 tests�[22m�[2m)�[22m�[33m 329�[2mms�[22m�[39m
�[33m�[2m✓�[22m�[39m module loads and exposes usePreviewHistory �[33m 327�[2mms�[22m�[39m
�[32m✓�[39m �[30m�[45m dom �[49m�[39m tests/unit/previews/OfficeDocViewer.dom.test.tsx �[2m(�[22m�[2m3 tests�[22m�[2m)�[22m�[32m 20�[2mms�[22m�[39m
�[32m✓�[39m �[30m�[45m dom �[49m�[39m tests/unit/previews/PptViewer.dom.test.tsx �[2m(�[22m�[2m3 tests�[22m�[2m)�[22m�[32m 19�[2mms�[22m�[39m
�[32m✓�[39m �[30m�...
GitHub Actions: PR Checks / 5_Coverage Test.txt: Restore Workbench Bridge resource proof
Conclusion: failure
load.ts | 0 | 0 | 0 | 0 | 20-113
...leSelector.ts | 0 | 0 | 0 | 0 | 21-48
...steService.ts | 0 | 0 | 0 | 0 | 23-92
...ewLauncher.ts | 5.08 | 0 | 0 | 5.17 | 25-29,71-204
...ploadState.ts | 74.71 | 67.39 | 45 | 76.92 | ...49-250,255-276
...ceSelector.ts | 0 | 0 | 0 | 0 | 21-59
...erer/hooks/mcp | 0 | 0 | 0 | 0 |
catalog.ts | 0 | 0 | 0 | 0 | 16-117
index.ts | 0 | 0 | 0 | 0 |
messageQueue.ts | 0 | 0 | 0 | 0 | 15-77
...Connection.ts | 0 | 0 | 0 | 0 | 14-266
useMcpModal.ts | 0 | 100 | 0 | 0 | 8-50
useMcpOAuth.ts | 0 | 0 | 0 | 0 | 16-181
...ServerCRUD.ts | 0 | 0 | 0 | 0 | 9-148
useMcpServers.ts | 0 | 0 | 0 | 0 | 11-76
...r/hooks/system | 7.95 | 3.64 | 3.65 | 8.01 |
...lesEnabled.ts | 44.44 | 0 | 0 | 50 | 7,10,16-17
useDeepLink.ts | 0 | 0 | 0 | 0 | 27-91
useExtI18n.ts | 0 | 0 | 0 | 0 | 18-67
...ttingsTabs.ts | 12.12 | 0 | 0 | 13.33 | 21-78
...ationClick.ts | 0 | 0 | 0 | 0 | 15-34
...lDetection.ts | 0 | 0 | 0 | 0 | 59-186
usePwaMode.ts | 0 | 0 | 0 | 0 | 15-30
...peechInput.ts | 0 | 0 | 0 | 0 | 40-423
useTheme.ts | 58.69 | 70 | 33.33 | 57.77 | 53-95
...derer/hooks/ui | 12.11 | 3.24 | 4.28 | 12.1 |
...olorScheme.ts | 57.77 | 62.5 | 33.33 | 56.81 | 65-116
...nShortcuts.ts | 0 | 0 | 0 | 0 | 11-74
useDebounce.ts | 0 | 0 | 0 | 0 | 11-38
...
GitHub Actions: PR Checks / Coverage Test: Restore Workbench Bridge resource proof
Conclusion: failure
39m
at �[90m/home/runner/work/evaOS-GUI/evaOS-GUI/�[39mtests/unit/assets/presetAssistantResources.test.ts:44:21
at �[90mfile:///home/runner/work/evaOS-GUI/evaOS-GUI/�[39mnode_modules/�[4m.bun�[24m/@vitest+runner@4.1.0/node_modules/�[4m@vitest/runner�[24m/dist/index.js:1258:20 {
[cause]: Error: connect ECONNREFUSED 127.0.0.1:13400
�[90m at TCPConnectWrap.afterConnect [as oncomplete] (node:net:1638:16)�[39m {
errno: �[33m-111�[39m,
code: �[32m'ECONNREFUSED'�[39m,
syscall: �[32m'connect'�[39m,
address: �[32m'127.0.0.1'�[39m,
port: �[33m13400�[39m
}
}
�[32m✓�[39m �[30m�[43m node �[49m�[39m tests/unit/assets/presetAssistantResources.test.ts �[2m(�[22m�[2m6 tests�[22m�[2m)�[22m�[32m 60�[2mms�[22m�[39m
�[32m✓�[39m �[30m�[43m node �[49m�[39m tests/unit/common/OpenAI2GeminiConverter.test.ts �[2m(�[22m�[2m2 tests�[22m�[2m)�[22m�[32m 7�[2mms�[22m�[39m
�[32m✓�[39m �[30m�[43m node �[49m�[39m tests/unit/common/platformConstants.test.ts �[2m(�[22m�[2m6 tests�[22m�[2m)�[22m�[32m 6�[2mms�[22m�[39m
�[32m✓�[39m �[30m�[43m node �[49m�[39m tests/unit/renderer/warmupConversation.test.ts �[2m(�[22m�[2m2 tests�[22m�[2m)�[22m�[32m 11�[2mms�[22m�[39m
�[32m✓�[39m �[30m�[43m node �[49m�[39m tests/unit/extension/extensionMapperIntegration.test.ts �[2m(�[22m�[2m3 tests�[22m�[2m)�[22m�[32m 5�[2mms�[22m�[39m
�[32m✓�[39m �[30m�[43m node �[49m�[39m tests/unit/common-adapter/workspaceMapper.test.ts �[2m(�[22m�[2m2 tests�[22m�[2m)�[22m�[32m 6�[2mms�[22m�[39m
�[32m✓�[39m �[30m�[43m node �[49m�[39m tests/unit/common/normalizeToolCall.test.ts �[2m(�[22m�[2m1 test�[22m�[2m)�[22m�[32m 7�[2mms�[22m�[39m
�[32m✓�[39m �[30m�[43m node �[49m�[39m tests/unit/chat/atFileQuery.test.ts �[2m(�[22m�[2m3 tests�[22m�[2m)�[22m�[32m 6�[2mms�[22m�[39m
�[90mstdout�[2m | tests/unit/chat/toolCallLogging.test.ts
�[22m�[39mbuild.buildStorage global
build.buildStorage system
�[32m✓�[39m �[30m�[43m node �[49m�[39m tests/unit/cron/cronUtils.test.ts �[2m(�[22m�...
GitHub Actions: PR Checks / 0_Thin App Smoke (macos-arm64).txt: Restore Workbench Bridge resource proof
Conclusion: failure
##[group]Run set -euo pipefail
�[36;1mset -euo pipefail�[0m
�[36;1mecho "=========================================="�[0m
�[36;1mecho "VERIFY UNPACKED APP RESOURCE SHAPE"�[0m
�[36;1mecho "=========================================="�[0m
�[36;1mAPP_PATH="$(find out -type d -name '*.app' -print -quit)"�[0m
�[36;1mif [ -z "$APP_PATH" ]; then�[0m
�[36;1m echo "::error::No .app produced under out/"�[0m
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Unused function parameters must be prefixed with underscore (_)
Use single-element arrays inline if they fit on one line; require trailing commas in multi-line arrays/objects; use single quotes for strings
Use English for code comments; provide JSDoc for public functions
Files:
scripts/afterPack.jstests/unit/bootstrap/afterPackPackagingProfile.test.tstests/unit/bootstrap/prepareEvaosDesktopBridgeResource.test.tsscripts/prepareEvaosDesktopBridgeResource.js
**/*[!.module].ts
📄 CodeRabbit inference engine (AGENTS.md)
Utility files must use camelCase naming (e.g., formatDate.ts)
Files:
tests/unit/bootstrap/afterPackPackagingProfile.test.tstests/unit/bootstrap/prepareEvaosDesktopBridgeResource.test.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use@icon-park/reactfor icons
TypeScript strict mode must be enabled; no 'any' types, no implicit returns
Use path aliases:@/,@process/,@renderer/* for imports
Prefer 'type' over 'interface' for type definitions (per Oxlint config)
Files:
tests/unit/bootstrap/afterPackPackagingProfile.test.tstests/unit/bootstrap/prepareEvaosDesktopBridgeResource.test.ts
**/*.{test,spec}.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Vitest 4 for testing with a coverage target of at least 80%
Files:
tests/unit/bootstrap/afterPackPackagingProfile.test.tstests/unit/bootstrap/prepareEvaosDesktopBridgeResource.test.ts
🪛 GitHub Actions: PR Checks / 2_Unit Tests (macos-14).txt
tests/unit/bootstrap/afterPackPackagingProfile.test.ts
[warning] 1-1: afterPackPackagingProfile test logs warning: "app.asar.unpacked not found" (appears multiple times across packaging profiles).
🔇 Additional comments (4)
scripts/prepareEvaosDesktopBridgeResource.js (1)
192-201: LGTM!Also applies to: 426-426
tests/unit/bootstrap/prepareEvaosDesktopBridgeResource.test.ts (1)
154-162: LGTM!scripts/afterPack.js (1)
12-16: LGTM!Also applies to: 169-192, 267-300
tests/unit/bootstrap/afterPackPackagingProfile.test.ts (1)
13-13: LGTM!Also applies to: 57-72, 129-148, 158-158, 211-371
1 participant
Why
Closes #461.
The Workbench build-cost sprint depends on a cheap smoke lane that still proves the packaged runtime resource shape. That lane is currently broken:
Workbench Functional SmokeverifiesContents/Resources/Bridge/evaos-desktop-bridge, butmainno longer prepares or shipsresources/Bridge.This PR restores the producer side of that contract instead of weakening the verifier. Without this, future agents could make PR iteration faster by skipping exactly the resource proof that catches Mac-control/runtime packaging regressions.
What changed
scripts/prepareEvaosDesktopBridgeResource.jswith default pinned bridge ref207f6528461ecae51c39efd2654733c1b07d39a4.thin-shellpackage builds and shipsresources/Bridge -> Contents/Resources/Bridge.afterPackBridge verification while preserving current AionCore/hub/no-acp checks.Thin App Smokesecret-free by allowing an explicit diagnostic Bridge placeholder only for package-shape smoke.Workbench Functional SmokeuseEVAOS_PACKAGING_PROFILE=functional-smoke, disable local/default bridge candidates, require a private source token, and verify the bundled Bridge manifest matches the requested ref.Release boundary
thin-shellremains UI-only.Local validation
Working directory:
/Volumes/LEXAR/repos/worktrees/evaos-gui-bridge-resource-proof-461node --check scripts/prepareEvaosDesktopBridgeResource.js && node --check scripts/build-with-builder.js && node --check scripts/afterPack.jsgit diff --checkactionlint .github/workflows/pr-checks.yml .github/workflows/workbench-functional-smoke.ymlbun run format:check./node_modules/.bin/vitest run tests/unit/bootstrap/buildWithBuilder.test.ts tests/unit/bootstrap/afterPackPackagingProfile.test.ts tests/unit/bootstrap/prepareEvaosDesktopBridgeResource.test.ts tests/unit/process/evaosPrCheckPlan.test.ts --maxWorkers=1Note: the fresh worktree used a local ignored
node_modulessymlink to/Volumes/LEXAR/repos/evaOS-GUI/node_modulesfor focused validation only.Remote proof still required
Workbench Functional Smokeon this PR head with bridge ref207f6528461ecae51c39efd2654733c1b07d39a4passes and records Bridge/AionCore/hub resource evidence.Summary by CodeRabbit
bridge_refinput for functional-smoke workflows with pinned-ref validation and controlled diagnostic placeholder behavior.manifest.json, placeholder rules, pinned SHA matching).resources/Bridgeand improve workflow audit/build robustness; refine thin-shell resource handling.