Skip to content
Draft
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
47 changes: 47 additions & 0 deletions scripts/adm-zip-security-lib.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -214,3 +214,50 @@ export function ensureSecureAdmZip(

return { patched, skipped: false, warnings };
}

/**
* @param {Record<string, unknown>} value
* @returns {Array<[string, unknown]>}
*/
export function sortedPackageEntries(value = {}) {
return Object.entries(value).sort(([left], [right]) => left.localeCompare(right));
}

/**
* Validate npm-shrinkwrap.json root metadata against package.json.
* @param {Record<string, unknown>} shrinkwrap
* @param {{ name: string; version: string; dependencies?: Record<string, string>; optionalDependencies?: Record<string, string> }} pkg
* @returns {string | null} error message when invalid, null when valid
*/
export function validateNpmShrinkwrapMetadata(shrinkwrap, pkg) {
const root = shrinkwrap.packages?.[""];
if (!root) {
return "root package metadata is missing";
}
if (root.name !== pkg.name || root.version !== pkg.version) {
return `root metadata does not match ${pkg.name}@${pkg.version}`;
}

if (
JSON.stringify(sortedPackageEntries(root.dependencies)) !==
JSON.stringify(sortedPackageEntries(pkg.dependencies))
) {
return "root dependencies do not match package.json";
}
if (
JSON.stringify(sortedPackageEntries(root.optionalDependencies)) !==
JSON.stringify(sortedPackageEntries(pkg.optionalDependencies))
) {
return "root optionalDependencies do not match package.json";
}

const hoistedAdmZipVersion = shrinkwrap.packages?.["node_modules/adm-zip"]?.version;
if (
!hoistedAdmZipVersion ||
!isSecureAdmZipVersion(hoistedAdmZipVersion, SECURE_ADM_ZIP_VERSION)
) {
return `hoisted adm-zip is ${hoistedAdmZipVersion ?? "missing"}`;
}

return null;
}
37 changes: 4 additions & 33 deletions scripts/generate-npm-shrinkwrap.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,7 @@ import { spawnSync } from "node:child_process";
import { fileURLToPath } from "node:url";
import {
ensureSecureAdmZip,
isSecureAdmZipVersion,
SECURE_ADM_ZIP_VERSION,
validateNpmShrinkwrapMetadata,
} from "./adm-zip-security-lib.mjs";

const repoRoot = join(dirname(fileURLToPath(import.meta.url)), "..");
Expand All @@ -31,10 +30,6 @@ function run(cmd, args, cwd) {

const pkg = JSON.parse(readFileSync(join(repoRoot, "package.json"), "utf8"));

function sortedEntries(value = {}) {
return Object.entries(value).sort(([left], [right]) => left.localeCompare(right));
}

function failCheck(message) {
console.error(`npm-shrinkwrap.json is out of date: ${message}`);
process.exit(1);
Expand All @@ -46,33 +41,9 @@ if (checkMode) {
}

const shrinkwrap = JSON.parse(readFileSync(shrinkwrapPath, "utf8"));
const root = shrinkwrap.packages?.[""];
if (!root) {
failCheck("root package metadata is missing");
}
if (root.name !== pkg.name || root.version !== pkg.version) {
failCheck(`root metadata does not match ${pkg.name}@${pkg.version}`);
}

if (
JSON.stringify(sortedEntries(root.dependencies)) !==
JSON.stringify(sortedEntries(pkg.dependencies))
) {
failCheck("root dependencies do not match package.json");
}
if (
JSON.stringify(sortedEntries(root.optionalDependencies)) !==
JSON.stringify(sortedEntries(pkg.optionalDependencies))
) {
failCheck("root optionalDependencies do not match package.json");
}

const hoistedAdmZipVersion = shrinkwrap.packages?.["node_modules/adm-zip"]?.version;
if (
!hoistedAdmZipVersion ||
!isSecureAdmZipVersion(hoistedAdmZipVersion, SECURE_ADM_ZIP_VERSION)
) {
failCheck(`hoisted adm-zip is ${hoistedAdmZipVersion ?? "missing"}`);
const validationError = validateNpmShrinkwrapMetadata(shrinkwrap, pkg);
if (validationError) {
failCheck(validationError);
}

console.error("npm-shrinkwrap.json metadata is up to date");
Expand Down
39 changes: 39 additions & 0 deletions tests/registry/gitops.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,29 @@ describe("gitops_agent", () => {
expect(call.method).toBe("DELETE");
expect(call.path).toBe("/gitops/api/v1/agents/agent1779094157087");
});

it("delete: rejects when agent_id is missing (paramsSchema, not resource_id)", async () => {
const client = makeClient(vi.fn());

await expect(
registry.dispatch(client, "gitops_agent", "delete", {}),
).rejects.toThrow(/Missing required param\(s\) for gitops_agent\.delete: agent_id/);
});

it("delete: account resource_scope omits org/project query params", async () => {
const mockRequest = vi.fn().mockResolvedValue({});
const client = makeClient(mockRequest);

await registry.dispatch(client, "gitops_agent", "delete", {
agent_id: "myagent",
resource_scope: "account",
});

const call = mockRequest.mock.calls[0][0];
expect(call.path).toBe("/gitops/api/v1/agents/myagent");
expect(call.params.orgIdentifier).toBeUndefined();
expect(call.params.projectIdentifier).toBeUndefined();
});
});

// ---------------------------------------------------------------------------
Expand Down Expand Up @@ -271,6 +294,22 @@ describe("gitops_application", () => {
).rejects.toThrow(/Deletion mode is required/);
});

it("delete: missing cascade error comes from bodyBuilder, not paramsSchema", async () => {
const client = makeClient(vi.fn());

try {
await registry.dispatch(client, "gitops_application", "delete", {
agent_id: "account.myagent",
app_name: "demo-app",
});
expect.fail("expected dispatch to throw");
} catch (err) {
const message = (err as Error).message;
expect(message).toMatch(/Deletion mode is required/);
expect(message).not.toMatch(/Missing required param\(s\)/);
}
});

it("delete: throws when cascade=true but propagation_policy is missing", async () => {
const client = makeClient(vi.fn());

Expand Down
36 changes: 36 additions & 0 deletions tests/registry/registry.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1895,6 +1895,30 @@ describe("Registry", () => {
expect(call.body.costCategoryDTOs).toBeUndefined();
});

it("cost_recommendation_count get does not include costCategoryDTOs when only cost_category is provided without cost_buckets", async () => {
const mockRequest = vi.fn().mockResolvedValue({ data: 5 });
const client = makeClient(mockRequest);

await registry.dispatch(client, "cost_recommendation_count", "get", {
cost_category: "AI Platform team",
});

const call = mockRequest.mock.calls[0][0];
expect(call.body.costCategoryDTOs).toBeUndefined();
});

it("cost_recommendation_count get surfaces _error when API returns non-number data", async () => {
const mockRequest = vi.fn().mockResolvedValue({ data: "not-a-number" });
const client = makeClient(mockRequest);

const result = await registry.dispatch(client, "cost_recommendation_count", "get", {});

expect(result).toEqual({
count: 0,
_error: "Unexpected response shape — data is not a number",
});
});

it("cost_recommendation_count get sends default body when no filters provided", async () => {
const mockRequest = vi.fn().mockResolvedValue({ data: 42 });
const client = makeClient(mockRequest);
Expand Down Expand Up @@ -1973,6 +1997,18 @@ describe("Registry", () => {
]);
});

it("cost_recommendation_stats get does not include costCategoryDTOs when only cost_category is provided without cost_buckets", async () => {
const mockRequest = vi.fn().mockResolvedValue({ data: {} });
const client = makeClient(mockRequest);

await registry.dispatch(client, "cost_recommendation_stats", "get", {
cost_category: "AI Platform team",
});

const call = mockRequest.mock.calls[0][0];
expect(call.body.costCategoryDTOs).toBeUndefined();
});

it("cost_recommendation_stats get passes recommendation_states", async () => {
const mockRequest = vi.fn().mockResolvedValue({ data: {} });
const client = makeClient(mockRequest);
Expand Down
79 changes: 79 additions & 0 deletions tests/scripts/adm-zip-security-lib.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,9 @@ import {
findOnnxRuntimeNodeDirs,
isSecureAdmZipVersion,
listInsecureAdmZipInstalls,
readAdmZipVersion,
SECURE_ADM_ZIP_VERSION,
validateNpmShrinkwrapMetadata,
} from "../../scripts/adm-zip-security-lib.mjs";

function writePackage(root: string, name: string, version = "1.0.0") {
Expand Down Expand Up @@ -93,4 +95,81 @@ describe("adm-zip-security-lib", () => {
expect(result.warnings.length).toBeGreaterThan(0);
expect(listInsecureAdmZipInstalls(root)).toHaveLength(1);
});

it("readAdmZipVersion returns version from package.json", () => {
const root = mkdtempSync(join(tmpdir(), "adm-zip-lib-"));
tempDirs.push(root);
const admZipDir = join(root, "node_modules", "adm-zip");
writeAdmZip(admZipDir, "0.6.0");

expect(readAdmZipVersion(admZipDir)).toBe("0.6.0");
expect(readAdmZipVersion(join(root, "missing"))).toBeNull();
});
});

describe("validateNpmShrinkwrapMetadata", () => {
const basePkg = {
name: "harness-mcp-v2",
version: "3.2.13",
dependencies: { "adm-zip": "^0.6.0", zod: "^4.0.0" },
optionalDependencies: { "@huggingface/transformers": "^4.2.0" },
};

function makeShrinkwrap(overrides: Record<string, unknown> = {}) {
return {
packages: {
"": {
name: basePkg.name,
version: basePkg.version,
dependencies: basePkg.dependencies,
optionalDependencies: basePkg.optionalDependencies,
},
"node_modules/adm-zip": { version: "0.6.0" },
...overrides,
},
};
}

it("accepts shrinkwrap that matches package.json with secure adm-zip", () => {
expect(validateNpmShrinkwrapMetadata(makeShrinkwrap(), basePkg)).toBeNull();
});

it("rejects missing root package metadata", () => {
expect(
validateNpmShrinkwrapMetadata({ packages: {} }, basePkg),
).toBe("root package metadata is missing");
});

it("rejects version mismatch", () => {
const shrinkwrap = makeShrinkwrap();
(shrinkwrap.packages[""] as { version: string }).version = "0.0.0";
expect(validateNpmShrinkwrapMetadata(shrinkwrap, basePkg)).toMatch(/root metadata does not match/);
});

it("rejects dependency drift from package.json", () => {
const shrinkwrap = makeShrinkwrap();
(shrinkwrap.packages[""] as { dependencies: Record<string, string> }).dependencies = {
zod: "^4.0.0",
};
expect(validateNpmShrinkwrapMetadata(shrinkwrap, basePkg)).toBe(
"root dependencies do not match package.json",
);
});

it("rejects insecure hoisted adm-zip", () => {
const shrinkwrap = makeShrinkwrap({
"node_modules/adm-zip": { version: "0.5.18" },
});
expect(validateNpmShrinkwrapMetadata(shrinkwrap, basePkg)).toBe(
"hoisted adm-zip is 0.5.18",
);
});

it("rejects missing hoisted adm-zip", () => {
const shrinkwrap = makeShrinkwrap();
delete (shrinkwrap.packages as Record<string, unknown>)["node_modules/adm-zip"];
expect(validateNpmShrinkwrapMetadata(shrinkwrap, basePkg)).toBe(
"hoisted adm-zip is missing",
);
});
});
22 changes: 22 additions & 0 deletions tests/tools/tool-handlers.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1469,6 +1469,28 @@ describe("harness_delete", () => {
expect(parseResult(result)).toMatchObject({ error: expect.stringContaining("Conflicting identifiers") });
expect(mockRequest).not.toHaveBeenCalled();
});

it("maps resource_id to agent_id for gitops_agent delete", async () => {
const gitopsRegistry = new Registry(makeConfig({ HARNESS_TOOLSETS: "gitops" }));
const gitopsServer = makeMcpServer("accept");
const { registerDeleteTool } = await import("../../src/tools/harness-delete.js");
registerDeleteTool(gitopsServer, gitopsRegistry, client, makeConfig());

const result = await gitopsServer.call("harness_delete", {
resource_type: "gitops_agent",
resource_id: "myagent",
resource_scope: "account",
confirm: true,
});

expect(result.isError).toBeUndefined();
expect(mockRequest).toHaveBeenCalledTimes(1);
const call = mockRequest.mock.calls[0]![0] as { method: string; path: string; params: Record<string, unknown> };
expect(call.method).toBe("DELETE");
expect(call.path).toBe("/gitops/api/v1/agents/myagent");
expect(call.params.orgIdentifier).toBeUndefined();
expect(call.params.projectIdentifier).toBeUndefined();
});
});

describe("harness_execute", () => {
Expand Down
Loading