Skip to content

Commit 4d4fb1b

Browse files
committed
Validate creative-opportunity slots at build time
build.rs deserialized slots into a stub whose validate_runtime was a no-op and only checked slot-id syntax, so an invalid trusted-server.toml or TRUSTED_SERVER__CREATIVE_OPPORTUNITIES__SLOT override (empty page_patterns, empty formats, zero dimensions, empty resolved GAM unit path) passed CI and got embedded, then failed at request time as a configuration error. Extract the validation into creative_slot_build_check, shared by build.rs (via #[path]) and the crate test build (via #[cfg(test)] mod) so the rules run under cargo test. It mirrors CreativeOpportunitySlot::validate_runtime and runs against the merged config (base TOML plus TRUSTED_SERVER__* env overrides) before the config is serialized and embedded, so an invalid slot fails the build and is never persisted.
1 parent 89281f0 commit 4d4fb1b

3 files changed

Lines changed: 232 additions & 26 deletions

File tree

crates/trusted-server-core/build.rs

Lines changed: 27 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,10 @@ mod creative_opportunities {
100100
#[path = "src/settings.rs"]
101101
mod settings;
102102

103+
#[path = "src/creative_slot_build_check.rs"]
104+
mod creative_slot_build_check;
105+
106+
use creative_slot_build_check::validate_creative_slot;
103107
use std::fs;
104108
use std::path::Path;
105109

@@ -118,38 +122,24 @@ fn main() {
118122
let toml_content = fs::read_to_string(init_config_path)
119123
.unwrap_or_else(|_| panic!("Failed to read {init_config_path:?}"));
120124

121-
// Merge base TOML with environment variable overrides and write output.
125+
// Merge base TOML with environment variable overrides.
122126
// Panics if admin endpoints are not covered by a handler.
123127
let settings = settings::Settings::from_toml_and_env(&toml_content)
124128
.expect("Failed to parse settings at build time");
125129

126-
let merged_toml =
127-
toml::to_string_pretty(&settings).expect("Failed to serialize settings to TOML");
128-
129-
// Only write when content changes to avoid unnecessary recompilation.
130-
let dest_path = Path::new(TRUSTED_SERVER_OUTPUT_CONFIG_PATH);
131-
let current = fs::read_to_string(dest_path).unwrap_or_default();
132-
if current != merged_toml {
133-
fs::write(dest_path, merged_toml)
134-
.unwrap_or_else(|_| panic!("Failed to write {dest_path:?}"));
135-
}
136-
137-
// Validate slot IDs from [creative_opportunities.slot] in trusted-server.toml
138-
let slot_id_re = regex::Regex::new(r"^[A-Za-z0-9_\-]+$").expect("should compile regex");
130+
// Validate [creative_opportunities.slot] entries from the *merged* config
131+
// (base trusted-server.toml plus any TRUSTED_SERVER__CREATIVE_OPPORTUNITIES__SLOT
132+
// env overrides) before it is serialized and embedded. This mirrors the
133+
// runtime validator (CreativeOpportunitySlot::validate_runtime) — the build
134+
// context uses a stub whose validate_runtime is a no-op, so without this an
135+
// invalid slot would pass CI and surface as a request-time configuration
136+
// error / service outage. The validator is shared with the crate (see
137+
// `creative_slot_build_check`) so it stays under test. Running it before the
138+
// write also means a rejected config is never persisted to the embedded file.
139139
if let Some(co) = &settings.creative_opportunities {
140140
for slot in &co.slot_raw {
141-
if let Some(id) = slot.get("id").and_then(|v| v.as_str()) {
142-
if !slot_id_re.is_match(id) {
143-
panic!(
144-
"trusted-server.toml [creative_opportunities.slot]: slot id '{}' is invalid; \
145-
only [A-Za-z0-9_-] allowed",
146-
id
147-
);
148-
}
149-
} else {
150-
panic!(
151-
"trusted-server.toml [creative_opportunities.slot]: a slot entry is missing the required 'id' field"
152-
);
141+
if let Err(err) = validate_creative_slot(slot, &co.gam_network_id) {
142+
panic!("trusted-server.toml [creative_opportunities.slot]: {err}");
153143
}
154144
}
155145
if !co.slot_raw.is_empty() {
@@ -159,4 +149,15 @@ fn main() {
159149
);
160150
}
161151
}
152+
153+
let merged_toml =
154+
toml::to_string_pretty(&settings).expect("Failed to serialize settings to TOML");
155+
156+
// Only write when content changes to avoid unnecessary recompilation.
157+
let dest_path = Path::new(TRUSTED_SERVER_OUTPUT_CONFIG_PATH);
158+
let current = fs::read_to_string(dest_path).unwrap_or_default();
159+
if current != merged_toml {
160+
fs::write(dest_path, merged_toml)
161+
.unwrap_or_else(|_| panic!("Failed to write {dest_path:?}"));
162+
}
162163
}
Lines changed: 201 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,201 @@
1+
//! Build-time validation for creative-opportunity slot definitions.
2+
//!
3+
//! This module is compiled in two contexts:
4+
//! - by `build.rs` (via `#[path]`), which runs it against the raw slot JSON
5+
//! merged from `trusted-server.toml` and `TRUSTED_SERVER__*` env overrides
6+
//! before the config is embedded into the binary;
7+
//! - by the crate's test build (via `#[cfg(test)] mod`), so the rules below are
8+
//! exercised under `cargo test`.
9+
//!
10+
//! It mirrors the runtime validator
11+
//! (`CreativeOpportunitySlot::validate_runtime`) so an invalid slot fails the
12+
//! build instead of surfacing as a request-time configuration error. It reads
13+
//! raw JSON (not the typed runtime struct) because the typed slot vec is
14+
//! intentionally empty in the build context, keeping `build.rs` free of the
15+
//! full runtime dependency graph.
16+
17+
/// Returns `true` when `id` is non-empty and only `[A-Za-z0-9_-]`.
18+
fn is_valid_slot_id(id: &str) -> bool {
19+
!id.is_empty()
20+
&& id
21+
.chars()
22+
.all(|c| c.is_ascii_alphanumeric() || c == '_' || c == '-')
23+
}
24+
25+
/// Validate a single raw creative-opportunity slot.
26+
///
27+
/// Mirrors the runtime checks in `CreativeOpportunitySlot::validate_runtime`:
28+
/// a syntactically safe non-empty id, at least one non-empty page pattern, at
29+
/// least one format with positive dimensions, and a non-empty resolved GAM unit
30+
/// path. Returns an error string describing the first problem found.
31+
///
32+
/// # Errors
33+
///
34+
/// Returns an error string when the slot is missing required fields, has an
35+
/// invalid id, has no usable page pattern or format, has a zero-dimension
36+
/// format, or resolves to an empty GAM unit path.
37+
pub(crate) fn validate_creative_slot(
38+
slot: &serde_json::Value,
39+
gam_network_id: &str,
40+
) -> Result<(), String> {
41+
let id = match slot.get("id").and_then(serde_json::Value::as_str) {
42+
Some(id) => id,
43+
None => return Err("a slot entry is missing the required 'id' field".to_string()),
44+
};
45+
if id.is_empty() {
46+
return Err("slot id must not be empty".to_string());
47+
}
48+
if !is_valid_slot_id(id) {
49+
return Err(format!(
50+
"slot id '{id}' is invalid; only [A-Za-z0-9_-] allowed"
51+
));
52+
}
53+
54+
// At least one non-empty page pattern.
55+
let has_valid_pattern = slot
56+
.get("page_patterns")
57+
.and_then(serde_json::Value::as_array)
58+
.is_some_and(|patterns| {
59+
patterns
60+
.iter()
61+
.any(|p| p.as_str().is_some_and(|s| !s.trim().is_empty()))
62+
});
63+
if !has_valid_pattern {
64+
return Err(format!(
65+
"slot `{id}` must include at least one non-empty page pattern"
66+
));
67+
}
68+
69+
// At least one format, each with positive width and height.
70+
match slot.get("formats").and_then(serde_json::Value::as_array) {
71+
Some(formats) if !formats.is_empty() => {
72+
for format in formats {
73+
let width = format.get("width").and_then(serde_json::Value::as_u64);
74+
let height = format.get("height").and_then(serde_json::Value::as_u64);
75+
if !matches!((width, height), (Some(w), Some(h)) if w > 0 && h > 0) {
76+
return Err(format!(
77+
"slot `{id}` format must have positive width and height"
78+
));
79+
}
80+
}
81+
}
82+
_ => {
83+
return Err(format!("slot `{id}` must include at least one format"));
84+
}
85+
}
86+
87+
// Resolved GAM unit path must not be empty. An explicit override is used
88+
// when present; otherwise it is derived as `/<gam_network_id>/<id>`.
89+
let resolved_gam_unit_path = match slot
90+
.get("gam_unit_path")
91+
.and_then(serde_json::Value::as_str)
92+
{
93+
Some(path) => path.to_string(),
94+
None => format!("/{gam_network_id}/{id}"),
95+
};
96+
if resolved_gam_unit_path.trim().is_empty() {
97+
return Err(format!(
98+
"slot `{id}` resolved GAM unit path must not be empty"
99+
));
100+
}
101+
102+
Ok(())
103+
}
104+
105+
#[cfg(test)]
106+
mod tests {
107+
use super::validate_creative_slot;
108+
use serde_json::json;
109+
110+
#[test]
111+
fn accepts_a_well_formed_slot() {
112+
let slot = json!({
113+
"id": "atf",
114+
"page_patterns": ["/20**"],
115+
"formats": [{ "width": 300, "height": 250 }]
116+
});
117+
assert!(validate_creative_slot(&slot, "123456789").is_ok());
118+
}
119+
120+
#[test]
121+
fn accepts_explicit_gam_unit_path_override() {
122+
let slot = json!({
123+
"id": "atf",
124+
"page_patterns": ["/"],
125+
"formats": [{ "width": 300, "height": 250 }],
126+
"gam_unit_path": "/123456789/publisher/atf"
127+
});
128+
assert!(validate_creative_slot(&slot, "123456789").is_ok());
129+
}
130+
131+
#[test]
132+
fn rejects_empty_formats() {
133+
let slot = json!({
134+
"id": "atf",
135+
"page_patterns": ["/20**"],
136+
"formats": []
137+
});
138+
let err = validate_creative_slot(&slot, "123456789")
139+
.expect_err("empty formats must fail at build time");
140+
assert!(err.contains("at least one format"), "got: {err}");
141+
}
142+
143+
#[test]
144+
fn rejects_zero_dimension_format() {
145+
let slot = json!({
146+
"id": "atf",
147+
"page_patterns": ["/20**"],
148+
"formats": [{ "width": 0, "height": 250 }]
149+
});
150+
let err = validate_creative_slot(&slot, "123456789")
151+
.expect_err("zero dimensions must fail at build time");
152+
assert!(err.contains("positive width and height"), "got: {err}");
153+
}
154+
155+
#[test]
156+
fn rejects_empty_page_patterns() {
157+
let slot = json!({
158+
"id": "atf",
159+
"page_patterns": [],
160+
"formats": [{ "width": 300, "height": 250 }]
161+
});
162+
let err = validate_creative_slot(&slot, "123456789")
163+
.expect_err("empty page patterns must fail at build time");
164+
assert!(err.contains("page pattern"), "got: {err}");
165+
}
166+
167+
#[test]
168+
fn rejects_blank_page_pattern_strings() {
169+
let slot = json!({
170+
"id": "atf",
171+
"page_patterns": [" "],
172+
"formats": [{ "width": 300, "height": 250 }]
173+
});
174+
assert!(validate_creative_slot(&slot, "123456789").is_err());
175+
}
176+
177+
#[test]
178+
fn rejects_blank_gam_unit_path_override() {
179+
let slot = json!({
180+
"id": "atf",
181+
"page_patterns": ["/20**"],
182+
"formats": [{ "width": 300, "height": 250 }],
183+
"gam_unit_path": " "
184+
});
185+
let err = validate_creative_slot(&slot, "123456789")
186+
.expect_err("blank GAM unit path must fail at build time");
187+
assert!(err.contains("GAM unit path"), "got: {err}");
188+
}
189+
190+
#[test]
191+
fn rejects_missing_id() {
192+
let slot = json!({ "page_patterns": ["/"], "formats": [{ "width": 1, "height": 1 }] });
193+
assert!(validate_creative_slot(&slot, "net").is_err());
194+
}
195+
196+
#[test]
197+
fn rejects_invalid_id_characters() {
198+
let slot = json!({ "id": "a b", "page_patterns": ["/"], "formats": [{ "width": 1, "height": 1 }] });
199+
assert!(validate_creative_slot(&slot, "net").is_err());
200+
}
201+
}

crates/trusted-server-core/src/lib.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,10 @@ pub mod constants;
4242
pub mod cookies;
4343
pub mod creative;
4444
pub mod creative_opportunities;
45+
// Build-time slot validation, shared with `build.rs` via `#[path]`. Compiled
46+
// here only under test so its rules stay exercised by `cargo test`.
47+
#[cfg(test)]
48+
mod creative_slot_build_check;
4549
pub mod ec;
4650
pub(crate) mod edge_cookie;
4751
pub mod error;

0 commit comments

Comments
 (0)