Conversation
* feat: add daemon mode and --json output for agent-friendly usage - Add --daemon flag to offckb node to run the devnet in the background - Add global --json flag for structured JSON log output - Write daemon PID and logs to devnet data folder - Add tests for logger JSON mode and node daemon spawning - Update README with daemon and --json usage Fixes #446 Co-Authored-By: Claude <noreply@anthropic.com> * feat: add node stop command Add offckb node stop to terminate the devnet daemon started by offckb node --daemon. It reads the PID file, sends SIGTERM, waits for graceful shutdown, falls back to SIGKILL if needed, and removes the PID file. Also add tests and update README to mention node stop instead of manual kill. Co-Authored-By: Claude <noreply@anthropic.com> * ci: fix formatting, windows test paths, and add changeset - Run prettier so lint job's git diff --exit-code passes - Use path.join in node-command tests so Windows path assertions pass - Add changeset for daemon mode, --json output, and node stop command Co-Authored-By: Claude <noreply@anthropic.com> * chore: change changeset level from minor to patch Co-Authored-By: Claude <noreply@anthropic.com> * fix(node): harden daemon lifecycle per test team review - Reject duplicate daemon starts when PID file points to a live process. - Verify target process identity before stopNode sends signals. - Harden CLI entry resolution with OFFCKB_CLI_PATH fallback and file validation. - Fix stopNode race condition between existsSync/readFileSync by reading once. - Always clean up PID file even when signal delivery fails. - Handle spawn sync exceptions, missing child.pid, and log dir creation failures. - Use taskkill on Windows to terminate the daemon process tree. - Distinguish EPERM vs ESRCH and surface clear error messages. Co-Authored-By: Claude <noreply@anthropic.com> * test(node): make daemon lifecycle tests platform-aware for Windows CI - Assert the resolved script path so Windows backslash normalization passes. - Mock WMIC output with the CommandLine= prefix required by the parser. - Normalize process.platform to linux in stop tests for deterministic POSIX signal assertions; the Windows taskkill path is covered by the daemon spawn tests and integration tests. Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com>
* feat: add UDT balance and transfer CLI commands Add offckb udt-balance and udt-transfer commands leveraging the CCC SDK. Supports both SUDT and xUDT via --kind. - udt-balance: query UDT balance for an address - udt-transfer: transfer UDT amount to an address with change output Closes #445 Co-Authored-By: Claude <noreply@anthropic.com> * refactor: reuse balance/transfer for UDT and add issue/destroy commands - Reuse existing balance/transfer commands via --udt-type-args/--udt-kind flags - Make balance default show CKB plus detected SUDT/xUDT balances - Replace udt-balance/udt-transfer with udt issue/destroy subcommands - Add detectUdtBalances, udtIssue, and udtDestroy to CKB SDK - Update tests for the revised CLI Co-Authored-By: Claude <noreply@anthropic.com> * chore: add changeset and fix formatting for UDT CLI refactor - Add minor changeset for the new UDT issue/destroy commands and balance/transfer reuse. - Apply prettier formatting to src/cmd/transfer.ts. Co-Authored-By: Claude <noreply@anthropic.com> * fix(udt): address PR review comments - Use minimal cell capacity (capacity: 0) instead of hardcoded 61 CKB for UDT outputs - Add missing cell deps in udtTransfer - Reject full UDT destroy to avoid potential script rejection - Add max input cell limit for udtDestroy - Skip corrupted UDT cells in balance detection instead of failing - Remove hard 500-cell cap on UDT balance scan (now 1000 with warning) - Add input validation: amount, type args length, UDT kind enum - Warn when --type-args is provided for SUDT issue - Remove process.exit(0) from balance command - Support filtering balance by --udt-kind alone - Add CLI enum choices for --udt-kind and --kind - Add SDK-level UDT tests and expand validator tests Co-Authored-By: Claude <noreply@anthropic.com> * fix(udt): address code review comments from review squad - Extract getUdtScriptInfo helper to unify SUDT/xUDT script/cellDeps handling - Filter detectUdtBalances by UDT type script with prefix search instead of scanning all cells - Add --no-udt flag and parallel CKB/UDT queries in balance command - Restore process.exit(0) in balanceOf to prevent CLI hang - Unify UDT kind CLI flag to --udt-kind across balance/transfer/udt commands - Move UdtKind type to src/type/base.ts and reuse in validator - Add u128 upper bound check to validateUdtAmount - Add logTxSuccess helper to remove duplicated testnet/devnet success logging - Fix ckb.udt.test.ts test title/content mismatch and cover SUDT type-args warning - Update tests for renamed udtKind option and process.exit mocking Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com>
* feat(cmd): add status command with ckb-tui v0.1.3 Integrates ckb-tui to provide a terminal UI for monitoring CKB network status from a local node. Changes: - Add CKBTui class with automatic binary download/install for v0.1.3 - Add status command with RPC port connectivity check - Update settings schema with tools.rootFolder and ckbTui.version - Register status command in CLI with network validation Closes RET-161 * fix: address review feedback (round 1) - Replace execSync shell interpolation with spawnSync array args (CRIT #1) - Add path validation for tools.rootFolder bounded to dataPath (CRIT #3) - Add SHA-256 checksum verification for downloaded binaries (CRIT #2) - Use -fsSL flags on curl, add timeouts, use fs.chmodSync, findFileInFolder - Fix deepMerge mutation by cloning defaultSettings before merge - Add settings validation for tools.rootFolder, ckbTui.version, proxy types - Fix status help text typo and use validateNetworkOpt() consistently - Replace nested ternary with lookup table, propagate exit code * fix: add non-TTY guard to status command to prevent CI/pipe hangs (round 2)
* chore(deps): resolve Dependabot security alerts Apply pnpm overrides to bump vulnerable transitive dependencies: - qs 6.15.0 -> 6.15.2 - ip-address 10.1.0 -> 10.1.1 - js-yaml 3.14.2 -> 3.15.0 / 4.1.1 -> 4.2.0 - @babel/core 7.28.6 -> 7.29.7 - @eslint/plugin-kit 0.2.8 -> 0.3.4 - brace-expansion 5.0.5 -> 5.0.6 elliptic remains unfixed because patched version >=6.6.2 is not yet published on npm. Co-Authored-By: Claude <noreply@anthropic.com> * docs(deps): add advisory comments and fix minimumReleaseAgeExclude syntax - Document each pnpm override with its GHSA ID and removal condition. - Split js-yaml@3.15.0 || 4.2.0 into two exact minimumReleaseAgeExclude entries. - Quote all minimumReleaseAgeExclude entries consistently. Co-Authored-By: Claude <noreply@anthropic.com> * chore(changeset): add changeset for dependency security fixes Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com>
* feat: add offckb devnet fork for mainnet/testnet chain data Add 'offckb devnet fork' implementing the Devnet From Existing Data flow: copy an existing mainnet/testnet data directory into the local devnet, import and patch the source chain spec for local Dummy mining, verify the genesis hash, and boot the first run with --skip-spec-check --overwrite-spec automatically. - src/devnet/fork.ts: fork command (source validation, chain detection, spec fetch/cache or --spec-file, ckb init --import-spec, dev.toml and ckb.toml alignment with offckb's devnet, fork.json state) - src/cmd/node.ts: first run of a fork adds the spec flags and clears them once the node answers RPC - system scripts resolve from the chain's own list-hashes and, when the genesis hash identifies a mainnet/testnet fork, supplement post-genesis deployments (sudt/xudt/omnilock/spore/...) from the static records; the devnet ccc client uses the ckb prefix on a mainnet fork - debug: fall back to get_transaction when the tx json is not in the local proxy cache; fix buildTxFileOptionBy checking the wrong path - tx dumper: embed full header objects in mock_info.header_deps instead of bare hashes (ckb-debugger rejects the latter) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix: address review findings on devnet fork - fork: run ckb init via execFileSync argv (no shell interpolation) - fork: roll back partial configPath when the data copy fails - fork: cross-check the source dir's own genesis (list-hashes) against the imported spec so a mislabeled --source/--spec-file is rejected before committing the fork - node: clear firstRunPending only when the spawned node answers with the fork's genesis; abort the poll when the process exits, and never trust an unrelated node occupying the port - json-rpc: reject on response stream error / premature close instead of hanging until the request timeout - docs: caution that fork transactions spending mainnet cells are replayable on mainnet (CKB has no chain id) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
#454) Bumps the npm_and_yarn group with 1 update in the / directory: [adm-zip](https://github.com/cthackers/adm-zip). Updates `adm-zip` from 0.5.16 to 0.6.0 - [Release notes](https://github.com/cthackers/adm-zip/releases) - [Changelog](https://github.com/cthackers/adm-zip/blob/master/history.md) - [Commits](cthackers/adm-zip@v0.5.16...v0.6.0) --- updated-dependencies: - dependency-name: adm-zip dependency-version: 0.6.0 dependency-type: direct:production dependency-group: npm_and_yarn ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
* fix canary devrel reliability issues * fix fork copy isolation on windows * remove implicit fork source discovery * address PR review feedback * address follow-up review feedback * fix daemon test on Windows runners * complete failed daemon cleanup * harden daemon startup recovery
📝 WalkthroughSummary by CodeRabbit
WalkthroughOffCKB 0.4.9 adds daemon lifecycle management, structured JSON output, devnet forking, readiness and replay-safety checks, UDT commands and SDK support, status monitoring, hardened tooling, expanded tests, and updated release documentation. ChangesCLI, daemon, and fork runtime
UDT and transaction operations
Tooling and reliability
Validation and release support
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
Comment |
|
❌ Missing Changeset Please add a changeset describing your changes: pnpm changesetIf your changes do not need a version bump (docs, CI, refactoring), For dependency updates, use the |
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
src/cmd/config.ts (1)
48-76: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winOverly broad try/catch mislabels unrelated errors as validation failures.
Both
set+proxy(lines 48-59) andset+ckbVersion(lines 61-76) wrapreadSettings()/writeSettings()inside the same try that validates input, so awriteSettingsI/O failure gets reported asinvalid proxyURL: .../invalid version value: ..., misleading users about the real cause. TheckbVersionbranch additionally re-wraps its own explicit validation-failure throw, producing a doubled prefix (invalid version value: invalid version value, ...).🛠️ Proposed fix — narrow the try/catch to only the validation call
case ConfigItem.proxy: { if (value == null) throw new Error('No proxyUrl!'); - try { - const proxy = Request.parseProxyUrl(value); - const settings = readSettings(); - settings.proxy = proxy; - return writeSettings(settings); - } catch (error: unknown) { - throw new Error(`invalid proxyURL: ${(error as Error).message}`); - } + let proxy; + try { + proxy = Request.parseProxyUrl(value); + } catch (error: unknown) { + throw new Error(`invalid proxyURL: ${(error as Error).message}`); + } + const settings = readSettings(); + settings.proxy = proxy; + return writeSettings(settings); } case ConfigItem.ckbVersion: { const settings = readSettings(); - try { - if (isValidVersion(value)) { - const version = extractVersion(value!); - settings.bins.defaultCKBVersion = version; - return writeSettings(settings); - } else { - throw new Error( - `invalid version value, ${value}. Check available versions on https://github.com/nervosnetwork/ckb/tags`, - ); - } - } catch (error: unknown) { - throw new Error(`invalid version value: ${(error as Error).message}`); - } + if (!isValidVersion(value)) { + throw new Error( + `invalid version value, ${value}. Check available versions on https://github.com/nervosnetwork/ckb/tags`, + ); + } + const version = extractVersion(value!); + settings.bins.defaultCKBVersion = version; + return writeSettings(settings); }🤖 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 `@src/cmd/config.ts` around lines 48 - 76, Restrict the try/catch blocks in the ConfigItem.proxy and ConfigItem.ckbVersion branches to only the relevant validation/parsing calls: Request.parseProxyUrl and the ckb version validation/extraction logic. Move readSettings and writeSettings outside those catches so their errors propagate unchanged, and ensure ckbVersion validation errors receive only one “invalid version value” prefix.src/cfg/setting.ts (1)
105-118: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winFallback paths still hand out the shared
defaultSettingsreference.The
deepCloneat Line 112 protects the merge path, but the two fallbacks (missing file / validation failure) return the module-leveldefaultSettingsobject directly. Callers mutate the returned settings in place (e.g.Configdoessettings.proxy = proxybeforewriteSettings), so on first run or on a validation error the shared default is corrupted for the rest of the process. Return a clone here too.🛡️ Proposed fix
} else { - return defaultSettings; + return deepClone(defaultSettings); } } catch (error) { logger.error('Error reading settings:', error); - return defaultSettings; + return deepClone(defaultSettings); }🤖 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 `@src/cfg/setting.ts` around lines 105 - 118, Update readSettings so both fallback paths—when configPath is absent and when reading or validation throws—return a deep clone of defaultSettings rather than the shared object. Reuse the existing deepClone helper and preserve the merged clone behavior for valid configuration files.src/cmd/transfer-all.ts (1)
19-27: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy liftOther (CWE-294): Authentication Bypass by Capture-replay
Reachability: External
● Entry src/cli.ts:183 transferAll │ ▼ ● Sink src/cmd/transfer-all.ts
transfer-allbypasses the mainnet-fork replay-safety enforcement applied elsewhere.
transferanddepositcallvalidateMainnetForkSigning(...), which blocks non-built-in keys and returns arejectInputsAtOrBeforeBlockboundary that the SDK enforces viaassertInputsCreatedAfter. HeretransferAllonly calls the warn-onlywarnIfMainnetForkSigningand invokesckb.transferAll({ toAddress, privateKey })with no boundary. Sincetransfer-allsweeps the entire balance — the path most likely to select copied pre-fork mainnet cells — this leaves the highest-risk command with no replay protection.Route it through
validateMainnetForkSigningand thread the returned boundary intotransferAll(SDK support required, mirroringtransfer).🤖 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 `@src/cmd/transfer-all.ts` around lines 19 - 27, Replace the warn-only warnIfMainnetForkSigning call in transfer-all with validateMainnetForkSigning, capture its rejectInputsAtOrBeforeBlock boundary, and pass that boundary through the CKB transferAll invocation. Update the SDK transferAll path as needed so it enforces the boundary via assertInputsCreatedAfter, mirroring transfer and preserving replay protection for swept inputs.README.md (1)
23-42: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winTOC missing the new "Fork Mainnet/Testnet" section.
The "Get started" TOC lists steps 1-6 only, but a new step 7 heading (
### 7. Fork Mainnet/Testnet Into Your Devnet {#fork-devnet}) was added later in the doc (line 389). The TOC entry for it is missing, so readers can't navigate to it from the table of contents.📝 Proposed fix
- [6. Tweak Devnet Config {`#tweak-devnet-config`}](`#6-tweak-devnet-config-tweak-devnet-config`) + - [7. Fork Mainnet/Testnet Into Your Devnet {`#fork-devnet`}](`#7-fork-mainnettestnet-into-your-devnet-fork-devnet`)🤖 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 `@README.md` around lines 23 - 42, Add the missing step 7 “Fork Mainnet/Testnet Into Your Devnet” entry under the “Get started” table-of-contents section, linking it to the existing fork-devnet anchor and preserving the ordering after step 6.
🧹 Nitpick comments (4)
src/cmd/devnet-config.ts (1)
73-77: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winPreserve the original error object
src/cmd/devnet-config.ts:73-77
Attach the tip to the caughtInitializationErrorand rethrow it directly so the original stack and name stay intact. For non-Errorvalues, keep normalizing tonew Error(String(error)).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/cmd/devnet-config.ts` around lines 73 - 77, Update the error handling in the devnet configuration flow to append the initialization tip to an InitializationError while rethrowing that same error object, preserving its name and stack. For non-Error values, continue normalizing them to a new Error using their string representation.src/sdk/ckb.ts (1)
280-335: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
scannedis a shared budget across both scans; XUDT balances can be silently dropped.
scannedis declared once and reused by the SUDT scan (Line 331) and the XUDT scan (Line 333). If the SUDT scan alone reachesmaxCells, the XUDT scan starts already at the cap, immediately warns, and returns no XUDT balances — so a lock with many SUDT cells reports incomplete UDT balances while only emitting a generic warning. Consider making the cap per-kind by declaring the counter insidescan.♻️ Per-kind scan budget
- let scanned = 0; - const scan = async (scriptInfo: UdtScriptInfo, kind: UdtKind) => { + let scanned = 0; for await (const cell of this.client.findCells(🤖 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 `@src/sdk/ckb.ts` around lines 280 - 335, Make the maxCells budget local to each invocation of the scan function by moving the scanned counter inside scan. Preserve the existing stopping and warning behavior so SUDT and XUDT scans each independently process up to maxCells cells.src/cmd/node.ts (1)
383-404: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winInjection (CWE-78): Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection')
Reachability: Internal
● Entry src/cli.ts:268 devnetInfo │ ▼ ● Hop src/cmd/devnet-info.ts │ ▼ ● Hop src/devnet/fork.ts:302 migrationNeededFromExitCode │ ▼ ● Hop src/cmd/status.ts │ ▼ ● Hop src/devnet/readiness.ts:69 waitForNodeReady │ ▼ ● Sink src/cmd/node.tsPrefer
execFileoverexecfor the command-line lookup.
pidis validated as a positive integer on every reachable path (guarded byisProcessAlive/Number.isInteger), so this isn't actually injectable — but switching toexecFilewith an argument array removes the interpolated shell command, silences the two static-analysis ERRORs here, and stays safe if a future refactor loosens the pid validation.♻️ Proposed refactor
-function getProcessCommandLine(pid: number): Promise<string | null> { - return new Promise((resolve) => { - if (process.platform === 'win32') { - exec(`wmic process where ProcessId=${pid} get CommandLine /format:list`, (error, stdout) => { - if (error) { - resolve(null); - return; - } - const match = stdout.match(/CommandLine=(.+)/); - resolve(match ? match[1].trim() : null); - }); - } else { - exec(`ps -p ${pid} -o args=`, (error, stdout) => { - if (error) { - resolve(null); - return; - } - resolve(stdout.trim()); - }); - } - }); -} +function getProcessCommandLine(pid: number): Promise<string | null> { + return new Promise((resolve) => { + const [cmd, args] = + process.platform === 'win32' + ? ['wmic', ['process', 'where', `ProcessId=${pid}`, 'get', 'CommandLine', '/format:list']] + : ['ps', ['-p', String(pid), '-o', 'args=']]; + execFile(cmd, args as string[], (error, stdout) => { + if (error) { + resolve(null); + return; + } + if (process.platform === 'win32') { + const match = stdout.match(/CommandLine=(.+)/); + resolve(match ? match[1].trim() : null); + } else { + resolve(stdout.trim()); + } + }); + }); +}Update the import to
import { execFile, spawn, ChildProcess } from 'child_process';.🤖 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 `@src/cmd/node.ts` around lines 383 - 404, Replace exec with execFile in getProcessCommandLine, importing execFile alongside the existing child_process symbols. Invoke WMIC and ps using executable names plus argument arrays rather than interpolated shell command strings, while preserving the current callbacks, error handling, and command-line parsing behavior.Source: Linters/SAST tools
src/util/validator.ts (1)
153-163: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winDuplicate branch, and the strict 32-byte check may reject valid xUDT args.
Both branches evaluate the identical condition
byteLength !== 32; only the message differs, so they can collapse into one check. Separately, xUDT type-script args are commonly the owner lock hash (32 bytes) plus an optional 4-byte flags field and extension data, whereas SUDT is exactly 32 bytes. Enforcing exactly 32 forxudtwill reject transfers/destroys/balance lookups against pre-existing xUDT tokens that carry extension args. Please confirm whether extended xUDT args must be supported here.♻️ If only 32-byte args are intended
- if (kind === 'sudt' && byteLength !== 32) { - throw new Error(`invalid SUDT type args length: expected 32 bytes, got ${byteLength}`); - } - if (kind === 'xudt' && byteLength !== 32) { - throw new Error(`invalid xUDT type args length: expected 32 bytes, got ${byteLength}`); - } + if (byteLength !== 32) { + throw new Error( + `invalid ${kind === 'sudt' ? 'SUDT' : 'xUDT'} type args length: expected 32 bytes, got ${byteLength}`, + ); + }xUDT type script args format owner lock hash flags extension length🤖 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 `@src/util/validator.ts` around lines 153 - 163, Update validateUdtTypeArgs so SUDT retains the exact 32-byte validation while xUDT accepts the supported owner-hash, flags, and extension argument lengths, including pre-existing extended xUDT tokens. Remove the duplicate identical branch and keep error messages specific to the validation that remains.
🤖 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 `@src/cmd/debug.ts`:
- Around line 91-117: The transaction hash must be validated before it is
interpolated into cache paths. In the flow that calls
buildTransactionJsonFilePath and buildDebugFullTransactionFilePath, reject any
--tx-hash that is not exactly an 0x-prefixed 32-byte hexadecimal value, and only
build or access those paths after validation.
In `@src/cmd/deploy.ts`:
- Around line 30-33: The deploy flow currently only warns about Mainnet-fork
signing and does not enforce the fork replay guard. Replace
warnIfMainnetForkSigning in deploy with validateMainnetForkSigning, retain its
returned fork block, and thread that value through the deploy
transaction/input-selection path so inputs at or before the fork boundary are
rejected, matching transfer/deposit behavior.
In `@src/cmd/node.ts`:
- Around line 216-227: Do not use the first successful RPC response in the
fork-boundary flow to call markForkFirstRunComplete; ensure forkBlockNumber
comes from validated state belonging to the spawned CKB process, or defer
clearing firstRunPending until that process has definitively bound the RPC
listener. Preserve the genesis validation and keep validateMainnetForkSigning()
aligned with the trusted boundary.
In `@src/cmd/udt.ts`:
- Around line 32-82: Update udtIssue and udtDestroy to use replay-safe fork
validation rather than only warnIfMainnetForkSigning: propagate the fork
boundary, reject non-built-in private keys unless allowMainnetReplayRisk is
enabled, and preserve the built-in-key exception. Before the CKB SDK sends each
transaction, apply the same assertInputsCreatedAfter gating used by the transfer
flow so inputs created before the fork are rejected.
In `@src/devnet/fork.ts`:
- Around line 270-273: Update the source-copy loop in the fork flow to inspect
each source entry with lstatSync or an equivalent non-following containment
check before fs.cpSync; reject symlinked entries, especially data/db, and ensure
resolved targets remain within configPath/sourceData before copying. Preserve
copying of valid regular files and directories.
In `@src/tools/ckb-tui.ts`:
- Around line 11-12: Align the download timeout constants and the curl timeout
used by the download flow so the outer spawnSync timeout does not terminate
before curl’s --max-time limit. Update the timeout referenced by the relevant
spawnSync call and curl arguments together, preserving the existing extraction
timeout and download behavior.
- Around line 182-188: Update the installation move step around fs.renameSync to
handle EXDEV by copying extractedBinary to this.binaryPath and then removing the
source when the rename crosses filesystems; rethrow other rename errors
unchanged. Preserve the existing Unix chmod behavior after either move path
completes.
In `@tests/node-command.test.ts`:
- Around line 80-83: Remove the unused logFile binding from the test setup,
unless the test is intended to verify log-file opening; in that case, add an
assertion using logFile with mockOpenSync. Keep the existing dataPath and
pidFile behavior unchanged.
---
Outside diff comments:
In `@README.md`:
- Around line 23-42: Add the missing step 7 “Fork Mainnet/Testnet Into Your
Devnet” entry under the “Get started” table-of-contents section, linking it to
the existing fork-devnet anchor and preserving the ordering after step 6.
In `@src/cfg/setting.ts`:
- Around line 105-118: Update readSettings so both fallback paths—when
configPath is absent and when reading or validation throws—return a deep clone
of defaultSettings rather than the shared object. Reuse the existing deepClone
helper and preserve the merged clone behavior for valid configuration files.
In `@src/cmd/config.ts`:
- Around line 48-76: Restrict the try/catch blocks in the ConfigItem.proxy and
ConfigItem.ckbVersion branches to only the relevant validation/parsing calls:
Request.parseProxyUrl and the ckb version validation/extraction logic. Move
readSettings and writeSettings outside those catches so their errors propagate
unchanged, and ensure ckbVersion validation errors receive only one “invalid
version value” prefix.
In `@src/cmd/transfer-all.ts`:
- Around line 19-27: Replace the warn-only warnIfMainnetForkSigning call in
transfer-all with validateMainnetForkSigning, capture its
rejectInputsAtOrBeforeBlock boundary, and pass that boundary through the CKB
transferAll invocation. Update the SDK transferAll path as needed so it enforces
the boundary via assertInputsCreatedAfter, mirroring transfer and preserving
replay protection for swept inputs.
---
Nitpick comments:
In `@src/cmd/devnet-config.ts`:
- Around line 73-77: Update the error handling in the devnet configuration flow
to append the initialization tip to an InitializationError while rethrowing that
same error object, preserving its name and stack. For non-Error values, continue
normalizing them to a new Error using their string representation.
In `@src/cmd/node.ts`:
- Around line 383-404: Replace exec with execFile in getProcessCommandLine,
importing execFile alongside the existing child_process symbols. Invoke WMIC and
ps using executable names plus argument arrays rather than interpolated shell
command strings, while preserving the current callbacks, error handling, and
command-line parsing behavior.
In `@src/sdk/ckb.ts`:
- Around line 280-335: Make the maxCells budget local to each invocation of the
scan function by moving the scanned counter inside scan. Preserve the existing
stopping and warning behavior so SUDT and XUDT scans each independently process
up to maxCells cells.
In `@src/util/validator.ts`:
- Around line 153-163: Update validateUdtTypeArgs so SUDT retains the exact
32-byte validation while xUDT accepts the supported owner-hash, flags, and
extension argument lengths, including pre-existing extended xUDT tokens. Remove
the duplicate identical branch and keep error messages specific to the
validation that remains.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 1aa213f6-0d19-43b1-b28d-51358a12c532
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (63)
CHANGELOG.mdREADME.mdpackage.jsonpnpm-workspace.yamlsrc/cfg/setting.tssrc/cli.tssrc/cmd/accounts.tssrc/cmd/balance.tssrc/cmd/clean.tssrc/cmd/config.tssrc/cmd/create.tssrc/cmd/debug.tssrc/cmd/deploy.tssrc/cmd/deposit.tssrc/cmd/devnet-config.tssrc/cmd/devnet-fork.tssrc/cmd/devnet-info.tssrc/cmd/node.tssrc/cmd/status.tssrc/cmd/system-scripts.tssrc/cmd/transfer-all.tssrc/cmd/transfer.tssrc/cmd/udt.tssrc/devnet/fork.tssrc/devnet/readiness.tssrc/node/init-chain.tssrc/node/install.tssrc/scripts/const.tssrc/scripts/private.tssrc/scripts/util.tssrc/sdk/ckb.tssrc/tools/ckb-tui.tssrc/tools/ckb-tx-dumper.tssrc/type/base.tssrc/util/fork-safety.tssrc/util/fs.tssrc/util/json-rpc.tssrc/util/link.tssrc/util/logger.tssrc/util/private-key.tssrc/util/validator.tstests/accounts.test.tstests/ckb-tui-checksum.test.tstests/ckb-tx-dumper.test.tstests/debug-tx-file.test.tstests/deposit.test.tstests/devnet-config-command.test.tstests/devnet-fork.test.tstests/devnet-info.test.tstests/fork-safety.test.tstests/init-chain.test.tstests/json-rpc.test.tstests/logger.test.tstests/node-command.test.tstests/node-supervisor.test.tstests/private-key.test.tstests/readiness-warning.test.tstests/readiness.test.tstests/sdk/ckb.udt.test.tstests/status.test.tstests/system-scripts.test.tstests/udt.test.tstests/validator.test.ts
| if (!fs.existsSync(txJsonFilePath)) { | ||
| await fetchTransactionIntoCache(rpc, txHash, txJsonFilePath); | ||
| } | ||
| await dumpTransaction({ rpc, txJsonFilePath, outputFilePath }); | ||
| } | ||
| const opt = `--tx-file ${encodeBinPathForTerminal(outputFilePath)}`; | ||
| return opt; | ||
| } | ||
|
|
||
| // Fallback for transactions that never went through the local RPC proxy | ||
| // (e.g. historical transactions on a forked devnet): pull the transaction | ||
| // from the node and cache it in the same JSON-RPC format the proxy stores. | ||
| async function fetchTransactionIntoCache(rpc: string, txHash: string, txJsonFilePath: string) { | ||
| logger.info(`Transaction ${txHash} not found in local cache, fetching from ${rpc} ..`); | ||
| const result = await callJsonRpc(rpc, 'get_transaction', [txHash]).catch((error: Error) => { | ||
| throw new Error(`Failed to fetch transaction ${txHash} from ${rpc}: ${error.message}`); | ||
| }); | ||
| if (!result?.transaction) { | ||
| throw new Error( | ||
| `Transaction ${txHash} not found on ${rpc}. ` + | ||
| `Check the hash and the --network option, or send the transaction through the offckb RPC proxy first.`, | ||
| ); | ||
| } | ||
| fs.mkdirSync(path.dirname(txJsonFilePath), { recursive: true }); | ||
| fs.writeFileSync(txJsonFilePath, JSON.stringify(result.transaction, null, 2)); | ||
| } | ||
|
|
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n "buildTransactionJsonFilePath|buildDebugFullTransactionFilePath" src/cmd/debug.ts -B2 -A6
rg -n "txHash" src/cli.ts -B3 -A3Repository: ckb-devrel/offckb
Length of output: 2987
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "## src/cmd/debug.ts"
cat -n src/cmd/debug.ts | sed -n '1,180p'
echo
echo "## search for tx hash validation helpers"
rg -n "0x[0-9a-fA-F]{64}|isValid.*hash|validate.*hash|txHash.*regex|hash.*regex|parseTxHash|transaction hash" srcRepository: ckb-devrel/offckb
Length of output: 13906
Path Traversal (CWE-22): Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal')
Reachability: External
Validate --tx-hash before using it in cache paths. buildTransactionJsonFilePath and buildDebugFullTransactionFilePath interpolate the CLI value directly, so a crafted --tx-hash with path separators can escape the cache directory and overwrite arbitrary files. Restrict it to the expected 0x-prefixed 32-byte hex format before building the file paths.
🧰 Tools
🪛 ast-grep (0.44.1)
[warning] 114-114: 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(txJsonFilePath, JSON.stringify(result.transaction, null, 2))
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename-typescript)
🤖 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 `@src/cmd/debug.ts` around lines 91 - 117, The transaction hash must be
validated before it is interpolated into cache paths. In the flow that calls
buildTransactionJsonFilePath and buildDebugFullTransactionFilePath, reject any
--tx-hash that is not exactly an 0x-prefixed 32-byte hexadecimal value, and only
build or access those paths after validation.
Source: Linters/SAST tools
| const privateKey = resolvePrivateKey(opt, deployerAccount.privkey); | ||
| warnIfMainnetForkSigning(network, privateKey); | ||
| await warnIfForkIndexerIsBehind(network); | ||
| const ckb = new CKB({ network }); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== locate relevant symbols ==\n'
rg -n "warnIfMainnetForkSigning|validateMainnetForkSigning|deployBinaries|fork boundary|Mainnet-derived devnet|non-built-in private keys|copied Mainnet cell" src -S
printf '\n== inspect src/cmd/deploy.ts ==\n'
wc -l src/cmd/deploy.ts
sed -n '1,220p' src/cmd/deploy.ts
printf '\n== inspect related helpers ==\n'
for f in $(rg -l "warnIfMainnetForkSigning|validateMainnetForkSigning" src); do
printf '\n-- %s --\n' "$f"
wc -l "$f"
sed -n '1,260p' "$f"
done
printf '\n== inspect deployBinaries references ==\n'
rg -n "deployBinaries\(" src -S
printf '\n== inspect transaction input / creation block handling ==\n'
rg -n "creation block|block number|created at|fork boundary|since" src -SRepository: ckb-devrel/offckb
Length of output: 23784
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== inspect src/deploy/index.ts ==\n'
wc -l src/deploy/index.ts
sed -n '1,260p' src/deploy/index.ts
printf '\n== inspect relevant signing guards in src/sdk/ckb.ts ==\n'
sed -n '360,470p' src/sdk/ckb.ts
printf '\n== inspect deploy-related methods in src/sdk/ckb.ts ==\n'
rg -n "deploy|rejectInputsAtOrBeforeBlock|validateMainnetForkSigning|allowMainnetReplayRisk|build.*deploy" src/sdk/ckb.ts -n -SRepository: ckb-devrel/offckb
Length of output: 10609
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '520,660p' src/sdk/ckb.tsRepository: ckb-devrel/offckb
Length of output: 5220
Other (CWE-294): Authentication Bypass by Capture-replay
Reachability: External
● Entry
src/cli.ts:96
DeployOptions
│
▼
● Sink
src/cmd/deploy.ts
Mainnet-fork deploy needs the same replay guard as transfer/deposit
src/cmd/deploy.ts only warns on Mainnet forks. That leaves deploys free to use non-built-in keys and to spend copied Mainnet inputs without any fork-boundary check, so a tx selected from a forked devnet can still be replayable on Mainnet. Switch to validateMainnetForkSigning and thread the returned fork block into the deploy path so input origins are rejected at or before the fork boundary.
🤖 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 `@src/cmd/deploy.ts` around lines 30 - 33, The deploy flow currently only warns
about Mainnet-fork signing and does not enforce the fork replay guard. Replace
warnIfMainnetForkSigning in deploy with validateMainnetForkSigning, retain its
returned fork block, and thread that value through the deploy
transaction/input-selection path so inputs at or before the fork boundary are
rejected, matching transfer/deposit behavior.
| const genesisHash = String(await callJsonRpc(rpcUrl, 'get_block_hash', ['0x0'], 5000)).toLowerCase(); | ||
| if (genesisHash !== expectedGenesisHash.toLowerCase()) { | ||
| logger.warn( | ||
| `A node is answering at ${rpcUrl} but reports a different genesis (${genesisHash}); ` + | ||
| 'leaving the first-run flags in place.', | ||
| ); | ||
| return; | ||
| } | ||
| // The miner has not started yet, so this tip is the exact boundary | ||
| // between copied public-chain state and cells mined on the local fork. | ||
| const forkBlockNumber = BigInt(String(await callJsonRpc(rpcUrl, 'get_tip_block_number', [], 5000))).toString(); | ||
| markForkFirstRunComplete(configPath, forkBlockNumber); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n### node.ts around target lines\n'
sed -n '180,260p' src/cmd/node.ts | cat -n
printf '\n### locate first-run helpers/usages\n'
rg -n "markForkFirstRunComplete|firstRunPending|forkBlockNumber|get_tip_block_number|get_block_hash|expectedGenesisHash|rpcUrl" src -S
printf '\n### outline candidate file if available\n'
ast-grep outline src/cmd/node.ts --view expanded || trueRepository: ckb-devrel/offckb
Length of output: 11373
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n### src/devnet/fork.ts relevant sections\n'
sed -n '1,130p' src/devnet/fork.ts | cat -n
printf '\n--- later sections around fork creation ---\n'
sed -n '340,560p' src/devnet/fork.ts | cat -n
printf '\n### src/cmd/node.ts relevant sections around daemon startup\n'
sed -n '1,180p' src/cmd/node.ts | cat -n
sed -n '480,560p' src/cmd/node.ts | cat -n
printf '\n### src/devnet/readiness.ts\n'
sed -n '1,120p' src/devnet/readiness.ts | cat -nRepository: ckb-devrel/offckb
Length of output: 31944
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n### fork safety consumers\n'
rg -n "forkBlockNumber|firstRunPending|readForkState|markForkFirstRunComplete|networkIsolated" src -S
printf '\n### src/util/fork-safety.ts\n'
sed -n '1,160p' src/util/fork-safety.ts | cat -n
printf '\n### settings for devnet rpc url and defaults\n'
sed -n '60,100p' src/cfg/setting.ts | cat -nRepository: ckb-devrel/offckb
Length of output: 7822
Other (CWE-345)
Reachability: External
● Entry
src/cli.ts:268
devnetInfo
│
▼
● Hop
src/cmd/devnet-info.ts
│
▼
● Hop
src/devnet/fork.ts:302
migrationNeededFromExitCode
│
▼
● Hop
src/cmd/status.ts
│
▼
● Hop
src/devnet/readiness.ts:69
waitForNodeReady
│
▼
● Sink
src/cmd/node.ts
Do not trust the first RPC responder for fork boundary state src/cmd/node.ts:216-227
forkBlockNumber is later used as the Mainnet fork replay boundary in validateMainnetForkSigning(), so a local process that wins the 127.0.0.1:8114 race can clear firstRunPending with a bogus tip before the spawned node is definitely the responder. Derive the boundary from the validated fork state, or only clear the flag after the RPC listener is bound to the spawned CKB process.
🧰 Tools
🪛 ast-grep (0.44.1)
[warning] 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 { exec, spawn, ChildProcess } from 'child_process';
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process-typescript)
🤖 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 `@src/cmd/node.ts` around lines 216 - 227, Do not use the first successful RPC
response in the fork-boundary flow to call markForkFirstRunComplete; ensure
forkBlockNumber comes from validated state belonging to the spawned CKB process,
or defer clearing firstRunPending until that process has definitively bound the
RPC listener. Preserve the genesis validation and keep
validateMainnetForkSigning() aligned with the trusted boundary.
| const privateKey = resolvePrivateKey(opt); | ||
| warnIfMainnetForkSigning(network, privateKey); | ||
| await warnIfForkIndexerIsBehind(network); | ||
|
|
||
| const ckb = new CKB({ network }); | ||
| const result = await ckb.udtIssue({ | ||
| privateKey, | ||
| kind: opt.udtKind, | ||
| amount, | ||
| typeArgs, | ||
| toAddress: opt.to, | ||
| }); | ||
|
|
||
| logTxSuccess(network, result.txHash, 'issued UDT'); | ||
| logger.info(`UDT kind: ${opt.udtKind}`); | ||
| logger.info(`UDT type args: ${result.typeArgs}`); | ||
| logger.info(`Receiver: ${result.receiver}`); | ||
| logger.info(`Next: offckb balance ${result.receiver} --udt-kind ${opt.udtKind} --udt-type-args ${result.typeArgs}`); | ||
| logger.result({ | ||
| command: 'udt.issue', | ||
| network, | ||
| kind: opt.udtKind, | ||
| amount, | ||
| receiver: result.receiver, | ||
| typeArgs: result.typeArgs, | ||
| txHash: result.txHash, | ||
| }); | ||
| return result; | ||
| } | ||
|
|
||
| export async function udtDestroy( | ||
| amount: string, | ||
| opt: UdtDestroyOption = { network: Network.devnet, udtKind: 'sudt', typeArgs: '' }, | ||
| ) { | ||
| const network = opt.network; | ||
| validateNetworkOpt(network); | ||
| validateUdtKind(opt.udtKind); | ||
| validateUdtAmount(amount); | ||
| const typeArgs = validateUdtTypeArgs(opt.udtKind, opt.typeArgs); | ||
|
|
||
| const privateKey = resolvePrivateKey(opt); | ||
| warnIfMainnetForkSigning(network, privateKey); | ||
| await warnIfForkIndexerIsBehind(network); | ||
|
|
||
| const ckb = new CKB({ network }); | ||
| const txHash = await ckb.udtDestroy({ | ||
| privateKey, | ||
| kind: opt.udtKind, | ||
| amount, | ||
| typeArgs, | ||
| }); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '--- src/cmd/udt.ts ---\n'
ast-grep outline src/cmd/udt.ts --view expanded || true
printf '\n--- fork-safety references ---\n'
rg -n "warnIfMainnetForkSigning|validateMainnetForkSigning|assertInputsCreatedAfter|udtIssue|udtDestroy" src -S
printf '\n--- src/util/fork-safety.ts ---\n'
wc -l src/util/fork-safety.ts
cat -n src/util/fork-safety.ts | sed -n '1,260p'
printf '\n--- src/cmd/udt.ts (relevant slice) ---\n'
cat -n src/cmd/udt.ts | sed -n '1,220p'Repository: ckb-devrel/offckb
Length of output: 246
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- src/cmd/udt.ts outline ---'
ast-grep outline src/cmd/udt.ts --view expanded || true
echo
echo '--- fork-safety references ---'
rg -n "warnIfMainnetForkSigning|validateMainnetForkSigning|assertInputsCreatedAfter|udtIssue|udtDestroy" src -S
echo
echo '--- src/util/fork-safety.ts ---'
wc -l src/util/fork-safety.ts
cat -n src/util/fork-safety.ts | sed -n '1,260p'
echo
echo '--- src/cmd/udt.ts (relevant slice) ---'
cat -n src/cmd/udt.ts | sed -n '1,220p'Repository: ckb-devrel/offckb
Length of output: 9462
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- src/sdk/ckb.ts around assertInputsCreatedAfter and UDT methods ---'
cat -n src/sdk/ckb.ts | sed -n '180,540p'Repository: ckb-devrel/offckb
Length of output: 14515
Other (CWE-294): Authentication Bypass by Capture-replay
Reachability: External
● Entry
src/cli.ts:157
DepositOptions
│
▼
● Hop
src/cmd/deposit.ts:16
deposit: deposit from devnet miner
│
▼
● Hop
src/sdk/ckb.ts:57
UdtIssueOption
│
▼
● Sink
src/cmd/udt.ts
Use replay-safe fork validation for UDT issue/destroy
udtIssue and udtDestroy only warn on a Mainnet fork, so they can still sign transactions that spend pre-fork inputs. Those signed txs can be replayed on Mainnet if they consume copied cells. Propagate the fork boundary here and reject non-built-in keys unless --allow-mainnet-replay-risk is set, then gate the SDK send path with assertInputsCreatedAfter like the transfer flow.
🤖 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 `@src/cmd/udt.ts` around lines 32 - 82, Update udtIssue and udtDestroy to use
replay-safe fork validation rather than only warnIfMainnetForkSigning: propagate
the fork boundary, reject non-built-in private keys unless
allowMainnetReplayRisk is enabled, and preserve the built-in-key exception.
Before the CKB SDK sends each transaction, apply the same
assertInputsCreatedAfter gating used by the transfer flow so inputs created
before the fork are rejected.
| for (const entry of fs.readdirSync(sourceData)) { | ||
| if (excludedTopLevelEntries.has(entry)) continue; | ||
| fs.cpSync(path.join(sourceData, entry), path.join(targetData, entry), { recursive: true }); | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== file outline =="
ast-grep outline src/devnet/fork.ts --view expanded || true
echo
echo "== relevant lines in src/devnet/fork.ts =="
nl -ba src/devnet/fork.ts | sed -n '1,380p'
echo
echo "== search validateSourceDir and data/db references =="
rg -n "validateSourceDir|data/db|cpSync|readdirSync|lstatSync|realpathSync|symlink" src/devnet -SRepository: ckb-devrel/offckb
Length of output: 2788
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== validateSourceDir / copySourceData region =="
sed -n '130,290p' src/devnet/fork.ts
echo
echo "== database migration region =="
sed -n '290,360p' src/devnet/fork.ts
echo
echo "== symlink-related search across src/devnet =="
rg -n "symlink|lstatSync|realpathSync|readlinkSync|cpSync|readdirSync|validateSourceDir|copySourceData|migrateDatabaseCopy" src/devnet -SRepository: ckb-devrel/offckb
Length of output: 9610
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== isFolderExists definition =="
rg -n "function isFolderExists|const isFolderExists|isFolderExists\\(" src -S
echo
echo "== surrounding code for isFolderExists if found =="
for f in $(rg -l "function isFolderExists|const isFolderExists" src -S); do
echo "--- $f ---"
sed -n '1,220p' "$f"
doneRepository: ckb-devrel/offckb
Length of output: 6536
🏁 Script executed:
#!/bin/bash
set -euo pipefail
node - <<'JS'
const fs = require('fs');
const path = require('path');
const os = require('os');
const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'cp-sync-probe-'));
const src = path.join(tmp, 'src');
const dest = path.join(tmp, 'dest');
const ext = path.join(tmp, 'external');
fs.mkdirSync(src, { recursive: true });
fs.mkdirSync(ext, { recursive: true });
fs.mkdirSync(path.join(src, 'data'), { recursive: true });
fs.symlinkSync(ext, path.join(src, 'data', 'db'), 'dir');
console.log('lstat on source link isSymlink:', fs.lstatSync(path.join(src, 'data', 'db')).isSymbolicLink());
console.log('stat on source link isDirectory:', fs.statSync(path.join(src, 'data', 'db')).isDirectory());
fs.cpSync(src, dest, { recursive: true });
const copied = path.join(dest, 'data', 'db');
console.log('lstat on copied link isSymlink:', fs.lstatSync(copied).isSymbolicLink());
console.log('copied symlink target:', fs.readlinkSync(copied));
console.log('copied link resolves to external dir:', fs.realpathSync(copied) === fs.realpathSync(ext));
JSRepository: ckb-devrel/offckb
Length of output: 368
Path Traversal (CWE-59)
Reachability: External
● Entry
src/cmd/udt.ts:25
udtIssue
│
▼
● Hop
src/devnet/readiness.ts:87
warnIfForkIndexerIsBehind
│
▼
● Sink
src/devnet/fork.ts
Reject symlinks in the source tree before copying validateSourceDir() follows data/db symlinks, and fs.cpSync() preserves them, so a crafted --from directory can fork a data/db link outside configPath and let migration/startup touch an external CKB database. Use lstatSync or realpath containment checks before copying.
🧰 Tools
🪛 ast-grep (0.44.1)
[warning] 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 { execFileSync, execSync, spawnSync } from 'child_process';
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process-typescript)
🤖 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 `@src/devnet/fork.ts` around lines 270 - 273, Update the source-copy loop in
the fork flow to inspect each source entry with lstatSync or an equivalent
non-following containment check before fs.cpSync; reject symlinked entries,
especially data/db, and ensure resolved targets remain within
configPath/sourceData before copying. Preserve copying of valid regular files
and directories.
| const DOWNLOAD_TIMEOUT_MS = 120_000; | ||
| const EXTRACT_TIMEOUT_MS = 60_000; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
--max-time 300 is unreachable — the outer spawnSync timeout (120s) fires first.
DOWNLOAD_TIMEOUT_MS is 120_000ms, but curl is told --max-time 300 (300s). Node's spawnSync will kill the process at 120s regardless, so the 300s curl flag never takes effect and slow downloads fail earlier than the code suggests.
🛠️ Proposed fix — align both timeouts
- const curlResult = spawnSync('curl', ['-fsSL', '--max-time', '300', '-o', archivePath, downloadUrl], {
+ const curlResult = spawnSync(
+ 'curl',
+ ['-fsSL', '--max-time', String(DOWNLOAD_TIMEOUT_MS / 1000), '-o', archivePath, downloadUrl],
+ {Also applies to: 152-165
🧰 Tools
🪛 ast-grep (0.44.1)
[warning] 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 'child_process';
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process-typescript)
🤖 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 `@src/tools/ckb-tui.ts` around lines 11 - 12, Align the download timeout
constants and the curl timeout used by the download flow so the outer spawnSync
timeout does not terminate before curl’s --max-time limit. Update the timeout
referenced by the relevant spawnSync call and curl arguments together,
preserving the existing extraction timeout and download behavior.
| // 5. Atomically move to the final location | ||
| fs.renameSync(extractedBinary, this.binaryPath); | ||
|
|
||
| // 6. Make executable on Unix | ||
| if (process.platform !== 'win32') { | ||
| fs.chmodSync(this.binaryPath, 0o755); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Cross-device rename can fail if the temp dir and data path are on different mounts.
fs.renameSync throws EXDEV when source and destination are on different filesystems. tempDir comes from os.tmpdir() while binaryPath is under the configured data path — these are commonly separate mounts in containers/CI (e.g. tmpfs /tmp). An EXDEV here would abort the entire install with an opaque low-level error.
🛠️ Proposed fix — fall back to copy+unlink on EXDEV
// 5. Atomically move to the final location
- fs.renameSync(extractedBinary, this.binaryPath);
+ try {
+ fs.renameSync(extractedBinary, this.binaryPath);
+ } catch (error) {
+ if ((error as NodeJS.ErrnoException).code === 'EXDEV') {
+ fs.copyFileSync(extractedBinary, this.binaryPath);
+ fs.unlinkSync(extractedBinary);
+ } else {
+ throw error;
+ }
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // 5. Atomically move to the final location | |
| fs.renameSync(extractedBinary, this.binaryPath); | |
| // 6. Make executable on Unix | |
| if (process.platform !== 'win32') { | |
| fs.chmodSync(this.binaryPath, 0o755); | |
| } | |
| // 5. Atomically move to the final location | |
| try { | |
| fs.renameSync(extractedBinary, this.binaryPath); | |
| } catch (error) { | |
| if ((error as NodeJS.ErrnoException).code === 'EXDEV') { | |
| fs.copyFileSync(extractedBinary, this.binaryPath); | |
| fs.unlinkSync(extractedBinary); | |
| } else { | |
| throw error; | |
| } | |
| } | |
| // 6. Make executable on Unix | |
| if (process.platform !== 'win32') { | |
| fs.chmodSync(this.binaryPath, 0o755); | |
| } |
🧰 Tools
🪛 ast-grep (0.44.1)
[warning] 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 'child_process';
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process-typescript)
🤖 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 `@src/tools/ckb-tui.ts` around lines 182 - 188, Update the installation move
step around fs.renameSync to handle EXDEV by copying extractedBinary to
this.binaryPath and then removing the source when the rename crosses
filesystems; rethrow other rename errors unchanged. Preserve the existing Unix
chmod behavior after either move path completes.
| const dataPath = '/tmp/offckb-devnet-data'; | ||
| const logDir = path.join(dataPath, 'logs'); | ||
| const pidFile = path.join(logDir, 'daemon.pid'); | ||
| const logFile = path.join(logDir, 'daemon.log'); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Remove or use the unused logFile binding.
logFile is declared but never referenced, which trips @typescript-eslint/no-unused-vars and will fail lint/CI. If it was meant to assert the daemon opens the log file, add an expect(mockOpenSync).toHaveBeenCalledWith(logFile, ...) check; otherwise drop it.
🧹 Proposed removal
const pidFile = path.join(logDir, 'daemon.pid');
-const logFile = path.join(logDir, 'daemon.log');📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const dataPath = '/tmp/offckb-devnet-data'; | |
| const logDir = path.join(dataPath, 'logs'); | |
| const pidFile = path.join(logDir, 'daemon.pid'); | |
| const logFile = path.join(logDir, 'daemon.log'); | |
| const dataPath = '/tmp/offckb-devnet-data'; | |
| const logDir = path.join(dataPath, 'logs'); | |
| const pidFile = path.join(logDir, 'daemon.pid'); |
🧰 Tools
🪛 ESLint
[error] 83-83: 'logFile' is assigned a value but never used. Allowed unused vars must match /^_/u.
(@typescript-eslint/no-unused-vars)
🤖 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/node-command.test.ts` around lines 80 - 83, Remove the unused logFile
binding from the test setup, unless the test is intended to verify log-file
opening; in that case, add an assertion using logFile with mockOpenSync. Keep
the existing dataPath and pidFile behavior unchanged.
Source: Linters/SAST tools
No description provided.