From ec0b081371f118e6e5e042fbef3479359c672ddb Mon Sep 17 00:00:00 2001
From: morluto <76467478+morluto@users.noreply.github.com>
Date: Tue, 13 Oct 2026 13:39:57 +0800
Subject: [PATCH] fix(filesystem): enforce symlink and decoded-text admission
---
src/apktool/ApktoolProvider.test.ts | 48 +++++++++++++++-
src/apktool/ApktoolProvider.ts | 21 ++++---
src/filesystem/RegularFile.ts | 12 +++-
.../regularFileNoFollowFallback.test.ts | 55 +++++++++++++++++++
4 files changed, 127 insertions(+), 9 deletions(-)
create mode 100644 tests/process-global/regularFileNoFollowFallback.test.ts
diff --git a/src/apktool/ApktoolProvider.test.ts b/src/apktool/ApktoolProvider.test.ts
index 5b2a32114d..b6ee249e54 100644
--- a/src/apktool/ApktoolProvider.test.ts
+++ b/src/apktool/ApktoolProvider.test.ts
@@ -96,6 +96,7 @@ const decodeStub = (
readonly withStrings?: boolean;
readonly withGerman?: boolean;
readonly failDecode?: boolean;
+ readonly ymlFile?: string;
readonly manifestFile?: string;
readonly stringsFile?: string;
readonly stringsXml?: string;
@@ -104,6 +105,10 @@ const decodeStub = (
const withStrings = options.withStrings ?? true;
const withGerman = options.withGerman ?? true;
const stringsXml = options.stringsXml ?? REAL_STRINGS;
+ const ymlWrite =
+ options.ymlFile === undefined
+ ? `printf '%s\\n' '${REAL_YML.replace(/'/gu, "'\\''")}' > "$5/apktool.yml"`
+ : `cp '${options.ymlFile}' "$5/apktool.yml"`;
const manifestWrite =
options.manifestFile === undefined
? `printf '%s\\n' '${REAL_MANIFEST.replace(/'/gu, "'\\''")}' > "$5/AndroidManifest.xml"`
@@ -117,7 +122,7 @@ if [ "$1" = "d" ]; then
${
options.failDecode === true
? `printf 'Exception in thread "main" brut.androlib.AndrolibException: Could not decode the APK\\n' >&2; exit 1;`
- : `printf '%s\\n' '${REAL_YML.replace(/'/gu, "'\\''")}' > "$5/apktool.yml"
+ : `${ymlWrite}
${manifestWrite}
mkdir -p "$5/res/values"${withGerman ? ' "$5/res/values-de"' : ""}
${withStrings ? stringsWrite : ""}
@@ -192,6 +197,47 @@ it("decodes resources with target identity, metadata, and strings", async () =>
expect(result.coverage).toBe("complete");
});
+it.each([
+ {
+ name: "malformed UTF-8",
+ bytes: Buffer.concat([
+ Buffer.from("versionInfo:\n versionName: com."),
+ Buffer.from([0xff]),
+ Buffer.from(".app\n"),
+ ]),
+ valid: false,
+ },
+ {
+ name: "a literal replacement character",
+ bytes: Buffer.from("versionInfo:\n versionName: com.�.app\n", "utf8"),
+ valid: true,
+ },
+])("handles $name in producer metadata", async ({ bytes, valid }) => {
+ const { command, root } = await writeApktoolStub((root) =>
+ decodeStub({ ymlFile: join(root, "malformed-apktool.yml") })(root),
+ );
+ await writeFile(join(root, "malformed-apktool.yml"), bytes);
+ const apk = await apkFixture(root);
+ if (valid) {
+ const result = await evidenceResult(providerOver(command), {
+ operation: "decode_android_resources",
+ input: { path: apk.path, include_strings: false },
+ });
+ expect(result.metadata).toMatchObject({ version_name: "com.�.app" });
+ expect(result.coverage).toBe("complete");
+ return;
+ }
+ const result = await providerOver(command).execute({
+ operation: "decode_android_resources",
+ input: { path: apk.path, include_strings: false },
+ });
+ if (result.ok) throw new Error("expected invalid producer output");
+ const projected = projectAnalysisError(result.error);
+ expect(projected).toMatchObject({ code: "unreadable_output" });
+ expect(result.error.message).toContain("invalid UTF-8");
+ expect(result.error.message).toContain("apktool.yml");
+});
+
it("preserves empty resource strings and reports markup omissions as partial", async () => {
const stringsXml =
' A & B Hello world';
diff --git a/src/apktool/ApktoolProvider.ts b/src/apktool/ApktoolProvider.ts
index 682e2f2ea5..1e1f187c0a 100644
--- a/src/apktool/ApktoolProvider.ts
+++ b/src/apktool/ApktoolProvider.ts
@@ -117,13 +117,20 @@ const readBoundedText = async (
throwIfCancelled(operation, signal);
const truncated = length > budget;
// A prefix may end within a UTF-8 character; omit those trailing bytes.
- const decoder = new TextDecoder("utf-8");
- return {
- text: decoder.decode(bytes.subarray(0, Math.min(length, budget)), {
- stream: truncated,
- }),
- truncated,
- };
+ try {
+ const decoder = new TextDecoder("utf-8", { fatal: true });
+ return {
+ text: decoder.decode(bytes.subarray(0, Math.min(length, budget)), {
+ stream: truncated,
+ }),
+ truncated,
+ };
+ } catch (cause: unknown) {
+ throw new AnalysisProtocolError(
+ `apktool produced invalid UTF-8 in ${path}`,
+ { cause },
+ );
+ }
} finally {
await file.close();
}
diff --git a/src/filesystem/RegularFile.ts b/src/filesystem/RegularFile.ts
index a3037c7281..4942e2e467 100644
--- a/src/filesystem/RegularFile.ts
+++ b/src/filesystem/RegularFile.ts
@@ -1,6 +1,6 @@
import { constants } from "node:fs";
import type { BigIntStats, Stats } from "node:fs";
-import { open, type FileHandle } from "node:fs/promises";
+import { lstat, open, type FileHandle } from "node:fs/promises";
import { OwnedFileHandle } from "./OwnedFileHandle.js";
@@ -74,6 +74,16 @@ export const openRegularFile = async (
options.signal?.throwIfAborted();
if (!metadata.isFile())
throw new NonRegularFileReadError(path, metadata.isDirectory());
+ // O_NOFOLLOW is unavailable on Windows. Recheck the name only on hosts
+ // where open could have followed a symlink, while the descriptor is owned.
+ if (options.symlinks === "reject" && (constants.O_NOFOLLOW ?? 0) === 0) {
+ const named = await lstat(path);
+ options.signal?.throwIfAborted();
+ if (named.isSymbolicLink())
+ throw new NonRegularFileReadError(path, false);
+ if (!sameRegularFileState(metadata, named))
+ throw new RegularFileChangedError(path);
+ }
return handle;
} catch (cause: unknown) {
try {
diff --git a/tests/process-global/regularFileNoFollowFallback.test.ts b/tests/process-global/regularFileNoFollowFallback.test.ts
new file mode 100644
index 0000000000..44ce66c7c0
--- /dev/null
+++ b/tests/process-global/regularFileNoFollowFallback.test.ts
@@ -0,0 +1,55 @@
+import { readFile, symlink } from "node:fs/promises";
+import { join } from "node:path";
+
+import { expect, it, vi } from "vitest";
+
+vi.mock("node:fs", async (importOriginal) => {
+ const original = await importOriginal();
+ return {
+ ...original,
+ constants: { ...original.constants, O_NOFOLLOW: 0 },
+ };
+});
+
+import { openRegularFile } from "../../src/filesystem/RegularFile.js";
+import { ZipArtifactReader } from "../../src/artifacts/ZipArtifactReader.js";
+import { writeOrderedZip } from "../fixtures/artifactEntryOrder.js";
+import { createTestTempDirectory } from "../fixtures/temporaryDirectory.js";
+
+it("rejects a symlink when O_NOFOLLOW is unavailable and still follows when requested", async () => {
+ const root = await createTestTempDirectory("rea-no-nofollow-");
+ const target = join(root, "target.zip");
+ const link = join(root, "selected.zip");
+ await writeOrderedZip(target, ["member.bin"]);
+ await symlink(target, link);
+
+ const reader = new ZipArtifactReader(target, "zip");
+ try {
+ const entries = [];
+ for await (const entry of reader.entries()) entries.push(entry);
+ expect(entries.map(({ path }) => path)).toEqual(["member.bin"]);
+ } finally {
+ await reader.close();
+ }
+
+ const linkedReader = new ZipArtifactReader(link, "zip");
+ try {
+ await expect(async () => {
+ for await (const _entry of linkedReader.entries()) {
+ // Consume through the public reader boundary.
+ }
+ }).rejects.toMatchObject({
+ name: "ArtifactReaderFailure",
+ reason: "format",
+ });
+ } finally {
+ await linkedReader.close();
+ }
+
+ const followed = await openRegularFile(link, { symlinks: "follow" });
+ try {
+ await expect(readFile(followed)).resolves.toEqual(await readFile(target));
+ } finally {
+ await followed.close();
+ }
+});