diff --git a/experiments/prompt-bfcl-ralph-matrix/run_prompt_bfcl_ralph_matrix.py b/experiments/prompt-bfcl-ralph-matrix/run_prompt_bfcl_ralph_matrix.py index eef6cbab..ab455b8f 100644 --- a/experiments/prompt-bfcl-ralph-matrix/run_prompt_bfcl_ralph_matrix.py +++ b/experiments/prompt-bfcl-ralph-matrix/run_prompt_bfcl_ralph_matrix.py @@ -363,9 +363,8 @@ def build_child_env(entry: dict[str, Any]) -> tuple[dict[str, str], list[str]]: if kind == "grok": api_key = env_value_from_entry(entry, "api_key") - env_name = entry.get("api_key_env", "GROK_API_KEY") if not api_key: - missing.append(str(env_name)) + missing.append("API key") else: child_env["GROK_API_KEY"] = api_key child_env["OPENAI_API_KEY"] = api_key @@ -376,16 +375,15 @@ def build_child_env(entry: dict[str, Any]) -> tuple[dict[str, str], list[str]]: return child_env, missing api_key = env_value_from_entry(entry, "api_key") - env_name = entry.get("api_key_env", "OPENAI_COMPATIBLE_API_KEY") if not api_key: - missing.append(str(env_name)) + missing.append("API key") else: child_env["OPENAI_COMPATIBLE_API_KEY"] = api_key child_env["OPENAI_API_KEY"] = api_key base_url = env_value_from_entry(entry, "base_url") if not base_url: - missing.append(str(entry.get("base_url_env", "base_url"))) + missing.append("base URL") headers = env_value_from_entry(entry, "default_headers_json") if headers: @@ -1639,9 +1637,9 @@ def run_single_model( runtime_root = matrix_runs_root / run_slug runtime_root.mkdir(parents=True, exist_ok=True) - child_env, missing_env = build_child_env(entry) + child_env, missing_config = build_child_env(entry) started_at = utc_now() - if missing_env: + if missing_config: ended_at = utc_now() return make_run_record( entry=entry, @@ -1651,7 +1649,7 @@ def run_single_model( ended_at=ended_at, duration_sec=0.0, status="failed", - error_message=f"Missing required env vars: {', '.join(missing_env)}", + error_message=f"Missing required configuration: {', '.join(missing_config)}", summary=None, ) diff --git a/experiments/prompt-bfcl-ralph-matrix/test_run_prompt_bfcl_ralph_matrix.py b/experiments/prompt-bfcl-ralph-matrix/test_run_prompt_bfcl_ralph_matrix.py index 90775182..9128d1c6 100644 --- a/experiments/prompt-bfcl-ralph-matrix/test_run_prompt_bfcl_ralph_matrix.py +++ b/experiments/prompt-bfcl-ralph-matrix/test_run_prompt_bfcl_ralph_matrix.py @@ -65,19 +65,57 @@ def test_load_and_select_models_filters_enabled_and_requested(self) -> None: selected = select_models(models, {"grok-1"}) self.assertEqual([item["id"] for item in selected], ["grok-1"]) - def test_build_child_env_for_openai_compatible_checks_missing_envs(self) -> None: + def test_build_child_env_reports_static_missing_configuration(self) -> None: entry = { "id": "openrouter-qwen", "kind": "openai-compatible", "enabled": True, "provider_name": "OpenRouter", "model_name": "qwen/test", - "base_url_env": "OPENROUTER_BASE_URL", - "api_key_env": "OPENROUTER_API_KEY", + "base_url_env": "PRIVATE_BASE_URL_NAME", + "api_key_env": "PASSWORD_VALUE_THAT_MUST_NOT_BE_LOGGED", } - with patch.dict("os.environ", {}, clear=False): + with patch.dict("os.environ", {}, clear=True): _env, missing = build_child_env(entry) - self.assertEqual(missing, ["OPENROUTER_API_KEY", "OPENROUTER_BASE_URL"]) + self.assertEqual(missing, ["API key", "base URL"]) + self.assertNotIn(entry["api_key_env"], missing) + self.assertNotIn(entry["base_url_env"], missing) + + def test_build_child_env_reports_static_missing_grok_key(self) -> None: + entry = { + "id": "grok-4", + "kind": "grok", + "model_name": "grok-4-latest", + "api_key_env": "GROK_PASSWORD_VALUE_THAT_MUST_NOT_BE_LOGGED", + } + with patch.dict("os.environ", {}, clear=True): + _env, missing = build_child_env(entry) + self.assertEqual(missing, ["API key"]) + self.assertNotIn(entry["api_key_env"], missing) + + def test_missing_configuration_record_excludes_configured_env_names(self) -> None: + entry = { + "id": "openrouter-qwen", + "kind": "openai-compatible", + "model_name": "qwen/test", + "base_url_env": "PRIVATE_BASE_URL_NAME", + "api_key_env": "PASSWORD_VALUE_THAT_MUST_NOT_BE_LOGGED", + } + with tempfile.TemporaryDirectory() as td, patch.dict( + "os.environ", {}, clear=True + ): + record = run_single_model( + entry=entry, + args=SimpleNamespace(), + matrix_runs_root=Path(td), + ) + + self.assertEqual( + record["error_message"], + "Missing required configuration: API key, base URL", + ) + self.assertNotIn(entry["api_key_env"], record["error_message"]) + self.assertNotIn(entry["base_url_env"], record["error_message"]) def test_build_child_env_prepends_cli_paths(self) -> None: entry = { diff --git a/scripts/datadog-assets.mjs b/scripts/datadog-assets.mjs index 542b26e8..643c2734 100644 --- a/scripts/datadog-assets.mjs +++ b/scripts/datadog-assets.mjs @@ -85,16 +85,33 @@ async function datadogRequest( return response.status === 204 ? null : response.json(); } +function configuredCredentialStatuses() { + let api = "not_configured"; + let application = "not_configured"; + if (apiKey) { + api = "configured"; + } + if (appKey) { + application = "configured"; + } + return { api, application }; +} + async function validateCredentials() { - const apiValidation = apiKey - ? await datadogRequest("GET", "/api/v1/validate", undefined, { - requireAppKey: false, - }) - : { valid: false, skipped: true }; + let api = "not_configured"; + if (apiKey) { + const validation = await datadogRequest( + "GET", + "/api/v1/validate", + undefined, + { requireAppKey: false } + ); + api = validation?.valid === true ? "valid" : "invalid"; + } return { - apiKeyValid: Boolean(apiValidation?.valid), - appKeyConfigured: Boolean(appKey), + api, + application: configuredCredentialStatuses().application, }; } @@ -153,10 +170,7 @@ async function main() { prefix, dashboard: assets.dashboard.title, monitors: assets.monitors.map((monitor) => monitor.name), - credentials: { - apiKeyConfigured: Boolean(apiKey), - appKeyConfigured: Boolean(appKey), - }, + credentials: configuredCredentialStatuses(), }, null, 2 diff --git a/src/__tests__/core/utils/xml-root-repair.unit.test.ts b/src/__tests__/core/utils/xml-root-repair.unit.test.ts new file mode 100644 index 00000000..4b673bdd --- /dev/null +++ b/src/__tests__/core/utils/xml-root-repair.unit.test.ts @@ -0,0 +1,49 @@ +import { describe, expect, it } from "vitest"; +import { tryRepairXmlSelfClosingRootWithBody } from "../../../core/utils/xml-root-repair"; + +describe("xml-root-repair", () => { + it("repairs LF and CRLF malformed roots while preserving body indentation", () => { + expect( + tryRepairXmlSelfClosingRootWithBody("", [ + "get_weather", + ]) + ).toBe("\n city: Seoul\n"); + + expect( + tryRepairXmlSelfClosingRootWithBody( + "Seoul\r\n />", + ["get_weather"] + ) + ).toBe("\n Seoul\n"); + }); + + it("rejects non-tool roots, empty bodies, and existing closing tags", () => { + expect( + tryRepairXmlSelfClosingRootWithBody("", [ + "get_weather", + ]) + ).toBeNull(); + expect( + tryRepairXmlSelfClosingRootWithBody("", [ + "get_weather", + ]) + ).toBeNull(); + expect( + tryRepairXmlSelfClosingRootWithBody( + "\n/>", + ["get_weather"] + ) + ).toBeNull(); + }); + + it("rejects CodeQL adversarial whitespace inputs without backtracking", () => { + const inputs = [ + `\s*$/; +function isAsciiLetterOrUnderscore(character: string): boolean { + const code = character.charCodeAt(0); + return ( + character === "_" || + (code >= 65 && code <= 90) || + (code >= 97 && code <= 122) + ); +} + +function isValidRootTag(tag: string): boolean { + if (tag.length === 0 || !isAsciiLetterOrUnderscore(tag[0])) { + return false; + } + + for (let index = 1; index < tag.length; index += 1) { + const character = tag[index]; + const code = character.charCodeAt(0); + const isDigit = code >= 48 && code <= 57; + if ( + !(isAsciiLetterOrUnderscore(character) || isDigit) && + character !== "-" + ) { + return false; + } + } + + return true; +} export function tryRepairXmlSelfClosingRootWithBody( rawText: string, @@ -10,18 +36,32 @@ export function tryRepairXmlSelfClosingRootWithBody( return null; } - const match = trimmed.match(XML_SELF_CLOSING_ROOT_WITH_BODY_REGEX); - if (!match) { + const openingLineEnd = trimmed.indexOf("\n"); + const closingLineStart = trimmed.lastIndexOf("\n"); + if (openingLineEnd === -1 || closingLineStart <= openingLineEnd) { + return null; + } + + let openingLine = trimmed.slice(0, openingLineEnd); + if (openingLine.endsWith("\r")) { + openingLine = openingLine.slice(0, -1); + } + if (!openingLine.startsWith("<")) { + return null; + } + + const rootTag = openingLine.slice(1).trimEnd(); + if (!(isValidRootTag(rootTag) && toolNames.includes(rootTag))) { return null; } - const rootTag = match[1]; - if (!toolNames.includes(rootTag)) { + const closingLine = trimmed.slice(closingLineStart + 1); + if (closingLine.trimStart() !== "/>") { return null; } // Keep leading indentation intact for YAML payloads. - const body = match[2].trimEnd(); + const body = trimmed.slice(openingLineEnd + 1, closingLineStart).trimEnd(); if (body.trim().length === 0 || body.includes(``)) { return null; } diff --git a/tests/datadog-assets.test.ts b/tests/datadog-assets.test.ts new file mode 100644 index 00000000..b6b20a50 --- /dev/null +++ b/tests/datadog-assets.test.ts @@ -0,0 +1,74 @@ +import { spawnSync } from "node:child_process"; +import { fileURLToPath, pathToFileURL } from "node:url"; +import { describe, expect, it } from "vitest"; + +const scriptPath = fileURLToPath( + new URL("../scripts/datadog-assets.mjs", import.meta.url) +); + +function runScript( + args: string[], + credentialEnv: Record, + evalSource?: string +) { + return spawnSync(process.execPath, args, { + encoding: "utf8", + env: { + ...process.env, + DD_API_KEY: "", + DD_APP_KEY: "", + ...credentialEnv, + }, + input: evalSource, + }); +} + +describe("datadog asset CLI credential output", () => { + it("reports only static credential statuses in plan mode", () => { + const apiSecret = "api-secret-that-must-not-appear"; + const applicationSecret = "app-secret-that-must-not-appear"; + const result = runScript([scriptPath, "plan"], { + DD_API_KEY: apiSecret, + DD_APP_KEY: applicationSecret, + }); + + expect(result.status).toBe(0); + expect(result.stderr).toBe(""); + expect(JSON.parse(result.stdout).credentials).toEqual({ + api: "configured", + application: "configured", + }); + expect(result.stdout).not.toContain(apiSecret); + expect(result.stdout).not.toContain(applicationSecret); + }); + + it("reports validation outcomes without exposing credential values", () => { + const apiSecret = "validated-api-secret-that-must-not-appear"; + const applicationSecret = "validated-app-secret-that-must-not-appear"; + const scriptUrl = pathToFileURL(scriptPath).href; + const source = ` + globalThis.fetch = async () => ({ + ok: true, + status: 200, + json: async () => ({ valid: true }), + text: async () => "", + }); + process.argv[2] = "validate"; + await import(${JSON.stringify(scriptUrl)}); + `; + const result = runScript( + ["--input-type=module"], + { DD_API_KEY: apiSecret, DD_APP_KEY: applicationSecret }, + source + ); + + expect(result.status).toBe(0); + expect(result.stderr).toBe(""); + expect(JSON.parse(result.stdout)).toEqual({ + api: "valid", + application: "configured", + }); + expect(result.stdout).not.toContain(apiSecret); + expect(result.stdout).not.toContain(applicationSecret); + }); +});