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
8 changes: 8 additions & 0 deletions docs/coding-standards.md
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,14 @@ Every `ResourceDefinition` declares a `scope`: `"project"`, `"org"`, or `"accoun

The `Registry.dispatch()` method handles scope injection automatically based on the definition. Do NOT manually inject `accountIdentifier`, `orgIdentifier`, or `projectIdentifier` in toolset specs.

**Multi-scope resources** (`supportedScopes: ["account", "org", "project"]`) with custom `pathBuilder` functions that embed scope in the URL path (not only query params) must:

- Accept `PathBuilderConfig` as the second argument: `(input, config) => ...`
- Honor `config.HARNESS_ORG` / `config.HARNESS_PROJECT` when callers omit `org_id` / `project_id`, matching list-operation defaults
- Respect explicit `resource_scope` and clear unused `org_id` / `project_id` from query params when targeting account scope

Enforced by `tests/coding-standards/multi-scope-pathbuilders.test.ts`.

### 5. Identifier Fields and Deep Links

Every `ResourceDefinition` must declare:
Expand Down
68 changes: 67 additions & 1 deletion src/data/schemas/v0/trigger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2822,7 +2822,8 @@ const schema: Record<string, any> = {
"IssueComment",
"Release",
"Delete",
"Create"
"Create",
"PullRequestReview"
]
}
}
Expand Down Expand Up @@ -2922,6 +2923,22 @@ const schema: Record<string, any> = {
}
}
}
},
{
"if": {
"properties": {
"type": {
"const": "PullRequestReview"
}
}
},
"then": {
"properties": {
"spec": {
"$ref": "#/definitions/trigger/webhook_trigger/github_pr_review_spec"
}
}
}
}
],
"$schema": "http://json-schema.org/draft-07/schema#"
Expand Down Expand Up @@ -3196,6 +3213,55 @@ const schema: Record<string, any> = {
],
"$schema": "http://json-schema.org/draft-07/schema#"
},
"github_pr_review_spec": {
"title": "github_pr_review_spec",
"allOf": [
{
"$ref": "#/definitions/trigger/webhook_trigger/github_event_spec"
},
{
"type": "object",
"properties": {
"actions": {
"type": "array",
"items": {
"type": "string",
"enum": [
"Submitted",
"Edited",
"Dismissed"
]
}
},
"autoAbortPreviousExecutions": {
"type": "boolean"
},
"connectorRef": {
"type": "string"
},
"headerConditions": {
"type": "array",
"items": {
"$ref": "#/definitions/trigger/trigger_event_data"
}
},
"jexlCondition": {
"type": "string"
},
"payloadConditions": {
"type": "array",
"items": {
"$ref": "#/definitions/trigger/trigger_event_data"
}
},
"repoName": {
"type": "string"
}
}
}
],
"$schema": "http://json-schema.org/draft-07/schema#"
},
"gitlab_spec": {
"title": "gitlab_spec",
"allOf": [
Expand Down
2 changes: 1 addition & 1 deletion src/data/schemas/v1/pipeline.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4545,7 +4545,7 @@ const schema: Record<string, any> = {
"description": "Image registry connector. Supports expressions.",
"type": "string"
},
"registryRef": {
"registry": {
"description": "Harness Artifact Registry reference for the image. Supports expressions.",
"type": "string"
},
Expand Down
2 changes: 1 addition & 1 deletion src/data/schemas/v1/template.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4733,7 +4733,7 @@ const schema: Record<string, any> = {
"description": "Image registry connector. Supports expressions.",
"type": "string"
},
"registryRef": {
"registry": {
"description": "Harness Artifact Registry reference for the image. Supports expressions.",
"type": "string"
},
Expand Down
17 changes: 17 additions & 0 deletions tasks/todo.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,22 @@
# Harness MCP Server — Task Tracking

## Coding Standards Compliance Review (2026-07-19)
- [x] Audit codebase against docs/coding-standards.md
- [x] Run pnpm standards:check and full test suite
- [x] Cherry-pick strengthened guardrails (security + multi-scope pathBuilders)
- [x] Commit, push, and open PR

### Plan
- Compare user-provided standards snapshot against canonical docs/coding-standards.md (11 tools incl. harness_schema, Zod v4, registerTool).
- Rely on existing tests/coding-standards/*.test.ts guardrails; add missing §4/§9 enforcement from prior branch.
- Verify no runtime code changes needed — standards compliance is already enforced in CI.

### Review
- Baseline `pnpm standards:check` passed (80 tests) before adding strengthened guardrails.
- Cherry-picked `e92468da`: security.test.ts (§9 secret metadata, RateLimiter, redaction), multi-scope-pathbuilders.test.ts (§4 PathBuilderConfig), docs-consistency extensions (registerTool, pathBuilder docs, AGENTS.md alignment), and multi-scope pathBuilder docs in coding-standards.md.
- User-provided standards snapshot is outdated vs canonical: 10 tools (actual 11 incl. harness_schema), Zod v3 (actual v4), server.tool() (actual registerTool()).
- Verification passed: `pnpm standards:check` (12 files / 92 tests), `pnpm typecheck`, `pnpm build`, `pnpm test` (121 files / 2619 tests).

## Version Bump 3.2.12 (2026-07-19)
- [x] Update package and bundle manifest versions to 3.2.12
- [x] Update the release metadata version test
Expand Down
27 changes: 27 additions & 0 deletions tests/coding-standards/docs-consistency.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,4 +45,31 @@ describe("Coding standards — documentation consistency", () => {
it("docs/coding-standards.md forbids new harness-*.ts handler files", () => {
expect(content).toMatch(/Do NOT add new `harness-\*\.ts` handler files/);
});

it("docs/coding-standards.md documents multi-scope pathBuilder contract", () => {
expect(content).toContain("multi-scope-pathbuilders.test.ts");
expect(content).toMatch(/PathBuilderConfig/);
expect(content).toMatch(/supportedScopes:\s*\["account",\s*"org",\s*"project"\]/);
});

it("docs/coding-standards.md documents registerTool (not deprecated server.tool)", () => {
expect(content).toMatch(/server\.registerTool\(\)/);
expect(content).not.toMatch(/10 consolidated tool handlers/);
});
});

describe("Coding standards — AGENTS.md consistency", () => {
const agentsContent = readFileSync(join(REPO_ROOT, "AGENTS.md"), "utf8");

it("AGENTS.md documents 11 consolidated MCP tools including harness_schema", () => {
expect(agentsContent).toMatch(/11 consolidated tools/);
for (const tool of REQUIRED_TOOLS) {
expect(agentsContent, `missing ${tool} in AGENTS.md`).toContain(tool);
}
});

it("AGENTS.md references Zod v4 and standards:check", () => {
expect(agentsContent).toMatch(/Zod v4/);
expect(agentsContent).toContain("pnpm standards:check");
});
});
112 changes: 112 additions & 0 deletions tests/coding-standards/multi-scope-pathbuilders.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
/**
* Multi-scope pathBuilder contract — docs/coding-standards.md §4.
*
* Custom pathBuilders that encode account/org/project in the URL path must accept
* PathBuilderConfig and honor HARNESS_ORG / HARNESS_PROJECT defaults. Otherwise
* list → get/update flows can target a different scope than the caller listed.
*/
import { describe, it, expect } from "vitest";
import { readFileSync, readdirSync, statSync } from "node:fs";
import { join, relative } from "node:path";

const REPO_ROOT = join(import.meta.dirname, "../..");
const TOOLSET_DIR = join(REPO_ROOT, "src/registry/toolsets");

const TOOLSET_HELPER_FILES = new Set([
"src/registry/toolsets/chaos-descriptions.ts",
"src/registry/toolsets/scopes.ts",
]);

/** Heuristics for pathBuilders that embed scope in the path (not query params). */
const SCOPE_ENCODING_PATTERNS = [
/let\s+scope\s*=/,
/scope\s*\+=/,
/HARNESS_ORG/,
/HARNESS_PROJECT/,
/templateV1BasePathFromScope/,
/effectiveOrgId/,
/shouldUseProject/,
/shouldUseOrg/,
/\/v1\/entities\/\$\{/,
];

function walkTsFiles(dir: string): string[] {
const results: string[] = [];
for (const entry of readdirSync(dir)) {
const full = join(dir, entry);
const stat = statSync(full);
if (stat.isDirectory()) {
results.push(...walkTsFiles(full));
} else if (entry.endsWith(".ts")) {
results.push(full);
}
}
return results;
}

function rel(path: string): string {
return relative(REPO_ROOT, path).replace(/\\/g, "/");
}

function encodesScopeInPath(snippet: string): boolean {
return SCOPE_ENCODING_PATTERNS.some((pattern) => pattern.test(snippet));
}

describe("Coding standards — multi-scope pathBuilders", () => {
it("named pathBuilder helpers that encode scope accept PathBuilderConfig", () => {
const violations: string[] = [];

for (const file of walkTsFiles(TOOLSET_DIR)) {
const fileRel = rel(file);
if (TOOLSET_HELPER_FILES.has(fileRel)) continue;

const content = readFileSync(file, "utf8");
const pathBuilderRefs = [...content.matchAll(/pathBuilder:\s*(\w+)/g)].map((m) => m[1]!);

for (const fnName of pathBuilderRefs) {
const sigRe = new RegExp(`const\\s+${fnName}\\s*=\\s*\\(([^)]*)\\)`);
const sigMatch = sigRe.exec(content);
if (!sigMatch) continue;

const params = sigMatch[1]!;
const fnSnippet = content.slice(sigMatch.index!, sigMatch.index! + 1200);
if (encodesScopeInPath(fnSnippet) && !/\bconfig\b/.test(params)) {
violations.push(
`${fileRel}: ${fnName} encodes scope in the path but lacks PathBuilderConfig — pass (input, config) and honor HARNESS_ORG/HARNESS_PROJECT`,
);
}
}
}

expect(violations, violations.join("\n")).toEqual([]);
});

it("inline pathBuilders on multi-scope resources that encode scope accept config", () => {
const violations: string[] = [];

for (const file of walkTsFiles(TOOLSET_DIR)) {
const fileRel = rel(file);
if (TOOLSET_HELPER_FILES.has(fileRel)) continue;

const content = readFileSync(file, "utf8");
if (!contentHasMultiScopeResource(content)) continue;

for (const match of content.matchAll(/pathBuilder:\s*\(([^)]*)\)\s*=>/g)) {
const params = match[1]!;
const start = match.index ?? 0;
const snippet = content.slice(start, start + 1200);
if (encodesScopeInPath(snippet) && !/\bconfig\b/.test(params)) {
violations.push(
`${fileRel}: inline pathBuilder encodes scope but params are (${params}) — use (input, config)`,
);
}
}
}

expect(violations, violations.join("\n")).toEqual([]);
});
});

function contentHasMultiScopeResource(content: string): boolean {
return /supportedScopes:\s*\[[^\]]*,/.test(content);
}
74 changes: 74 additions & 0 deletions tests/coding-standards/security.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
/**
* Safety and security rules from docs/coding-standards.md §9.
*
* Complements registry-metadata write-handler checks with secret exposure,
* client-side rate limiting, and response redaction guardrails.
*/
import { describe, it, expect } from "vitest";
import { readFileSync } from "node:fs";
import { join } from "node:path";
import { Registry } from "../../src/registry/index.js";

const REPO_ROOT = join(import.meta.dirname, "../..");

const MINIMAL_CONFIG = {
HARNESS_API_KEY: "pat.testaccount.testtoken.testsecret",
HARNESS_BASE_URL: "https://app.harness.io",
} as const;

/** Schema/body field names that must never appear in secret toolset specs. */
const FORBIDDEN_SECRET_VALUE_FIELDS = [
"secretValue",
"secretText",
"decryptedValue",
"plaintext",
] as const;

describe("Coding standards — safety and security (§9)", () => {
const registry = new Registry(MINIMAL_CONFIG);

it("secret resource is read-only metadata (list + get only)", () => {
const def = registry.getResource("secret");
const ops = Object.keys(def.operations);

expect(ops.sort()).toEqual(["get", "list"]);
expect(def.executeActions ?? {}).toEqual({});
});

it("secret resource description states values are never exposed", () => {
const def = registry.getResource("secret");
expect(def.description.toLowerCase()).toMatch(/never|not returned|metadata/);
});

it("HarnessClient enforces client-side rate limiting via RateLimiter", () => {
const content = readFileSync(join(REPO_ROOT, "src/client/harness-client.ts"), "utf8");
expect(content).toMatch(/import\s*\{[^}]*RateLimiter[^}]*\}\s*from/);
expect(content).toMatch(/new\s+RateLimiter\s*\(/);
});

it("HarnessClient redacts sensitive fields in logged API bodies by default", () => {
const content = readFileSync(join(REPO_ROOT, "src/client/harness-client.ts"), "utf8");
expect(content).toContain("redactJsonString");
expect(content).toMatch(/logUnsafeBodies\s*\?\s*[^:]+:\s*redactJsonString/);
});

it("redact utility covers common credential key patterns", () => {
const content = readFileSync(join(REPO_ROOT, "src/utils/redact.ts"), "utf8");
expect(content).toMatch(/api[_-]?key|access[_-]?token|password|secret/i);
expect(content).toContain("redactSensitiveFields");
});

it("secrets toolset does not define body fields that expose secret values", () => {
const content = readFileSync(
join(REPO_ROOT, "src/registry/toolsets/secrets.ts"),
"utf8",
);

for (const key of FORBIDDEN_SECRET_VALUE_FIELDS) {
expect(
content,
`secrets toolset must not define exposed value field "${key}"`,
).not.toMatch(new RegExp(`\\b${key}\\s*:`));
}
});
});
Loading