Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
187 changes: 187 additions & 0 deletions plans/pnpm-auto-deny-ignored-builds.md

Large diffs are not rendered by default.

4 changes: 4 additions & 0 deletions src/__tests__/app_upgrade_utils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,10 @@ vi.mock("../ipc/utils/git_utils", () => ({
gitCommit: vi.fn().mockResolvedValue(undefined),
}));

vi.mock("../ipc/utils/telemetry", () => ({
sendTelemetryEvent: vi.fn(),
}));

describe("isComponentTaggerUpgradeNeeded Heuristics", () => {
const mockPath = "/mock/app";

Expand Down
48 changes: 36 additions & 12 deletions src/ipc/handlers/app_upgrade_handlers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import { DyadError, DyadErrorKind } from "@/errors/dyad_error";
import {
isComponentTaggerUpgradeNeeded,
applyComponentTagger,
simpleSpawnWithDeniedPnpmBuildSelfHeal,
} from "../utils/app_upgrade_utils";
import fs from "node:fs";
import path from "node:path";
Expand Down Expand Up @@ -87,12 +88,23 @@ async function applyCapacitor({
appPath: string;
}) {
// Install Capacitor dependencies
await simpleSpawn({
command: `pnpm ${PNPM_PM_ON_FAIL_IGNORE_ARG} add --ignore-workspace-root-check @capacitor/core@7.4.4 @capacitor/cli@7.4.4 @capacitor/ios@7.4.4 @capacitor/android@7.4.4 || npm install @capacitor/core@7.4.4 @capacitor/cli@7.4.4 @capacitor/ios@7.4.4 @capacitor/android@7.4.4 --legacy-peer-deps`,
cwd: appPath,
successMessage: "Capacitor dependencies installed successfully",
errorPrefix: "Failed to install Capacitor dependencies",
});
try {
await simpleSpawnWithDeniedPnpmBuildSelfHeal({
command: `pnpm ${PNPM_PM_ON_FAIL_IGNORE_ARG} add --ignore-workspace-root-check @capacitor/core@7.4.4 @capacitor/cli@7.4.4 @capacitor/ios@7.4.4 @capacitor/android@7.4.4`,
cwd: appPath,
successMessage: "Capacitor dependencies installed successfully with pnpm",
errorPrefix: "Failed to install Capacitor dependencies via pnpm",
});
} catch (pnpmErr) {
logger.info("pnpm install failed, falling back to npm", pnpmErr);
await simpleSpawn({
command:
"npm install @capacitor/core@7.4.4 @capacitor/cli@7.4.4 @capacitor/ios@7.4.4 @capacitor/android@7.4.4 --legacy-peer-deps",
cwd: appPath,
successMessage: "Capacitor dependencies installed successfully with npm",
errorPrefix: "Failed to install Capacitor dependencies",
});
}

// Initialize Capacitor
await simpleSpawn({
Expand All @@ -105,12 +117,24 @@ async function applyCapacitor({
// Intentionally omit PNPM_INSTALL_POLICY_ARGS because:
// 1. confirmModulesPurge will almost never be needed for capacitor (i.e. user would need to switch from npm to pnpm and not triggered a rebuild).
// 2. strictBuildDeps should be kept true (default value) in case capacitor has native deps.
await simpleSpawn({
command: `pnpm ${PNPM_PM_ON_FAIL_IGNORE_ARG} install --prod=false || npm install --include=dev --legacy-peer-deps`,
cwd: appPath,
successMessage: "Development dependencies installed successfully",
errorPrefix: "Failed to install development dependencies",
});
try {
await simpleSpawnWithDeniedPnpmBuildSelfHeal({
command: `pnpm ${PNPM_PM_ON_FAIL_IGNORE_ARG} install --prod=false`,
cwd: appPath,
successMessage:
"Development dependencies installed successfully with pnpm",
errorPrefix: "Failed to install development dependencies via pnpm",
});
} catch (pnpmErr) {
logger.info("pnpm install failed, falling back to npm", pnpmErr);
await simpleSpawn({
command: "npm install --include=dev --legacy-peer-deps",
cwd: appPath,
successMessage:
"Development dependencies installed successfully with npm",
errorPrefix: "Failed to install development dependencies",
});
}

// Add iOS and Android platforms
await simpleSpawn({
Expand Down
61 changes: 60 additions & 1 deletion src/ipc/processors/executeAddDependency.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,21 @@ const {
commitPnpmAllowBuildsConfigIfChangedMock,
ensureSocketFirewallInstalledMock,
getPnpmMinimumReleaseAgeSupportMock,
readPnpmIgnoredBuildsMock,
recordDeniedPnpmBuildsMock,
runCommandMock,
sendTelemetryEventMock,
readEffectiveSettingsMock,
dbUpdateSetMock,
dbUpdateWhereMock,
} = vi.hoisted(() => ({
commitPnpmAllowBuildsConfigIfChangedMock: vi.fn(),
ensureSocketFirewallInstalledMock: vi.fn(),
getPnpmMinimumReleaseAgeSupportMock: vi.fn(),
readPnpmIgnoredBuildsMock: vi.fn(),
recordDeniedPnpmBuildsMock: vi.fn(),
runCommandMock: vi.fn(),
sendTelemetryEventMock: vi.fn(),
readEffectiveSettingsMock: vi.fn(),
dbUpdateSetMock: vi.fn(),
dbUpdateWhereMock: vi.fn(),
Expand Down Expand Up @@ -58,10 +64,16 @@ vi.mock("@/ipc/utils/socket_firewall", async () => {
commitPnpmAllowBuildsConfigIfChangedMock,
ensureSocketFirewallInstalled: ensureSocketFirewallInstalledMock,
getPnpmMinimumReleaseAgeSupport: getPnpmMinimumReleaseAgeSupportMock,
readPnpmIgnoredBuilds: readPnpmIgnoredBuildsMock,
recordDeniedPnpmBuilds: recordDeniedPnpmBuildsMock,
runCommand: runCommandMock,
};
});

vi.mock("@/ipc/utils/telemetry", () => ({
sendTelemetryEvent: sendTelemetryEventMock,
}));

describe("executeAddDependency", () => {
beforeEach(() => {
vi.clearAllMocks();
Expand All @@ -74,7 +86,11 @@ describe("executeAddDependency", () => {
minimumReleaseAgeSupported: true,
version: "10.16.0",
});
commitPnpmAllowBuildsConfigIfChangedMock.mockResolvedValue(undefined);
commitPnpmAllowBuildsConfigIfChangedMock.mockResolvedValue({
promotedPackages: [],
});
readPnpmIgnoredBuildsMock.mockResolvedValue([]);
recordDeniedPnpmBuildsMock.mockResolvedValue({ deniedBuilds: [] });
readEffectiveSettingsMock.mockResolvedValue({
blockUnsafeNpmPackages: true,
});
Expand Down Expand Up @@ -578,4 +594,47 @@ describe("executeAddDependency", () => {
'<dyad-add-dependency packages="react-safe">installed &lt;react&gt;</dyad-add-dependency>',
});
});

it("records denied pnpm builds and surfaces the security-policy note", async () => {
ensureSocketFirewallInstalledMock.mockResolvedValue({
available: false,
});
runCommandMock.mockResolvedValueOnce({
stdout: "installed via pnpm",
stderr: "",
});
readPnpmIgnoredBuildsMock.mockResolvedValue([
{ packageName: "core-js", packageSpec: "core-js@3.49.0" },
]);
recordDeniedPnpmBuildsMock.mockResolvedValue({
deniedBuilds: [{ packageName: "core-js", packageSpec: "core-js@3.49.0" }],
});

const result = await executeAddDependency({
packages: ["core-js"],
message: {
id: 1,
content:
'<dyad-add-dependency packages="core-js"></dyad-add-dependency>',
} as any,
appPath: "/tmp/app",
});

expect(result.installResults).toContain("installed via pnpm");
expect(result.installResults).toContain(
"Note: build scripts for core-js were not run (Dyad security policy).",
);
expect(sendTelemetryEventMock).toHaveBeenCalledWith(
"pnpm:build-auto-denied",
{
packages: ["core-js@3.49.0"],
source: "add-dependency",
},
);
expect(dbUpdateSetMock).toHaveBeenCalledWith({
content: expect.stringContaining(
"Note: build scripts for core-js were not run",
),
});
});
});
62 changes: 58 additions & 4 deletions src/ipc/processors/executeAddDependency.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,11 @@ import {
getCommandExecutionDisplayDetails,
getPackageManagerCommandEnv,
getPnpmMinimumReleaseAgeSupport,
readPnpmIgnoredBuilds,
recordDeniedPnpmBuilds,
runCommand,
} from "@/ipc/utils/socket_firewall";
import { sendTelemetryEvent } from "@/ipc/utils/telemetry";
import {
choosePackageManagerFromSignal,
getPackageManagerSignal,
Expand Down Expand Up @@ -169,6 +172,34 @@ async function runAddDependencyCommand(
}
}

function formatDeniedBuildsNote(packageNames: string[]): string {
if (packageNames.length === 0) {
return "";
}

const packageList = packageNames.join(", ");
return `\n\nNote: build scripts for ${packageList} were not run (Dyad security policy).`;
}

async function rebuildPromotedPnpmBuilds(
appPath: string,
packageNames: string[],
): Promise<void> {
if (packageNames.length === 0) {
return;
}

try {
await runCommand("pnpm", ["rebuild", ...packageNames], {
cwd: appPath,
env: getPackageManagerCommandEnv(),
timeoutMs: ADD_DEPENDENCY_INSTALL_TIMEOUT_MS,
});
} catch {
// Best effort: if the build is still broken, the install should not regress.
}
}

export async function installPackages({
packages,
appPath,
Expand Down Expand Up @@ -223,9 +254,10 @@ export async function installPackages({
) {
warningMessages.push(pnpmSupport.warningMessage);
}
if (packageManager === "pnpm") {
await commitPnpmAllowBuildsConfigIfChanged(appPath);
}
const promotedPackages =
packageManager === "pnpm"
? (await commitPnpmAllowBuildsConfigIfChanged(appPath)).promotedPackages
: [];
const { succeeded, installResults, lastError } =
await runAddDependencyCommand(
buildAddDependencyCommand(packages, packageManager, useSocketFirewall, {
Expand All @@ -241,8 +273,30 @@ export async function installPackages({
});
}

await rebuildPromotedPnpmBuilds(appPath, promotedPackages);

let installResultsWithPolicyNotes = installResults;
if (packageManager === "pnpm") {
const ignoredBuilds = await readPnpmIgnoredBuilds(appPath);
const { deniedBuilds } = await recordDeniedPnpmBuilds({
appPath,
ignoredBuilds,
});
if (deniedBuilds.length > 0) {
sendTelemetryEvent("pnpm:build-auto-denied", {
packages: deniedBuilds.map((ignoredBuild) => ignoredBuild.packageSpec),
source: "add-dependency",
});
installResultsWithPolicyNotes += formatDeniedBuildsNote(
Array.from(
new Set(deniedBuilds.map((ignoredBuild) => ignoredBuild.packageName)),
).sort((left, right) => left.localeCompare(right)),
);
}
}

return {
installResults,
installResults: installResultsWithPolicyNotes,
warningMessages,
};
}
Expand Down
49 changes: 47 additions & 2 deletions src/ipc/services/app_runtime_service.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,11 @@ const {
minimumReleaseAgeSupported: false,
})),
ensurePnpmAllowBuildsConfiguredMock:
vi.fn<(args: unknown) => Promise<{ changed: boolean }>>(),
vi.fn<
(
args: unknown,
) => Promise<{ changed: boolean; promotedPackages: string[] }>
>(),
readSettingsMock: vi.fn<() => Record<string, unknown>>(() => ({
runtimeMode2: "host",
})),
Expand Down Expand Up @@ -82,13 +86,22 @@ vi.mock("@/ipc/utils/socket_firewall", () => ({
npm_config_pm_on_fail: "ignore",
}),
getPnpmMinimumReleaseAgeSupport: () => getPnpmMinimumReleaseAgeSupportMock(),
isPnpmIgnoredBuildsError: (error: unknown) =>
String(error).includes("ERR_PNPM_IGNORED_BUILDS"),
parsePnpmIgnoredBuildsFromOutput: vi.fn(() => []),
readPnpmIgnoredBuilds: vi.fn(async () => []),
recordDeniedPnpmBuilds: vi.fn(async () => ({ deniedBuilds: [] })),
PNPM_INSTALL_POLICY_ARGS: [
"--config.pm-on-fail=ignore",
"--minimum-release-age=1440",
],
PNPM_PM_ON_FAIL_IGNORE_ARG: "--config.pm-on-fail=ignore",
}));

vi.mock("@/ipc/utils/telemetry", () => ({
sendTelemetryEvent: vi.fn(),
}));

vi.mock("@/ipc/utils/cloud_sandbox_provider", () => ({
buildCloudSandboxFileMap: vi.fn(),
CloudSandboxApiError: class CloudSandboxApiError extends Error {
Expand Down Expand Up @@ -187,7 +200,10 @@ describe("executeApp", () => {
minimumReleaseAgeSupported: false,
});
ensurePnpmAllowBuildsConfiguredMock.mockReset();
ensurePnpmAllowBuildsConfiguredMock.mockResolvedValue({ changed: false });
ensurePnpmAllowBuildsConfiguredMock.mockResolvedValue({
changed: false,
promotedPackages: [],
});
readSettingsMock.mockReset();
readSettingsMock.mockReturnValue({
runtimeMode2: "host",
Expand Down Expand Up @@ -307,6 +323,35 @@ describe("executeApp", () => {
);
});

it("rebuilds promoted pnpm builds before starting the dev server", async () => {
const process = new FakeChildProcess(101);
spawnMock.mockReturnValueOnce(process);
getPnpmMinimumReleaseAgeSupportMock.mockResolvedValue({
available: true,
minimumReleaseAgeSupported: true,
});
ensurePnpmAllowBuildsConfiguredMock.mockResolvedValue({
changed: true,
promotedPackages: ["core-js", "@scope/native"],
});

await executeApp({
appPath: "/tmp/app",
appId: 1,
event: createEvent(),
isNeon: false,
});

expect(spawnMock).toHaveBeenCalledWith(
"pnpm --config.pm-on-fail=ignore --minimum-release-age=1440 install && (pnpm rebuild 'core-js' '@scope/native' || true) && pnpm --config.pm-on-fail=ignore run dev --port 32101",
[],
expect.objectContaining({
cwd: "/tmp/app",
shell: true,
}),
);
});

it("does not warn about old pnpm for apps that explicitly use npm", async () => {
const appPath = await createTempAppDir();
try {
Expand Down
Loading
Loading