Skip to content

Commit 576df0b

Browse files
committed
fix(evidence): validate complete rendered paths before drafting
Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
1 parent df04e3b commit 576df0b

5 files changed

Lines changed: 267 additions & 23 deletions

File tree

‎crates/registry-evidencectl/src/suggest/emit.rs‎

Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,10 @@ const ADMITTED_METHODS: [&str; 2] = ["GET", "POST"];
147147
/// source, from already-decided inputs.
148148
pub fn draft(inputs: &EmitInputs) -> Result<DraftArtifacts> {
149149
let method = request_method(&inputs.operation.method)?;
150+
if !inputs.request_mappings.is_empty() {
151+
validate_mapped_request_path(inputs)
152+
.context("generated request path is outside the Evidence fixed-request contract")?;
153+
}
150154

151155
let mut get_paths = Vec::with_capacity(inputs.selection.len());
152156
for pointer in &inputs.selection {
@@ -321,6 +325,108 @@ fn request_path(inputs: &EmitInputs) -> String {
321325
format!("{prefix}{}", inputs.operation.path)
322326
}
323327

328+
/// Mirror the pure FixedRequest path rules without linking the runtime into
329+
/// authoring. The runtime-admission parity test covers the complete rendered
330+
/// path, including its server prefix, bounds, escapes and binding names.
331+
fn validate_mapped_request_path(inputs: &EmitInputs) -> Result<()> {
332+
let path = request_path(inputs);
333+
if !(2..=2048).contains(&path.len()) || !path.starts_with('/') || !path.is_ascii() {
334+
bail!("request paths must be absolute ASCII paths between 2 and 2048 bytes");
335+
}
336+
let bindings = inputs
337+
.request_mappings
338+
.iter()
339+
.filter_map(|mapping| {
340+
if let RequestTarget::Path(name) = &mapping.target {
341+
Some(name.as_str())
342+
} else {
343+
None
344+
}
345+
})
346+
.collect::<BTreeSet<_>>();
347+
let mut placeholders = BTreeSet::new();
348+
let mut normalized = String::with_capacity(path.len());
349+
for segment in path.split('/').skip(1) {
350+
if segment.is_empty() || matches!(segment, "." | "..") {
351+
bail!("request paths must not contain empty or dot segments");
352+
}
353+
normalized.push('/');
354+
if segment.contains(['{', '}']) {
355+
let name = segment
356+
.strip_prefix('{')
357+
.and_then(|value| value.strip_suffix('}'))
358+
.ok_or_else(|| {
359+
anyhow::anyhow!("a path placeholder must occupy one complete segment")
360+
})?;
361+
LocalId::new(name).with_context(|| {
362+
format!(
363+
"path parameter `{name}` must be a Registry Stack LocalId for a path binding"
364+
)
365+
})?;
366+
if !placeholders.insert(name) {
367+
bail!("path placeholders must be unique: `{name}` is repeated");
368+
}
369+
if !bindings.contains(name) {
370+
bail!("required request input `path:{name}` has no --map-selector assignment");
371+
}
372+
normalized.push('x');
373+
} else {
374+
normalized.push_str(segment);
375+
}
376+
}
377+
if placeholders != bindings {
378+
bail!("path bindings must exactly match the generated path placeholders");
379+
}
380+
let bytes = normalized.as_bytes();
381+
let mut index = 0;
382+
while index < bytes.len() {
383+
let byte = bytes[index];
384+
if byte == b'%' {
385+
if index + 2 >= bytes.len()
386+
|| !bytes[index + 1].is_ascii_hexdigit()
387+
|| !bytes[index + 2].is_ascii_hexdigit()
388+
|| bytes[index + 1].is_ascii_lowercase()
389+
|| bytes[index + 2].is_ascii_lowercase()
390+
{
391+
bail!("request path contains a non-canonical escape");
392+
}
393+
let decoded = u8::from_str_radix(&normalized[index + 1..index + 3], 16)?;
394+
if decoded.is_ascii_alphanumeric()
395+
|| matches!(decoded, b'-' | b'.' | b'_' | b'~' | b'/' | b'\\')
396+
{
397+
bail!("request path contains an ambiguous escape");
398+
}
399+
index += 3;
400+
} else if byte.is_ascii_alphanumeric()
401+
|| matches!(
402+
byte,
403+
b'/' | b'.'
404+
| b'_'
405+
| b'~'
406+
| b'!'
407+
| b'$'
408+
| b'&'
409+
| b'\''
410+
| b'('
411+
| b')'
412+
| b'*'
413+
| b'+'
414+
| b','
415+
| b';'
416+
| b'='
417+
| b':'
418+
| b'@'
419+
| b'-'
420+
)
421+
{
422+
index += 1;
423+
} else {
424+
bail!("request path contains a prohibited character");
425+
}
426+
}
427+
Ok(())
428+
}
429+
324430
/// Write every named Function asset under `<project>/`, creating parent
325431
/// directories as needed. Refuses to overwrite any existing file: if any
326432
/// target already exists, nothing is written and the error lists every

‎crates/registry-evidencectl/src/suggest/mod.rs‎

Lines changed: 0 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1026,28 +1026,6 @@ fn validate_required_inputs(
10261026
body: Option<&types::OperationRequestBody>,
10271027
mappings: &[RequestMapping],
10281028
) -> Result<()> {
1029-
let mut placeholders = BTreeSet::new();
1030-
for segment in operation.path.split('/') {
1031-
if !segment.contains(['{', '}']) {
1032-
continue;
1033-
}
1034-
let name = segment
1035-
.strip_prefix('{')
1036-
.and_then(|segment| segment.strip_suffix('}'))
1037-
.ok_or_else(|| anyhow!("a path placeholder must occupy one complete segment"))?;
1038-
registry_platform_yaml::LocalId::new(name).with_context(|| {
1039-
format!("path parameter `{name}` must be a Registry Stack LocalId for a path binding")
1040-
})?;
1041-
if !placeholders.insert(name) {
1042-
bail!("path placeholders must be unique: `{name}` is repeated");
1043-
}
1044-
if !mappings
1045-
.iter()
1046-
.any(|mapping| mapping.target == RequestTarget::Path(name.to_owned()))
1047-
{
1048-
bail!("required request input `path:{name}` has no --map-selector assignment");
1049-
}
1050-
}
10511029
for parameter in parameters.iter().filter(|parameter| parameter.required) {
10521030
let target = match parameter.location {
10531031
ParameterLocation::Path => RequestTarget::Path(parameter.name.clone()),

‎crates/registry-evidencectl/tests/suggest_e2e.rs‎

Lines changed: 70 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -259,7 +259,11 @@ fn unsupported_path_placeholders_are_refused_without_mutation() {
259259
&[format!(
260260
"subject.subject-reference-v1.reference=path:{name}"
261261
)],
262-
"must be a Registry Stack LocalId",
262+
if name.is_ascii() {
263+
"must be a Registry Stack LocalId"
264+
} else {
265+
"absolute ASCII paths"
266+
},
263267
&format!("unsupported path placeholder {name}"),
264268
);
265269
}
@@ -290,6 +294,71 @@ fn unsupported_path_placeholders_are_refused_without_mutation() {
290294
}
291295
}
292296

297+
#[test]
298+
fn invalid_rendered_request_paths_are_refused_without_mutation() {
299+
for path in [
300+
"/records/{record_id}/".to_owned(),
301+
"//records/{record_id}".to_owned(),
302+
"/records//{record_id}".to_owned(),
303+
"/records/./{record_id}".to_owned(),
304+
"/records/../{record_id}".to_owned(),
305+
"/récords/{record_id}".to_owned(),
306+
"/records?fixed=1/{record_id}".to_owned(),
307+
"/records#fragment/{record_id}".to_owned(),
308+
"/records\\other/{record_id}".to_owned(),
309+
"/records/%2e/{record_id}".to_owned(),
310+
"/records/%2F/{record_id}".to_owned(),
311+
"/records/%41/{record_id}".to_owned(),
312+
"/records/%GG/{record_id}".to_owned(),
313+
format!("/{}/{{record_id}}", "a".repeat(2048)),
314+
] {
315+
let document = request_openapi(
316+
"GET",
317+
&path,
318+
vec![parameter("record_id", "path", true, "string")],
319+
None,
320+
);
321+
let workspace = tempfile::tempdir().expect("tempdir");
322+
let project = scaffold(workspace.path(), &document);
323+
assert_mapping_failure_is_atomic(
324+
&project,
325+
&["subject.subject-reference-v1.reference=path:record_id".to_owned()],
326+
"generated request path",
327+
&path,
328+
);
329+
}
330+
}
331+
332+
#[test]
333+
fn invalid_rendered_server_prefixes_are_refused_without_mutation() {
334+
for prefix in [
335+
"/api//v1".to_owned(),
336+
"/api/./v1".to_owned(),
337+
"/api/../v1".to_owned(),
338+
"/api/%2F".to_owned(),
339+
"/api/%2e".to_owned(),
340+
format!("/{}", "a".repeat(2040)),
341+
] {
342+
let mut document: Value = serde_norway::from_str(MAPPED_GET_OPENAPI).expect("OpenAPI");
343+
document["servers"] =
344+
serde_json::json!([{"url":format!("https://records.example.invalid{prefix}")}]);
345+
let workspace = tempfile::tempdir().expect("tempdir");
346+
let project = scaffold(
347+
workspace.path(),
348+
&serde_norway::to_string(&document).expect("OpenAPI YAML"),
349+
);
350+
assert_mapping_failure_is_atomic(
351+
&project,
352+
&[
353+
"subject.subject-reference-v1.reference=path:record_id".to_owned(),
354+
"subject.subject-reference-v1.reference=query:reference".to_owned(),
355+
],
356+
"generated request path",
357+
&prefix,
358+
);
359+
}
360+
}
361+
293362
#[test]
294363
fn invalid_selector_mappings_refuse_before_mutating_the_project() {
295364
struct Case {

‎crates/registry-evidencectl/tests/suggest_emit.rs‎

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -190,6 +190,94 @@ fn explicit_path_and_query_mappings_generate_an_ordinary_source_request() {
190190
assert!(!prepare.contains("TODO(evidencectl)"), "{prepare}");
191191
}
192192

193+
#[test]
194+
fn mapped_rendered_path_guard_agrees_with_runtime_admission() {
195+
use registry_evidence::config::EvidenceConfig;
196+
let maximum = format!("/{}", "a".repeat(2047));
197+
let overlong = format!("/{}", "a".repeat(2048));
198+
let cases = [
199+
("/v1/records", ""),
200+
("/v1/records/{record_id}", "/api"),
201+
("/v1/exact:record/{record_id}", ""),
202+
("/v1/value%20with%20space/{record_id}", ""),
203+
(maximum.as_str(), ""),
204+
(overlong.as_str(), ""),
205+
("/v1/records/{record_id}/", ""),
206+
("/v1//records/{record_id}", ""),
207+
("/v1/./records/{record_id}", ""),
208+
("/v1/../records/{record_id}", ""),
209+
("/v1/récords/{record_id}", ""),
210+
("/v1/records/{record_id}?fixed=1", ""),
211+
("/v1/records/{record_id}#fragment", ""),
212+
("/v1/records\\other/{record_id}", ""),
213+
("/v1/%2e/{record_id}", ""),
214+
("/v1/%2F/{record_id}", ""),
215+
("/v1/%41/{record_id}", ""),
216+
("/v1/%GG/{record_id}", ""),
217+
("/v1/records/{record_id}", "/api//v2"),
218+
("/v1/records/{record_id}", "/api/%2F"),
219+
];
220+
for (case, (path, prefix)) in cases.into_iter().enumerate() {
221+
let mut inputs = base_inputs();
222+
inputs.operation.path = path.to_owned();
223+
inputs.base_url_suggestion =
224+
emit::split_server_url(&format!("https://api.example.invalid{prefix}"));
225+
let selector = SelectorReference {
226+
role: "subject".to_owned(),
227+
profile: "person-demographics-v1".to_owned(),
228+
field: "given_name".to_owned(),
229+
scalar_type: SelectorScalarType::String,
230+
maximum_bytes: 200,
231+
};
232+
inputs.request_mappings = vec![RequestMapping {
233+
selector: selector.clone(),
234+
target: RequestTarget::Query("reference".to_owned()),
235+
}];
236+
let mut bindings = serde_json::Map::new();
237+
if path.contains("{record_id}") {
238+
inputs.request_mappings.push(RequestMapping {
239+
selector,
240+
target: RequestTarget::Path("record_id".to_owned()),
241+
});
242+
bindings.insert("record_id".to_owned(), json!({"type":"selector", "role":"subject", "profile":"person-demographics-v1", "field":"given_name"}));
243+
}
244+
let rendered = format!("{prefix}{path}");
245+
let mut bundle: serde_json::Value = serde_norway::from_str(include_str!(
246+
"../../../products/evidence/fixtures/acceptance/adult-status/evidence.yaml"
247+
))
248+
.expect("runtime acceptance bundle");
249+
let request = &mut bundle["sources"]["source-a"]["request"];
250+
request["path"] = json!(rendered);
251+
if !bindings.is_empty() {
252+
request["pathBindings"] = json!(bindings);
253+
}
254+
let runtime =
255+
EvidenceConfig::parse_yaml(&serde_json::to_vec(&bundle).expect("runtime bundle"));
256+
let draft = emit::draft(&inputs);
257+
assert_eq!(
258+
draft.is_ok(),
259+
runtime.is_ok(),
260+
"path case {case} ({} bytes): draft {:?}, runtime {:?}",
261+
rendered.len(),
262+
draft.as_ref().err(),
263+
runtime.as_ref().err()
264+
);
265+
if let Ok(draft) = draft {
266+
let source: serde_json::Value =
267+
serde_json::from_str(&draft.authoring_source).expect("Source JSON");
268+
assert_eq!(source["request"]["path"], rendered);
269+
}
270+
}
271+
}
272+
273+
#[test]
274+
fn unmapped_incomplete_path_drafts_remain_available() {
275+
let mut inputs = base_inputs();
276+
inputs.operation.path = "/v1/records/{RecordId}/".to_owned();
277+
let draft = emit::draft(&inputs).expect("unmapped authoring draft");
278+
assert!(file_contents(&draft, "functions/search-a-prepare.rhai").contains("TODO(evidencectl)"));
279+
}
280+
193281
#[test]
194282
fn explicit_post_body_mappings_generate_a_closed_json_object() {
195283
let mut inputs = base_inputs();

‎products/evidence/reference/authoring-projects/CONFIG.md‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -169,6 +169,9 @@ inputs are inspected only when you map them. Parameter and request-body
169169
references must resolve locally so their requiredness is known.
170170
Every path placeholder must occupy a unique complete segment, have a mapping,
171171
and use a Registry Stack local identifier (`[a-z][a-z0-9_-]{0,63}`).
172+
The complete request path, including the server path prefix, must fit the
173+
runtime's 2–2048-byte absolute ASCII path contract, with no empty or dot segments,
174+
query, fragment, backslash, or noncanonical or ambiguous percent escapes.
172175

173176
Scalar types must agree, and the selector's admitted domain must fit the
174177
supported string-length or integer-range constraints. Integer formats `int32`

0 commit comments

Comments
 (0)