Skip to content
Merged
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
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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:
Expand Down Expand Up @@ -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,
Expand All @@ -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,
)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand Down
36 changes: 25 additions & 11 deletions scripts/datadog-assets.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};
}

Expand Down Expand Up @@ -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
Expand Down
49 changes: 49 additions & 0 deletions src/__tests__/core/utils/xml-root-repair.unit.test.ts
Original file line number Diff line number Diff line change
@@ -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\n city: Seoul\n/>", [
"get_weather",
])
).toBe("<get_weather>\n city: Seoul\n</get_weather>");

expect(
tryRepairXmlSelfClosingRootWithBody(
"<get_weather \r\n <city>Seoul</city>\r\n />",
["get_weather"]
)
).toBe("<get_weather>\n <city>Seoul</city>\n</get_weather>");
});

it("rejects non-tool roots, empty bodies, and existing closing tags", () => {
expect(
tryRepairXmlSelfClosingRootWithBody("<unknown\nvalue: 1\n/>", [
"get_weather",
])
).toBeNull();
expect(
tryRepairXmlSelfClosingRootWithBody("<get_weather\n \n/>", [
"get_weather",
])
).toBeNull();
expect(
tryRepairXmlSelfClosingRootWithBody(
"<get_weather\nvalue: 1\n</get_weather>\n/>",
["get_weather"]
)
).toBeNull();
});

it("rejects CodeQL adversarial whitespace inputs without backtracking", () => {
const inputs = [
`<A\n${"\n ".repeat(50_000)}not-a-closing-line`,
`<A\na\n${" \n".repeat(50_000)}not-a-closing-line`,
];

for (const input of inputs) {
expect(tryRepairXmlSelfClosingRootWithBody(input, ["A"])).toBeNull();
}
});
});
54 changes: 47 additions & 7 deletions src/core/utils/xml-root-repair.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,31 @@
const XML_SELF_CLOSING_ROOT_WITH_BODY_REGEX =
/^<([A-Za-z_][A-Za-z0-9_-]*)\s*\r?\n([\s\S]+?)\r?\n\s*\/>\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,
Expand All @@ -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(`</${rootTag}>`)) {
return null;
}
Expand Down
74 changes: 74 additions & 0 deletions tests/datadog-assets.test.ts
Original file line number Diff line number Diff line change
@@ -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<string, string>,
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);
});
});
Loading