Skip to content

Commit 09be4a6

Browse files
authored
Ignore runner metadata in snapshot identity (#8193)
* fix: remap Lab cook attempt plan paths * Ignore runner metadata in snapshot identity
1 parent ca4fc16 commit 09be4a6

3 files changed

Lines changed: 279 additions & 6 deletions

File tree

src/core/runner/workspace/snapshot.rs

Lines changed: 35 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@ use super::util::{
1515
tar_exclude_args,
1616
};
1717

18+
const RUNNER_WORKSPACE_METADATA_FILE: &str = ".homeboy/runner-workspace.json";
19+
1820
pub(crate) fn snapshot_identity(
1921
local_path: &Path,
2022
excludes: &[String],
@@ -90,9 +92,10 @@ fn collect_content_hash_entries(
9092
let entry_path = entry.path();
9193
let relative_path = logical.join(entry.file_name());
9294
let relative = relative_path.to_string_lossy().replace('\\', "/");
95+
let is_runner_metadata_directory = relative == ".homeboy";
9396
if relative == ".git"
94-
|| relative == ".homeboy/runner-workspace.json"
9597
|| is_excluded(root, &root.join(&relative_path), excludes, &[])
98+
|| relative == RUNNER_WORKSPACE_METADATA_FILE
9699
{
97100
continue;
98101
}
@@ -126,7 +129,12 @@ fn collect_content_hash_entries(
126129
None,
127130
));
128131
}
129-
entries.push((relative, "\0dir\0", mode_bits(&metadata), Vec::new()));
132+
// The runner adds `.homeboy/runner-workspace.json` after transport.
133+
// Recurse through the directory so user-owned children remain bound,
134+
// but omit the transport-owned container entry and record itself.
135+
if !is_runner_metadata_directory {
136+
entries.push((relative, "\0dir\0", mode_bits(&metadata), Vec::new()));
137+
}
130138
ancestors.push(canonical);
131139
collect_content_hash_entries(
132140
root,
@@ -267,7 +275,12 @@ pub(super) fn is_excluded(
267275
return false;
268276
}
269277
excludes.iter().any(|pattern| {
270-
pattern == rel || pattern == name || glob_match(pattern, rel) || glob_match(pattern, name)
278+
let directory_pattern = pattern.trim_end_matches('/');
279+
pattern == rel
280+
|| pattern == name
281+
|| directory_pattern == rel
282+
|| glob_match(pattern, rel)
283+
|| glob_match(pattern, name)
271284
})
272285
}
273286

@@ -463,6 +476,25 @@ pub(crate) fn copy_snapshot_to_directory(
463476
)
464477
}
465478

479+
pub(crate) fn ensure_no_runner_workspace_metadata_collision(local_path: &Path) -> Result<()> {
480+
let metadata_path = local_path.join(RUNNER_WORKSPACE_METADATA_FILE);
481+
match fs::symlink_metadata(&metadata_path) {
482+
Ok(_) => Err(Error::validation_invalid_argument(
483+
"workspace",
484+
"source workspace contains the reserved runner metadata path `.homeboy/runner-workspace.json`; remove or rename it before syncing",
485+
Some(metadata_path.display().to_string()),
486+
Some(vec![
487+
"Remove or rename the source file before syncing; Homeboy writes this path only after runner materialization.".to_string(),
488+
]),
489+
)),
490+
Err(error) if error.kind() == std::io::ErrorKind::NotFound => Ok(()),
491+
Err(error) => Err(Error::internal_io(
492+
error.to_string(),
493+
Some("inspect reserved runner metadata path".to_string()),
494+
)),
495+
}
496+
}
497+
466498
fn materialize_snapshot_piped(
467499
local_path: &Path,
468500
target_command: &str,

src/core/runner/workspace/sync.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,8 @@ use super::super::{
1919
};
2020
use super::git::{git_snapshot, materialize_git, materialize_git_from_controller_bundle};
2121
use super::snapshot::{
22-
effective_snapshot_excludes, local_snapshot_stats, materialize_snapshot,
23-
materialize_snapshot_git, snapshot_identity,
22+
effective_snapshot_excludes, ensure_no_runner_workspace_metadata_collision,
23+
local_snapshot_stats, materialize_snapshot, materialize_snapshot_git, snapshot_identity,
2424
};
2525
use super::types::{
2626
canonical_workspace_path, ByteFileCounts, LocalGitState, RunnerWorkspaceCurrentSummary,
@@ -85,6 +85,7 @@ pub fn sync_workspace(
8585

8686
match options.mode {
8787
RunnerWorkspaceSyncMode::Snapshot | RunnerWorkspaceSyncMode::SnapshotGit => {
88+
ensure_no_runner_workspace_metadata_collision(&local_path)?;
8889
let snapshot = snapshot_identity(&local_path, &excludes, &includes)?;
8990
let remote_path = temp::unique_name(
9091
&deterministic_remote_path(

src/core/runner/workspace/tests/snapshot.rs

Lines changed: 241 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,8 @@ use std::fs;
44
use std::path::Path;
55

66
use crate::core::runner::workspace::snapshot::{
7-
snapshot_archive_command, snapshot_install_command,
7+
copy_snapshot_to_directory, snapshot_archive_command, snapshot_install_command,
8+
workspace_content_hash,
89
};
910
use crate::core::runner::workspace::sync::{list_workspaces, sync_workspace};
1011
use crate::core::runner::workspace::types::{
@@ -108,6 +109,74 @@ fn runner_snapshot_excludes_extend_default_snapshot_policy() {
108109
});
109110
}
110111

112+
#[test]
113+
fn runner_snapshot_rejects_source_runner_workspace_metadata_collision() {
114+
crate::test_support::with_isolated_home(|_| {
115+
let source = tempfile::tempdir().expect("source tempdir");
116+
let runner_root = tempfile::tempdir().expect("runner root tempdir");
117+
fs::create_dir_all(source.path().join(".homeboy")).expect("metadata directory");
118+
fs::write(
119+
source.path().join(".homeboy/runner-workspace.json"),
120+
"user-owned collision\n",
121+
)
122+
.expect("metadata collision");
123+
crate::core::runner::create(
124+
&format!(
125+
r#"{{"id":"lab-local-collision","kind":"local","workspace_root":"{}"}}"#,
126+
runner_root.path().display()
127+
),
128+
false,
129+
)
130+
.expect("create runner");
131+
132+
let error = sync_workspace(
133+
"lab-local-collision",
134+
RunnerWorkspaceSyncOptions {
135+
path: source.path().display().to_string(),
136+
mode: RunnerWorkspaceSyncMode::Snapshot,
137+
controller_routed_git: false,
138+
changed_since_base: None,
139+
git_fetch_refs: Vec::new(),
140+
snapshot_includes: Vec::new(),
141+
allow_dirty_lab_workspace: false,
142+
run_isolation_token: None,
143+
},
144+
)
145+
.expect_err("reserved runner metadata must reject staging");
146+
147+
assert!(error.message.contains("reserved runner metadata path"));
148+
assert!(error.message.contains("remove or rename"));
149+
assert_eq!(
150+
fs::read_dir(runner_root.path())
151+
.expect("runner root entries")
152+
.count(),
153+
0,
154+
"collision must fail before creating a materialized workspace"
155+
);
156+
});
157+
}
158+
159+
#[test]
160+
fn generic_snapshot_copy_allows_source_owned_runner_workspace_path() {
161+
let source = tempfile::tempdir().expect("source tempdir");
162+
let destination = tempfile::tempdir().expect("destination tempdir");
163+
fs::create_dir_all(source.path().join(".homeboy")).expect("metadata directory");
164+
fs::write(
165+
source.path().join(".homeboy/runner-workspace.json"),
166+
"source-owned generic snapshot content\n",
167+
)
168+
.expect("source metadata");
169+
170+
copy_snapshot_to_directory(source.path(), destination.path(), &[])
171+
.expect("generic snapshot copy");
172+
173+
assert_eq!(
174+
fs::read_to_string(destination.path().join(".homeboy/runner-workspace.json"))
175+
.expect("copied metadata"),
176+
"source-owned generic snapshot content\n"
177+
);
178+
}
179+
111180
#[test]
112181
fn test_sync_workspace() {
113182
crate::test_support::with_isolated_home(|_| {
@@ -569,6 +638,177 @@ fn snapshot_archive_command_dereferences_symlinked_dependencies() {
569638
);
570639
}
571640

641+
#[test]
642+
fn snapshot_content_hash_matches_materialized_workspace_after_runner_metadata_injection() {
643+
// This mirrors a Lab snapshot of a repository such as homeboy-extensions:
644+
// the runner creates its metadata directory after extracting a source that
645+
// has no `.homeboy` directory of its own.
646+
let controller = tempfile::tempdir().expect("controller");
647+
let source = controller.path().join("homeboy-extensions@fixture");
648+
let dependency = controller.path().join("dependency");
649+
let destination = controller.path().join("materialized");
650+
let excludes = vec![
651+
".git/".to_string(),
652+
"generated-state".to_string(),
653+
"generated-state/**".to_string(),
654+
];
655+
656+
fs::create_dir_all(source.join("packages/runtime")).expect("source package directory");
657+
fs::create_dir_all(source.join("generated-state")).expect("generated state directory");
658+
fs::write(
659+
source.join("packages/runtime/runner.sh"),
660+
"#!/bin/sh\nexit 0\n",
661+
)
662+
.expect("runner script");
663+
fs::write(source.join("generated-state/cache.bin"), "excluded\n").expect("generated state");
664+
665+
#[cfg(unix)]
666+
{
667+
use std::os::unix::fs::{symlink, PermissionsExt};
668+
669+
let script = source.join("packages/runtime/runner.sh");
670+
fs::set_permissions(&script, fs::Permissions::from_mode(0o755)).expect("executable mode");
671+
fs::create_dir_all(dependency.join("dist")).expect("dependency directory");
672+
fs::write(dependency.join("dist/index.js"), "export default {};\n")
673+
.expect("dependency file");
674+
symlink(&dependency, source.join("packages/runtime/dependency"))
675+
.expect("dependency symlink");
676+
}
677+
678+
let expected = workspace_content_hash(&source, &excludes).expect("controller hash");
679+
copy_snapshot_to_directory(&source, &destination, &excludes).expect("materialize snapshot");
680+
fs::create_dir_all(destination.join(".homeboy")).expect("runner metadata directory");
681+
fs::write(
682+
destination.join(".homeboy/runner-workspace.json"),
683+
r#"{"schema":"homeboy/runner-workspace/v1"}"#,
684+
)
685+
.expect("runner metadata");
686+
687+
assert!(destination.join("packages/runtime/runner.sh").is_file());
688+
assert!(!destination.join("generated-state").exists());
689+
assert_eq!(
690+
workspace_content_hash(&destination, &excludes).expect("materialized hash"),
691+
expected,
692+
"the controller hash must describe the bytes and structure that the runner verifies"
693+
);
694+
}
695+
696+
#[test]
697+
fn snapshot_content_hash_binds_user_owned_homeboy_files_but_ignores_runner_metadata() {
698+
let controller = tempfile::tempdir().expect("controller");
699+
let source = controller.path().join("homeboy-extensions@fixture");
700+
let destination = controller.path().join("materialized");
701+
let excludes = vec![".git/".to_string()];
702+
fs::create_dir_all(source.join(".homeboy")).expect("user metadata directory");
703+
fs::create_dir_all(source.join("src")).expect("source directory");
704+
fs::write(
705+
source.join(".homeboy/user-settings.json"),
706+
"{\"enabled\":true}\n",
707+
)
708+
.expect("user metadata");
709+
fs::write(source.join("src/lib.rs"), "pub fn fixture() {}\n").expect("source file");
710+
711+
let expected = workspace_content_hash(&source, &excludes).expect("controller hash");
712+
copy_snapshot_to_directory(&source, &destination, &excludes).expect("materialize snapshot");
713+
fs::write(
714+
destination.join(".homeboy/runner-workspace.json"),
715+
r#"{"schema":"homeboy/runner-workspace/v1"}"#,
716+
)
717+
.expect("runner metadata");
718+
719+
assert_eq!(
720+
workspace_content_hash(&destination, &excludes).expect("materialized hash"),
721+
expected,
722+
"runner metadata must not change the controller identity"
723+
);
724+
725+
fs::write(
726+
destination.join(".homeboy/runner-workspace.json"),
727+
r#"{"schema":"homeboy/runner-workspace/v2","changed":true}"#,
728+
)
729+
.expect("changed runner metadata");
730+
assert_eq!(
731+
workspace_content_hash(&destination, &excludes).expect("metadata-insensitive hash"),
732+
expected,
733+
"runner metadata bytes and mode are transport state"
734+
);
735+
736+
#[cfg(unix)]
737+
{
738+
use std::os::unix::fs::PermissionsExt;
739+
740+
fs::set_permissions(
741+
destination.join(".homeboy/runner-workspace.json"),
742+
fs::Permissions::from_mode(0o600),
743+
)
744+
.expect("changed runner metadata mode");
745+
assert_eq!(
746+
workspace_content_hash(&destination, &excludes).expect("mode-insensitive hash"),
747+
expected,
748+
"runner metadata mode is transport state"
749+
);
750+
}
751+
752+
fs::write(
753+
destination.join(".homeboy/user-settings.json"),
754+
"{\"enabled\":false}\n",
755+
)
756+
.expect("changed user metadata");
757+
assert_ne!(
758+
workspace_content_hash(&destination, &excludes).expect("mutated hash"),
759+
expected,
760+
"user-owned `.homeboy` children must remain fail-closed"
761+
);
762+
763+
fs::write(
764+
destination.join(".homeboy/user-settings.json"),
765+
"{\"enabled\":true}\n",
766+
)
767+
.expect("restore user metadata");
768+
assert_eq!(
769+
workspace_content_hash(&destination, &excludes).expect("restored hash"),
770+
expected,
771+
"restoring user metadata restores the materialized identity"
772+
);
773+
fs::write(destination.join("src/lib.rs"), "pub fn changed() {}\n")
774+
.expect("changed source file");
775+
assert_ne!(
776+
workspace_content_hash(&destination, &excludes).expect("changed source hash"),
777+
expected,
778+
"ordinary workspace files must remain bound by the identity"
779+
);
780+
}
781+
782+
#[test]
783+
fn snapshot_content_hash_matches_tar_when_homeboy_is_excluded() {
784+
for pattern in [".homeboy", ".homeboy/", ".homeboy/**"] {
785+
let controller = tempfile::tempdir().expect("controller");
786+
let source = controller.path().join("source");
787+
let destination = controller.path().join("materialized");
788+
let excludes = vec![pattern.to_string()];
789+
fs::create_dir_all(source.join(".homeboy")).expect("user metadata directory");
790+
fs::create_dir_all(source.join("src")).expect("source directory");
791+
fs::write(source.join(".homeboy/user-settings.json"), "user-owned\n")
792+
.expect("user metadata");
793+
fs::write(source.join("src/lib.rs"), "pub fn fixture() {}\n").expect("source file");
794+
795+
let expected = workspace_content_hash(&source, &excludes).expect("controller hash");
796+
copy_snapshot_to_directory(&source, &destination, &excludes).expect("materialize snapshot");
797+
fs::create_dir_all(destination.join(".homeboy")).expect("runner metadata directory");
798+
fs::write(
799+
destination.join(".homeboy/runner-workspace.json"),
800+
"transport metadata\n",
801+
)
802+
.expect("runner metadata");
803+
804+
assert_eq!(
805+
workspace_content_hash(&destination, &excludes).expect("materialized hash"),
806+
expected,
807+
"exclude pattern `{pattern}` must hash the same tree tar materializes"
808+
);
809+
}
810+
}
811+
572812
#[test]
573813
#[cfg(unix)]
574814
fn copy_snapshot_materializes_symlinked_dependency_contents() {

0 commit comments

Comments
 (0)