Skip to content

Commit 2fa21f1

Browse files
committed
fix(stream): close remaining ingress races
1 parent 9be176f commit 2fa21f1

3 files changed

Lines changed: 129 additions & 7 deletions

File tree

src/agent_author.rs

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -229,7 +229,7 @@ fn author_stream(
229229
format!("acquire catalog-authoring lock: {error:#}"),
230230
)
231231
})?;
232-
let found = crate::discover(catalog_root);
232+
let found = crate::discover_strict(catalog_root);
233233
if let Some(error) = found.errors.first() {
234234
return Err(AuthorError::new(
235235
"catalog-malformed",
@@ -1949,4 +1949,34 @@ mod tests {
19491949
"invalid-stream"
19501950
);
19511951
}
1952+
1953+
#[test]
1954+
fn stream_authoring_refuses_catalogs_with_concealed_declarations() {
1955+
use std::os::unix::fs::symlink;
1956+
1957+
let temporary = tempfile::tempdir().unwrap();
1958+
let root = temporary.path().join("catalog");
1959+
let concealed = temporary.path().join("concealed");
1960+
let declaration_path = write(
1961+
&root,
1962+
"h/worker/agent.kdl",
1963+
&declaration("worker", "h", None, "catalog"),
1964+
);
1965+
write(
1966+
&concealed,
1967+
"agent.kdl",
1968+
&declaration("shadow", "h", None, "catalog"),
1969+
);
1970+
symlink(&concealed, root.join("concealed-link")).unwrap();
1971+
let original = fs::read(&declaration_path).unwrap();
1972+
1973+
let error = add_stream(&root, "h.worker", "h", None, "events", None).unwrap_err();
1974+
1975+
assert_eq!(error.code(), "catalog-malformed");
1976+
assert!(
1977+
error.to_string().contains("unobservable declaration entry"),
1978+
"{error}"
1979+
);
1980+
assert_eq!(fs::read(declaration_path).unwrap(), original);
1981+
}
19521982
}

src/event.rs

Lines changed: 58 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@
44
//! own bounded, agent-local receipt ring rather than writing the agent's immutable Sent ledger.
55
66
use std::fs::{self, File, OpenOptions};
7+
use std::io::Write as _;
8+
use std::os::fd::{AsRawFd as _, FromRawFd as _};
79
use std::path::Path;
810
use std::sync::atomic::{AtomicU64, Ordering};
911

@@ -263,7 +265,10 @@ pub fn emit(
263265
record
264266
.recent
265267
.iter()
266-
.find(|entry| entry.key.as_deref() == key && entry.filename != filename)
268+
.find(|entry| {
269+
entry.filename != filename
270+
&& key.is_none_or(|key| entry.key.as_deref() == Some(key))
271+
})
267272
.map(|entry| entry.filename.clone())
268273
} else {
269274
None
@@ -353,15 +358,63 @@ fn read_record(path: &Path) -> anyhow::Result<Option<StreamRecord>> {
353358
}
354359

355360
fn write_record(path: &Path, record: &StreamRecord) -> anyhow::Result<()> {
361+
use std::ffi::CString;
362+
356363
let parent = path.parent().context("stream state has no parent")?;
357364
fs::create_dir_all(parent)?;
358-
let temporary = parent.join(format!(
365+
let directory = File::open(parent)
366+
.with_context(|| format!("open stream state directory {}", parent.display()))?;
367+
let temporary = format!(
359368
".state.tmp-{}-{}",
360369
std::process::id(),
361370
TMP_COUNTER.fetch_add(1, Ordering::Relaxed)
362-
));
363-
fs::write(&temporary, serde_json::to_vec(record)?)?;
364-
fs::rename(&temporary, path)?;
371+
);
372+
let temporary = CString::new(temporary)?;
373+
let target = CString::new(
374+
path.file_name()
375+
.context("stream state has no filename")?
376+
.as_encoded_bytes(),
377+
)?;
378+
let fd = unsafe {
379+
libc::openat(
380+
directory.as_raw_fd(),
381+
temporary.as_ptr(),
382+
libc::O_WRONLY | libc::O_CREAT | libc::O_EXCL | libc::O_NOFOLLOW | libc::O_CLOEXEC,
383+
0o600,
384+
)
385+
};
386+
if fd < 0 {
387+
return Err(std::io::Error::last_os_error()).with_context(|| {
388+
format!(
389+
"create fresh stream state temporary in {}",
390+
parent.display()
391+
)
392+
});
393+
}
394+
let mut file = unsafe { File::from_raw_fd(fd) };
395+
let result = (|| -> anyhow::Result<()> {
396+
file.write_all(&serde_json::to_vec(record)?)?;
397+
file.sync_all()?;
398+
let renamed = unsafe {
399+
libc::renameat(
400+
directory.as_raw_fd(),
401+
temporary.as_ptr(),
402+
directory.as_raw_fd(),
403+
target.as_ptr(),
404+
)
405+
};
406+
if renamed != 0 {
407+
return Err(std::io::Error::last_os_error()).context("publish stream state atomically");
408+
}
409+
directory.sync_all()?;
410+
Ok(())
411+
})();
412+
if result.is_err() {
413+
unsafe {
414+
libc::unlinkat(directory.as_raw_fd(), temporary.as_ptr(), 0);
415+
}
416+
}
417+
result?;
365418
Ok(())
366419
}
367420

tests/event_e2e.rs

Lines changed: 40 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -256,6 +256,45 @@ fn symlinked_stream_state_ancestor_cannot_escape_the_agent_capability() {
256256
assert_eq!(fs::read_dir(outside.path()).unwrap().count(), 0);
257257
}
258258

259+
#[cfg(unix)]
260+
#[test]
261+
fn predictable_stream_state_temporary_symlink_is_never_followed() {
262+
use std::os::unix::fs::symlink;
263+
264+
let catalog = tempfile::tempdir().unwrap();
265+
let outside = tempfile::tempdir().unwrap();
266+
let agent = declare_agent(catalog.path(), "\"running\"", " stream \"gh-ci\" {}\n");
267+
let state_dir = agent.join("resources/streams/gh-ci");
268+
fs::create_dir_all(&state_dir).unwrap();
269+
let victim = outside.path().join("victim");
270+
fs::write(&victim, "must remain unchanged").unwrap();
271+
for counter in 0..4096 {
272+
symlink(
273+
&victim,
274+
state_dir.join(format!(".state.tmp-{}-{counter}", std::process::id())),
275+
)
276+
.unwrap();
277+
}
278+
279+
let error = event::emit(
280+
catalog.path(),
281+
"hetz",
282+
"hetz.worker",
283+
"gh-ci",
284+
"temp-symlink",
285+
None,
286+
None,
287+
"payload",
288+
false,
289+
)
290+
.unwrap_err()
291+
.to_string();
292+
293+
assert!(error.contains("fresh stream state temporary"), "{error}");
294+
assert_eq!(fs::read_to_string(victim).unwrap(), "must remain unchanged");
295+
assert!(!state_dir.join("state.json").exists());
296+
}
297+
259298
#[cfg(unix)]
260299
#[test]
261300
fn symlinked_inbox_cannot_escape_the_agent_capability() {
@@ -312,7 +351,7 @@ fn supersede_collapses_only_the_matching_key_and_preserves_archive_receipts() {
312351
fn keyless_supersede_replaces_the_stream_wide_head() {
313352
let catalog = tempfile::tempdir().unwrap();
314353
let agent = declare_agent(catalog.path(), "\"running\"", " stream \"gh-ci\" {}\n");
315-
let old = emit(catalog.path(), "old", None, true);
354+
let old = emit(catalog.path(), "old", Some("pr-1"), true);
316355
let new = emit(catalog.path(), "new", None, true);
317356
assert_eq!(new.superseded.as_deref(), Some(old.filename.as_str()));
318357
assert!(!message::inbox_dir(&agent).join(old.filename).exists());

0 commit comments

Comments
 (0)