Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
540 changes: 163 additions & 377 deletions packages/@dcl/sdk-commands/package-lock.json

Large diffs are not rendered by default.

6 changes: 6 additions & 0 deletions packages/@dcl/sdk-commands/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,12 @@
"@types/qrcode": "^1.5.6",
"@types/ws": "^8.5.4"
},
"engines": {
"node": ">=22.0.0"
},
"overrides": {
"protobufjs": "^7.6.3"
},
"files": [
"dist",
".dclrc",
Expand Down
7 changes: 3 additions & 4 deletions packages/@dcl/sdk-commands/src/components/log.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,9 @@ export const writeToStderr = (...parameters: readonly unknown[]) => {
// @see https://no-color.org
// @see https://www.npmjs.com/package/chalk
const useColor = process.env.FORCE_COLOR !== '0' && !process.env.NO_COLOR
const paint =
(format: Parameters<typeof styleText>[0]) =>
(text: string | number) =>
useColor ? styleText(format, String(text)) : String(text)
// skip styleText's stream validation: it checks stdout, but the logger writes to stderr
const paint = (format: Parameters<typeof styleText>[0]) => (text: string | number) =>
useColor ? styleText(format, String(text), { validateStream: false }) : String(text)

export const colors = {
bgBlack: paint('bgBlack'),
Expand Down
15 changes: 11 additions & 4 deletions packages/@dcl/sdk-commands/src/logic/get-free-port.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
import * as net from 'net'

// search upward from 8000 (portfinder's contract) so the preview URL stays stable across runs
const BASE_PORT = 8000
const HIGHEST_PORT = 65535

function tryListen(port: number): Promise<number> {
return new Promise((resolve, reject) => {
const server = net.createServer()
Expand All @@ -16,11 +20,14 @@ export async function getPort(port: number, failoverPort = 2044) {
const resolvedPort = port && Number.isInteger(port) ? +port : 0

if (!resolvedPort) {
try {
return await tryListen(0)
} catch (e) {
return failoverPort
for (let candidate = BASE_PORT; candidate <= HIGHEST_PORT; candidate++) {
try {
return await tryListen(candidate)
} catch {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] The bare catch treats every failure as "port busy". If createServer/listen fails for a reason that won't change on the next port (EACCES under a sandbox, EMFILE, a mocked net module), this walks all 57,536 candidates one await at a time before finally returning failoverPort. The existing should return the fail-over port when probing fails spec exercises exactly that path today.

Bailing out on anything that isn't an address conflict keeps the happy path identical and makes the failure fast:

} catch (e) {
  const code = (e as NodeJS.ErrnoException)?.code
  if (code !== 'EADDRINUSE') break
}

// busy, try the next one
}
}
return failoverPort
}

return resolvedPort
Expand Down
29 changes: 20 additions & 9 deletions packages/@dcl/sdk-commands/src/logic/open.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { spawn } from 'child_process'
import { spawn, SpawnOptions } from 'child_process'
import { release } from 'os'

function isWsl(): boolean {
Expand All @@ -10,14 +10,25 @@ export default async function open(target: string): Promise<void> {
if (protocol !== 'http:' && protocol !== 'https:') {
throw new Error(`Refusing to open non-http(s) URL: ${target}`)
}
const [command, args] =
process.platform === 'darwin'
? ['open', [target]]
: process.platform === 'win32' || isWsl()
? // `start` treats the first quoted arg as a window title; `&` must be escaped for cmd
[isWsl() ? 'cmd.exe' : 'cmd', ['/c', 'start', '""', target.replace(/&/g, '^&')]]
: ['xdg-open', [target]]
const child = spawn(command, args, { stdio: 'ignore', detached: true })
const spawnOptions: SpawnOptions = { stdio: 'ignore', detached: true }
let command: string
let args: string[]
if (process.platform === 'darwin') {
command = 'open'
args = [target]
} else if (process.platform === 'win32') {
// libuv's CRT-style quoting mangles the '""' title arg, so hand cmd the exact line
command = 'cmd'
args = ['/c', `start "" "${target}"`]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Defense-in-depth: the target string is interpolated into a cmd argument with windowsVerbatimArguments: true. If a URL ever contained a " character, it would break out of the double-quote boundary in start "" "${target}" and could theoretically execute arbitrary commands via cmd.exe.

In practice, both current callsites construct URLs from internal values (localhost preview, decentraland.org/bevy-web), so this is not exploitable today. But as a defense-in-depth measure, consider stripping or rejecting " in target:

target = target.replace(/"/g, '')

Non-blocking.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] With windowsVerbatimArguments the raw target string is interpolated straight into a cmd command line, so a " in the URL closes the quoting and leaves the remainder exposed to cmd metacharacter parsing. Not reachable today — both call sites build localhost/preview URLs — but you already parse the URL two lines above for the protocol check, and new URL(target).href percent-encodes " for free:

const { protocol, href } = new URL(target)
// ...
args = ['/c', `start "" "${href}"`]

Separately: the WSL branch keeps the old ['/c', 'start', '""', ...] form. I assume that's deliberate (libuv doesn't apply CRT-style quoting when the host is POSIX, so the mangling you're fixing doesn't happen there) — a one-line comment saying so would stop the next reader from "fixing" it to match the win32 branch.

spawnOptions.windowsVerbatimArguments = true
} else if (isWsl()) {
command = 'cmd.exe'
args = ['/c', 'start', '""', target.replace(/&/g, '^&')]
} else {
command = 'xdg-open'
args = [target]
}
const child = spawn(command, args, spawnOptions)
// opening the browser is best-effort: without this handler a missing
// xdg-open/open/cmd (headless, containers) crashes the CLI
child.on('error', () => {})
Expand Down
14 changes: 8 additions & 6 deletions packages/@dcl/sdk-commands/src/logic/project-files.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { ContentMapping } from '@dcl/schemas/dist/misc/content-mapping'
import { CliComponents } from '../components'
import { getDCLIgnorePatterns } from './dcl-ignore'
import { sync as globSync } from 'glob'
import { globSync, statSync, Dirent } from 'fs'
import ignore from 'ignore'
import i18next from 'i18next'
import os from 'os'
Expand All @@ -27,13 +27,15 @@ export async function getPublishableFiles(
const ig = ignore().add(ignorePatterns)
const allFiles = globSync('**/*', {
cwd: projectRoot,
absolute: false,
dot: false,
ignore: ignorePatterns,
nodir: true
// prune walking into trees ig.filter below always excludes anyway (perf only;
// exclude receives a string on some Node versions and a Dirent on others)
exclude: (entry: string | Dirent) => {
const name = typeof entry === 'string' ? path.basename(entry) : entry.name
return name.startsWith('.') || name === 'node_modules'
}
})

return ig.filter(allFiles)
return ig.filter(allFiles).filter((file) => statSync(resolve(projectRoot, file)).isFile())

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] statSync follows symlinks and throws, so a dangling symlink anywhere under the project root (or a file removed between the glob walk and the stat) aborts deploy/export-static with an unhandled ENOENT instead of a CLI error message. Reproduced on Node 24 with a broken symlink in the scene root.

throwIfNoEntry: false turns that into a silent skip, which matches the intent ("only keep real files"):

Suggested change
return ig.filter(allFiles).filter((file) => statSync(resolve(projectRoot, file)).isFile())
return ig.filter(allFiles).filter((file) => statSync(resolve(projectRoot, file), { throwIfNoEntry: false })?.isFile() ?? false)

Alternatively globSync('**/*', { withFileTypes: true, ... }) gives you isFile() straight off the Dirent and drops one stat syscall per entry — though the exclude callback typing gets a bit noisier, so the suggestion above is probably the better trade here.

}

/**
Expand Down
5 changes: 4 additions & 1 deletion packages/@dcl/sdk-commands/src/logic/prompts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,13 @@ export default async function prompts(
options: Options = {}
): Promise<Record<string, string | number | boolean>> {
const rl = readline.createInterface({ input: process.stdin, output: process.stdout })
// on Node <24 Ctrl+C only pauses stdin instead of rejecting rl.question; abort so onCancel runs
const canceled = new AbortController()
rl.on('SIGINT', () => canceled.abort())
try {
for (;;) {
const suffix = question.type === 'confirm' ? (question.initial ? ' [Y/n] ' : ' [y/N] ') : ' '
const answer = await rl.question(`${question.message}${suffix}`)
const answer = await rl.question(`${question.message}${suffix}`, { signal: canceled.signal })
if (question.type === 'confirm') {
const value = answer === '' ? !!question.initial : /^y(es)?$/i.test(answer)
return { [question.name]: value }
Expand Down
13 changes: 7 additions & 6 deletions scripts/helpers.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
import { exec } from 'child_process'
import { sync as globSync } from 'glob'
import { resolve, relative } from 'path'
import { existsSync, readFileSync, writeFileSync, lstatSync, removeSync, copySync } from 'fs-extra'
import { rmSync } from 'fs'
import { rmSync, globSync } from 'fs'

/**
* @returns the resolved absolute path
Expand Down Expand Up @@ -51,10 +50,12 @@ export function itDeletesFolder(folder: string, cwd: string) {
}
export function itDeletesGlob(pattern: string, cwd: string) {
it(`deletes ${pattern} in ${cwd}`, () => {
globSync(pattern, { absolute: true, cwd }).forEach((file) => {
console.log(`> deleting ${file}`)
rmSync(file, { recursive: true, force: true })
})
globSync(pattern, { cwd })
.map((file) => resolve(cwd, file))
.forEach((file) => {
console.log(`> deleting ${file}`)
rmSync(file, { recursive: true, force: true })
})
})
}

Expand Down
5 changes: 2 additions & 3 deletions test/ecs/add-entity-from-composite.spec.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,12 @@
import { readFileSync } from 'fs'
import { glob } from 'glob'
import { readFileSync, globSync } from 'fs'
import path from 'path'
import { Composite, Engine, Entity, IEngine } from '../../packages/@dcl/ecs/src'
import { components } from '../../packages/@dcl/ecs/src'

const COMPOSITE_BASE_PATH = 'test/ecs/composites'

function getJsonCompositeFrom(globPath: string, cwd: string): Composite.Resource[] {
const compositeFileContent = glob.sync(globPath, { cwd }).map((item) => ({
const compositeFileContent = globSync(globPath, { cwd }).map((item) => ({
src: item,
content: readFileSync(path.resolve(cwd, item)).toString()
}))
Expand Down
9 changes: 4 additions & 5 deletions test/ecs/composite.spec.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import { existsSync, readFileSync, writeFileSync } from 'fs'
import { glob } from 'glob'
import { existsSync, readFileSync, writeFileSync, globSync } from 'fs'
import path from 'path'
import {
Composite,
Expand All @@ -23,7 +22,7 @@ const nonBinaryCompositeJsonPath = 'non-binary.composite'
// ########

function getJsonCompositeFrom(globPath: string, cwd: string): Composite.Resource[] {
const compositeFileContent = glob.sync(globPath, { cwd }).map((item) => ({
const compositeFileContent = globSync(globPath, { cwd }).map((item) => ({
src: item,
content: readFileSync(path.resolve(cwd, item)).toString()
}))
Expand All @@ -34,7 +33,7 @@ function getJsonCompositeFrom(globPath: string, cwd: string): Composite.Resource
}

function getBinaryCompositeFrom(globPath: string, cwd: string) {
const compositeFileContent = glob.sync(globPath, { cwd }).map((item) => ({
const compositeFileContent = globSync(globPath, { cwd }).map((item) => ({
src: item,
content: readFileSync(path.resolve(cwd, item))
}))
Expand Down Expand Up @@ -149,7 +148,7 @@ describe('composite instantiation system', () => {
// actually instances nested child composites. Leaf composites (whose
// definition carries no `composite::root` component) emit none.
const hasNestedComposites = composite.composite.components.some(
component => component.name === CompositeRootComponent.componentName
(component) => component.name === CompositeRootComponent.componentName
)
if (hasNestedComposites) {
expect(composites.length).toBeGreaterThan(0)
Expand Down
10 changes: 3 additions & 7 deletions test/snapshots.spec.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { version as vmVersion } from '@dcl/quickjs-emscripten/package.json'
import { exec } from 'child_process'
import { existsSync, readFileSync, writeFileSync } from 'fs-extra'
import { glob } from 'glob'
import { globSync } from 'fs'
import path from 'path'
import { CrdtMessageType, engine } from '../packages/@dcl/ecs/src'
import { ReadWriteByteBuffer } from '../packages/@dcl/ecs/src/serialization/ByteBuffer'
Expand Down Expand Up @@ -40,13 +40,9 @@ describe('Runs the snapshots', () => {
ENV
)

glob
.sync('test/snapshots/production-bundles/*.ts', { absolute: false })
.forEach((file) => testFileSnapshot(file, true))
globSync('test/snapshots/production-bundles/*.ts').forEach((file) => testFileSnapshot(file, true))

glob
.sync('test/snapshots/development-bundles/*.ts', { absolute: false })
.forEach((file) => testFileSnapshot(file, false))
globSync('test/snapshots/development-bundles/*.ts').forEach((file) => testFileSnapshot(file, false))
})

function testFileSnapshot(fileName: string, _productionBuild: boolean) {
Expand Down
3 changes: 3 additions & 0 deletions test/snapshots/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading