Skip to content
Open
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 @@ -292,7 +292,15 @@ pub fn validate_structure(
};
for fs in &all_fs {
for name in fs.expression_names() {
if let Some(file_name) = name.strip_prefix("Task.File.") {
if let Some(tail) = name.strip_prefix("Task.File.") {
// Only the first dotted segment is the embedded-file
// key. `Task.File.<name>` is `path` typed, so any
// further segments are property access on that value —
// RFC 0005 uses `{{Task.File.Run.name}}` directly.
// Embedded file names are identifiers (no dots, see
// the name validation below), so the first segment is
// unambiguous.
let file_name = tail.split_once('.').map_or(tail, |(key, _)| key);
if !file_names.contains(file_name) {
errors.add(
&script_path,
Expand Down
151 changes: 151 additions & 0 deletions crates/openjd-model/tests/integration/test_embedded.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@
//! test_environment_template.rs and test_actions_and_steps.rs.
//! Uses job templates for limit enforcement (filename length) since
//! the Rust crate enforces limits in the job template validation path.
//!
//! Also covers validation of `Task.File.<name>` *references* from a step
//! script's action fields back to the embedded files it declares.

use openjd_model::CallerLimits;
use openjd_model::{decode_environment_template, decode_job_template};
Expand Down Expand Up @@ -188,3 +191,151 @@ fn filename_too_long_job() {
r#"{{"name": "Foo", "type": "TEXT", "data": "hello", "filename": "{name}"}}"#
));
}
// ══════════════════════════════════════════════════════════════
// Task.File.<name> reference validation
//
// RFC 0005 declares `Task.File.<name>` as `path` typed and shows property
// access on one directly inside a format string:
//
// echo "Script: {{Task.File.Run.name}}"
//
// The embedded-file key is only the FIRST dotted segment after `Task.File.`;
// any further segments are property access on the resulting path value. A
// validator that looks the whole dotted tail up as an embedded-file key
// rejects the form the RFC itself uses.
// ══════════════════════════════════════════════════════════════

/// Job template declaring one embedded file named `file_name`, with `command`
/// and the single onRun arg supplied separately, so a test can vary the
/// `Task.File.*` reference — and its position — independently of the embedded
/// file that is actually declared. Both fields are scanned by the validator.
fn job_with_command_and_arg(file_name: &str, command: &str, arg: &str) -> serde_json::Value {
yaml_val(&format!(
r#"{{
"specificationVersion": "jobtemplate-2023-09",
"extensions": ["EXPR"],
"name": "Test",
"steps": [{{
"name": "S",
"script": {{
"embeddedFiles": [{{"name": "{file_name}", "type": "TEXT", "filename": "run.txt", "data": "contents"}}],
"actions": {{"onRun": {{"command": "{command}", "args": ["{arg}"]}}}}
}}
}}]
}}"#
))
}

fn decode_template(v: serde_json::Value) -> Result<(), String> {
decode_job_template(v, Some(&["EXPR"]), &CallerLimits::default())
.map(|_| ())
.map_err(|e| e.to_string())
}

/// Reference placed in the onRun `args`.
fn decode_with_arg(file_name: &str, arg: &str) -> Result<(), String> {
decode_template(job_with_command_and_arg(file_name, "foo", arg))
}

/// Reference placed in the onRun `command`.
fn decode_with_command(file_name: &str, command: &str) -> Result<(), String> {
decode_template(job_with_command_and_arg(file_name, command, "arg"))
}

#[test]
fn task_file_direct_reference_accepted() {
// Baseline: the bare reference with no property access.
decode_with_arg("Run", "{{Task.File.Run}}").expect("bare Task.File.<name> must be accepted");
}

#[test]
fn task_file_direct_property_access_accepted() {
// The exact form RFC 0005 uses. `.name` is a property of the path value,
// not part of the embedded-file key.
decode_with_arg("Run", "{{Task.File.Run.name}}")
.expect("direct .name property access on Task.File.<name> must be accepted");
}

#[test]
fn task_file_direct_property_access_suffix_accepted() {
decode_with_arg("Run", "{{Task.File.Run.suffix}}")
.expect("direct .suffix property access on Task.File.<name> must be accepted");
}

#[test]
fn task_file_direct_property_access_parent_accepted() {
decode_with_arg("Run", "{{Task.File.Run.parent}}")
.expect("direct .parent property access on Task.File.<name> must be accepted");
}

#[test]
fn task_file_nested_property_chain_accepted() {
// Only the FIRST segment is the file key — not "all but the last". A
// chained property access must not be mistaken for a longer file key.
decode_with_arg("Run", "{{Task.File.Run.parent.name}}")
.expect("chained property access on Task.File.<name> must be accepted");
}

#[test]
fn task_file_property_access_in_command_accepted() {
// The validator scans onRun.command as well as args.
decode_with_command("Run", "{{Task.File.Run.name}}")
.expect("property access in onRun.command must be accepted");
}

// ── Negative controls: an undefined embedded file must still be rejected ──
//
// Note: these templates also fail the format-string symbol resolution, which
// independently reports `Undefined variable`. So the load-bearing part of each
// assertion below is the *message content*, not merely that decoding failed.

#[test]
fn task_file_unknown_name_rejected() {
let msg = decode_with_arg("Run", "{{Task.File.Missing}}").expect_err("expected decode failure");
assert!(
msg.contains("steps[0] -> script:\n\treferences undefined embedded file 'Missing'."),
"Expected undefined-embedded-file error at steps[0] -> script naming 'Missing', got: {msg}"
);
}

#[test]
fn task_file_unknown_name_with_property_rejected() {
// Still an undefined embedded file, and the error must name the file key
// ('Missing') rather than the property-access tail ('Missing.name').
let msg =
decode_with_arg("Run", "{{Task.File.Missing.name}}").expect_err("expected decode failure");
assert!(
msg.contains("steps[0] -> script:\n\treferences undefined embedded file 'Missing'."),
"Expected error to name the embedded-file key 'Missing', got: {msg}"
);
// The property tail must not be folded into the embedded-file key. Only
// this specific message is constrained: the format-string validator also
// reports the whole dotted path as an undefined variable, which is correct.
assert!(
!msg.contains("undefined embedded file 'Missing.name'"),
"Property tail must not be treated as part of the file key, got: {msg}"
);
}

#[test]
fn task_file_unknown_name_in_command_rejected() {
let msg = decode_with_command("Run", "{{Task.File.Missing.name}}")
.expect_err("expected decode failure");
assert!(
msg.contains("steps[0] -> script:\n\treferences undefined embedded file 'Missing'."),
"Expected undefined-embedded-file error for a reference in command, got: {msg}"
);
}

#[test]
fn embedded_file_name_with_dot_rejected() {
// Load-bearing invariant for the first-segment split above: an embedded
// file name is an identifier, so it can never itself contain a dot. If
// this ever changed, `Task.File.a.b` would become ambiguous.
let msg = decode_with_arg("My.File", "{{Task.File.Run}}")
.expect_err("dotted embedded file name must be rejected");
assert!(
msg.contains("'My.File' is not a valid identifier"),
"Expected invalid-identifier error for a dotted embedded file name, got: {msg}"
);
}
Loading