fix: harden evidence secret file handoff

This commit is contained in:
2026-08-09 19:27:52 +02:00
parent e50aad7e41
commit 3b9681a63a
4 changed files with 83 additions and 24 deletions
+12 -11
View File
@@ -60,18 +60,18 @@ function isInside(path: string, root: string): boolean {
return pathRelative !== "" && !pathRelative.startsWith("..") && !isAbsolute(pathRelative); return pathRelative !== "" && !pathRelative.startsWith("..") && !isAbsolute(pathRelative);
} }
function isSafeSecretFile(path: string, secretRoots: readonly string[]): boolean { function safeSecretFilePath(path: string, secretRoots: readonly string[]): string | undefined {
if (!isAbsolute(path)) return false; if (!isAbsolute(path)) return undefined;
try { try {
const resolvedPath = realpathSync(path); const resolvedPath = realpathSync(path);
const resolvedRoots = secretRoots.map((root) => realpathSync(root)); const resolvedRoots = secretRoots.map((root) => realpathSync(root));
if (!resolvedRoots.some((root) => isInside(resolvedPath, root))) return false; if (!resolvedRoots.some((root) => isInside(resolvedPath, root))) return undefined;
if (!statSync(resolvedPath).isFile()) return false; if (!statSync(resolvedPath).isFile()) return undefined;
accessSync(resolvedPath, constants.R_OK); accessSync(resolvedPath, constants.R_OK);
return true; return resolvedPath;
} catch { } catch {
return false; return undefined;
} }
} }
@@ -132,11 +132,12 @@ export function resolveBinding(
const value = env[variable.name]; const value = env[variable.name];
const present = value !== undefined && value.trim() !== ""; const present = value !== undefined && value.trim() !== "";
const safe = !variable.secret || (present && isSafeSecretFile(value, secretRoots)); const safePath = variable.secret && present ? safeSecretFilePath(value, secretRoots) : undefined;
const safe = !variable.secret || safePath !== undefined;
if ((required.has(variable.suffix) && !present) || (present && !safe)) { if ((required.has(variable.suffix) && !present) || (present && !safe)) {
missing.push(variable.name); missing.push(variable.name);
} }
if (present && safe) values[variable.name] = value; if (present && safe) values[variable.name] = variable.secret ? safePath! : value;
} }
return { transport: selectedTransport, values, missing }; return { transport: selectedTransport, values, missing };
@@ -166,11 +167,11 @@ export function resolveEvidenceBinding(
for (const variable of variables) { for (const variable of variables) {
const value = env[variable.name]; const value = env[variable.name];
const present = value !== undefined && value.trim() !== ""; const present = value !== undefined && value.trim() !== "";
const safe = present && isSafeSecretFile(value, secretRoots); const safePath = present ? safeSecretFilePath(value, secretRoots) : undefined;
if ((required.has(variable.suffix) && !present) || (present && !safe)) { if ((required.has(variable.suffix) && !present) || (present && safePath === undefined)) {
missing.push(variable.name); missing.push(variable.name);
} }
if (safe) values[variable.name] = value; if (safePath !== undefined) values[variable.name] = safePath;
} }
return { values, missing }; return { values, missing };
} }
+22 -6
View File
@@ -1,4 +1,4 @@
import { chmodSync, mkdirSync, mkdtempSync, rmSync, symlinkSync, writeFileSync } from "node:fs"; import { chmodSync, mkdirSync, mkdtempSync, realpathSync, rmSync, symlinkSync, writeFileSync } from "node:fs";
import { tmpdir } from "node:os"; import { tmpdir } from "node:os";
import { join } from "node:path"; import { join } from "node:path";
import { afterEach, expect, test } from "vitest"; import { afterEach, expect, test } from "vitest";
@@ -153,7 +153,7 @@ test("resolves direct bindings from the stable workspace namespace", () => {
values: { values: {
THT_WS_PSD_CLINICAL_DWH_HOST: "dwh.internal", THT_WS_PSD_CLINICAL_DWH_HOST: "dwh.internal",
THT_WS_PSD_CLINICAL_DWH_PORT: "5432", THT_WS_PSD_CLINICAL_DWH_PORT: "5432",
THT_WS_PSD_CLINICAL_DWH_PASSWORD_FILE: password.path, THT_WS_PSD_CLINICAL_DWH_PASSWORD_FILE: realpathSync(password.path),
}, },
}); });
}); });
@@ -304,10 +304,26 @@ test("requires only a safe HTTP signed-URL file and never reads its contents", (
[variable]: signed.path, [variable]: signed.path,
[evidenceVariable("ACCESS_KEY_FILE")]: signed.path, [evidenceVariable("ACCESS_KEY_FILE")]: signed.path,
}, [signed.root]); }, [signed.root]);
expect(resolved).toEqual({ values: { [variable]: signed.path }, missing: [] }); expect(resolved).toEqual({ values: { [variable]: realpathSync(signed.path) }, missing: [] });
expect(JSON.stringify(resolved)).not.toContain("CANARY-SIGNED-URL-CONTENT"); expect(JSON.stringify(resolved)).not.toContain("CANARY-SIGNED-URL-CONTENT");
}); });
test("canonicalizes an in-root Evidence symlink before passing it to the harness", () => {
const signed = secretPath("evidence-signed-target");
const link = join(signed.root, "signed-urls-link");
symlinkSync(signed.path, link);
const source = withEvidence({
type: "http",
uris: ["https://evidence.example.test/guide.md"],
authentication: "signed_urls_file",
});
const variable = evidenceVariable("SIGNED_URLS_FILE");
expect(resolveEvidenceBinding(source, { [variable]: link }, [signed.root])).toEqual({
values: { [variable]: realpathSync(signed.path) }, missing: [],
});
});
test("requires S3 access and secret files together while accepting an optional safe session token", () => { test("requires S3 access and secret files together while accepting an optional safe session token", () => {
const access = secretPath("evidence-access"); const access = secretPath("evidence-access");
const secret = secretPath("evidence-secret"); const secret = secretPath("evidence-secret");
@@ -327,9 +343,9 @@ test("requires S3 access and secret files together while accepting an optional s
}, [access.root]).missing).toEqual([evidenceVariable("SECRET_KEY_FILE")]); }, [access.root]).missing).toEqual([evidenceVariable("SECRET_KEY_FILE")]);
expect(resolveEvidenceBinding(source, env, [access.root, secret.root, token.root])).toEqual({ expect(resolveEvidenceBinding(source, env, [access.root, secret.root, token.root])).toEqual({
values: { values: {
[evidenceVariable("ACCESS_KEY_FILE")]: access.path, [evidenceVariable("ACCESS_KEY_FILE")]: realpathSync(access.path),
[evidenceVariable("SECRET_KEY_FILE")]: secret.path, [evidenceVariable("SECRET_KEY_FILE")]: realpathSync(secret.path),
[evidenceVariable("SESSION_TOKEN_FILE")]: token.path, [evidenceVariable("SESSION_TOKEN_FILE")]: realpathSync(token.path),
}, },
missing: [], missing: [],
}); });
@@ -223,6 +223,37 @@ def test_signed_http_rejects_reordered_extra_mismatch_userinfo_and_duplicate_pro
assert_no_canaries(caught.value) assert_no_canaries(caught.value)
def test_s3_rejects_inline_credentials_and_never_discloses_them(tmp_path):
path = write_config(tmp_path, {
"type": "s3", "bucket": "clinical-evidence",
"access_key": ACCESS_CANARY,
"secret_key": SECRET_CANARY,
"session_token": TOKEN_CANARY,
})
with pytest.raises(ConfigError) as caught:
load_config(path)
assert "file" in str(caught.value).lower()
assert_no_canaries(caught.value)
assert_no_canaries("".join(traceback.format_exception(caught.value)))
def test_s3_scalar_secret_files_are_bounded(tmp_path):
access = tmp_path / "oversized-access-key"
access.write_bytes(b"A" * (64 * 1024 + 1))
secret = tmp_path / "secret-key"
secret.write_text("bounded-secret")
path = write_config(tmp_path, {
"type": "s3", "bucket": "clinical-evidence",
"access_key_file": str(access), "secret_key_file": str(secret),
})
with pytest.raises(ConfigError) as caught:
load_config(path)
assert "secret file" in str(caught.value).lower()
assert "bounded-secret" not in str(caught.value)
def test_s3_ambient_and_static_file_credentials_are_secret_typed(tmp_path): def test_s3_ambient_and_static_file_credentials_are_secret_typed(tmp_path):
ambient = load_config(write_config(tmp_path, { ambient = load_config(write_config(tmp_path, {
"type": "s3", "bucket": "clinical-evidence", "prefix": "published/", "type": "s3", "bucket": "clinical-evidence", "prefix": "published/",
+18 -7
View File
@@ -49,13 +49,17 @@ def _expand_env(value: Any) -> Any:
_MAX_SIGNED_URL_FILE_BYTES = 1024 * 1024 _MAX_SIGNED_URL_FILE_BYTES = 1024 * 1024
def _resolve_http_signed_url_files(value: Any) -> Any: def _resolve_evidence_secret_files(value: Any) -> Any:
"""Resolve only signed HTTP URL arrays, keeping their values out of public errors.""" """Resolve only signed HTTP URL arrays, keeping their values out of public errors."""
if isinstance(value, dict): if isinstance(value, dict):
resolved = { resolved = {
key: _resolve_http_signed_url_files(item) key: _resolve_evidence_secret_files(item)
for key, item in value.items() for key, item in value.items()
} }
if resolved.get("type") == "s3" and any(
name in resolved for name in ("access_key", "secret_key", "session_token")
):
raise ConfigError("S3 Evidence credentials require *_file references")
if resolved.get("type") != "http" or "signed_urls_file" not in resolved: if resolved.get("type") != "http" or "signed_urls_file" not in resolved:
return resolved return resolved
if "urls" in resolved: if "urls" in resolved:
@@ -94,10 +98,13 @@ def _resolve_http_signed_url_files(value: Any) -> Any:
resolved["urls"] = parsed resolved["urls"] = parsed
return resolved return resolved
if isinstance(value, list): if isinstance(value, list):
return [_resolve_http_signed_url_files(item) for item in value] return [_resolve_evidence_secret_files(item) for item in value]
return value return value
_MAX_SCALAR_SECRET_FILE_BYTES = 64 * 1024
def _resolve_secret_files(value: Any) -> Any: def _resolve_secret_files(value: Any) -> Any:
if isinstance(value, dict): if isinstance(value, dict):
resolved = {key: _resolve_secret_files(item) for key, item in value.items()} resolved = {key: _resolve_secret_files(item) for key, item in value.items()}
@@ -109,9 +116,13 @@ def _resolve_secret_files(value: Any) -> Any:
raise ConfigError(f"{secret_name} and {file_name} are mutually exclusive") raise ConfigError(f"{secret_name} and {file_name} are mutually exclusive")
path = Path(resolved.pop(file_name)) path = Path(resolved.pop(file_name))
try: try:
secret = path.read_text() with path.open("rb") as stream:
except (OSError, UnicodeError) as exc: payload = stream.read(_MAX_SCALAR_SECRET_FILE_BYTES + 1)
raise ConfigError(f"Cannot read secret file: {path}") from exc if len(payload) > _MAX_SCALAR_SECRET_FILE_BYTES:
raise OSError
secret = payload.decode("utf-8")
except (OSError, UnicodeError):
raise ConfigError(f"Cannot read secret file: {path}") from None
if not secret or any(char.isspace() for char in secret) or "\x00" in secret: if not secret or any(char.isspace() for char in secret) or "\x00" in secret:
raise ConfigError(f"Invalid secret file: {path}") raise ConfigError(f"Invalid secret file: {path}")
resolved[secret_name] = secret resolved[secret_name] = secret
@@ -532,7 +543,7 @@ def load_config(path: Path) -> Config:
raise ConfigError(f"Configurazione YAML non valida: {path}") from exc raise ConfigError(f"Configurazione YAML non valida: {path}") from exc
if not isinstance(raw, dict): if not isinstance(raw, dict):
raise ConfigError(f"Configurazione non valida (atteso un mapping YAML): {path}") raise ConfigError(f"Configurazione non valida (atteso un mapping YAML): {path}")
expanded = _resolve_secret_files(_resolve_http_signed_url_files(_expand_env(raw))) expanded = _resolve_secret_files(_resolve_evidence_secret_files(_expand_env(raw)))
_validate_internal_embedding_contract(expanded, path) _validate_internal_embedding_contract(expanded, path)
_validate_internal_vector_contract(expanded, path) _validate_internal_vector_contract(expanded, path)
translated, used_legacy = translate_legacy_config(expanded) translated, used_legacy = translate_legacy_config(expanded)