Skip to content

Minor correctness/robustness fixes: empty-string override, secret logging, version precedence, ZIP bounds #267

@aosmcleod

Description

@aosmcleod

Summary

A bundle of smaller, verified correctness/robustness issues found during an audit. Grouped because each is low severity; happy to split into separate issues if preferred.

1. Empty-string platform override silently ignored — src/shared/config.ts:122

result.command = platformConfig.command || result.command uses ||, so a deliberate empty-string override (command: "") is falsy and falls back to the base command. Reproduced: base command:"default", darwin override command:"" → resolved "default" (expected ""). Fix: use !== undefined presence checks.

2. replaceVariables logs raw config values (potential secret leak) — src/shared/config.ts:32-35

When a replacement is an array used in string context, the code does console.warn(..., { key, replacement }), printing the raw value. user_config options can be sensitive: true (API keys, passwords), and this path uses bare console.warn (not the silenceable logger). Fix: log the key/var name only; never the value.

3. manifest_version vs deprecated dxt_version precedence — src/shared/manifestVersionResolve.ts:5-22

If manifest_version is present but unsupported (e.g. "0.5") while a supported dxt_version (e.g. "0.3") is also present, the authoritative newer field is ignored and resolution falls through to the deprecated dxt_version. Mostly self-correcting (strict schema rejects later), but produces a confusing error. Fix: if manifest_version is present but unsupported, error on that rather than silently using dxt_version.

4. Malformed ZIP central directory → uncaught RangeError region — src/cli/unpack.ts:53-84

The EOCD scan and central-directory walk call readUInt32LE/readUInt16LE/toString at offsets derived from untrusted ZIP fields with no bounds validation against zipBuffer.length. A crafted centralDirOffset throws RangeError; it's caught by the outer handler but silently skips the permissions pass for the whole archive. Fix: validate eocdOffset + 16 <= len, centralDirOffset < len, and offset + 46 + filenameLength <= len before each read.

Environment: current main (70fe3b3).

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions