Hare/yoi
1
1
Fork 0
Code Actions Packages Releases Activity

fix: retain repository access for workdir sessions

Browse Source
This commit is contained in:
Hare
2026-09-24 18:37:11 +09:00
parent 58d43af3e6
commit 3cd535cc8a
2 changed files with 250 additions and 261 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/worker-runtime/src/working_directory.rs
+97 -261
View File
@@ -17,7 +17,7 @@ use std::os::unix::net::{UnixListener, UnixStream};
use std::path::{Component, Path, PathBuf};
use std::process::{Command, Stdio};
use std::sync::atomic::{AtomicBool, AtomicU64, AtomicUsize, Ordering};
use std::sync::{Arc, Condvar, Mutex, Weak};
use std::sync::{Arc, Condvar, Mutex};
use std::thread::JoinHandle;
use std::time::{Duration, Instant, SystemTime, UNIX_EPOCH};
use workdir::WorkdirSessionResource;
@@ -263,24 +263,10 @@ struct PendingRepositoryAccessLease {
access: RepositorySshMaterializationAccess,
}
#[derive(Debug)]
struct ActiveRepositoryAccessLease {
generation: u64,
expires_at_epoch_seconds: u64,
access: Weak<RepositoryCommandAccess>,
}
#[derive(Debug, Clone)]
enum RepositoryAccessExpiryKey {
Pending(String),
Active(u64),
}
#[derive(Debug, Default)]
struct RepositoryAccessExpiryState {
next_generation: u64,
pending: HashMap<String, PendingRepositoryAccessLease>,
active: HashMap<u64, ActiveRepositoryAccessLease>,
shutdown: bool,
}
@@ -420,165 +406,80 @@ impl RepositoryAccessExpiryScheduler {
Ok(())
}
fn register_active(
&self,
access: &Arc<RepositoryCommandAccess>,
expires_at_epoch_seconds: u64,
) -> Result<RepositoryAccessExpiryRegistration, WorkingDirectoryDiagnostic> {
let mut state = self.inner.state.lock().map_err(|_| {
WorkingDirectoryDiagnostic::new(
"working_directory_repository_access_unavailable",
"Runtime Repository access state is unavailable",
)
})?;
if state.shutdown {
return Err(WorkingDirectoryDiagnostic::new(
"working_directory_repository_access_unavailable",
"Runtime Repository access state is shutting down",
));
}
let generation = Self::next_generation(&mut state)?;
let lease_id = generation;
state.active.insert(
lease_id,
ActiveRepositoryAccessLease {
generation,
expires_at_epoch_seconds,
access: Arc::downgrade(access),
},
);
drop(state);
self.inner.wake.notify_one();
Ok(RepositoryAccessExpiryRegistration {
scheduler: Arc::downgrade(&self.inner),
lease_id,
generation,
})
}
fn next_expiry(
state: &RepositoryAccessExpiryState,
) -> Option<(RepositoryAccessExpiryKey, u64, u64)> {
let mut next = None;
for (working_directory_id, lease) in &state.pending {
let candidate = (
RepositoryAccessExpiryKey::Pending(working_directory_id.clone()),
lease.generation,
lease.access.expires_at_epoch_seconds,
);
if next
.as_ref()
.map_or(true, |(_, _, expires_at)| candidate.2 < *expires_at)
{
next = Some(candidate);
}
}
for (lease_id, lease) in &state.active {
let candidate = (
RepositoryAccessExpiryKey::Active(*lease_id),
lease.generation,
lease.expires_at_epoch_seconds,
);
if next
.as_ref()
.map_or(true, |(_, _, expires_at)| candidate.2 < *expires_at)
{
next = Some(candidate);
}
}
next
fn next_expiry(state: &RepositoryAccessExpiryState) -> Option<(String, u64, u64)> {
state
.pending
.iter()
.map(|(working_directory_id, lease)| {
(
working_directory_id.clone(),
lease.generation,
lease.access.expires_at_epoch_seconds,
)
})
.min_by_key(|(_, _, expires_at)| *expires_at)
}
fn expire_if_current(
state: &mut RepositoryAccessExpiryState,
key: &RepositoryAccessExpiryKey,
working_directory_id: &str,
generation: u64,
now_epoch_seconds: u64,
) -> Option<Weak<RepositoryCommandAccess>> {
match key {
RepositoryAccessExpiryKey::Pending(working_directory_id) => {
let should_remove = state
.pending
.get(working_directory_id)
.is_some_and(|lease| {
lease.generation == generation
&& lease.access.expires_at_epoch_seconds <= now_epoch_seconds
});
if should_remove {
state.pending.remove(working_directory_id);
}
None
}
RepositoryAccessExpiryKey::Active(lease_id) => {
let should_remove = state.active.get(lease_id).is_some_and(|lease| {
lease.generation == generation
&& lease.expires_at_epoch_seconds <= now_epoch_seconds
});
if should_remove {
state.active.remove(lease_id).map(|lease| lease.access)
} else {
None
}
}
) {
let should_remove = state
.pending
.get(working_directory_id)
.is_some_and(|lease| {
lease.generation == generation
&& lease.access.expires_at_epoch_seconds <= now_epoch_seconds
});
if should_remove {
state.pending.remove(working_directory_id);
}
}
fn run(inner: Arc<RepositoryAccessExpirySchedulerInner>) {
loop {
let expired_access = {
let mut state = match inner.state.lock() {
Ok(state) => state,
Err(_) => return,
};
loop {
if state.shutdown {
return;
}
let Some((key, generation, expires_at_epoch_seconds)) =
Self::next_expiry(&state)
else {
state = match inner.wake.wait(state) {
Ok(state) => state,
Err(_) => return,
};
continue;
};
let now_epoch_seconds = repository_access_now_epoch_seconds();
if expires_at_epoch_seconds > now_epoch_seconds {
let wait = Duration::from_secs(
(expires_at_epoch_seconds - now_epoch_seconds)
.min(Self::MAX_WAIT.as_secs()),
);
state = match inner.wake.wait_timeout(state, wait) {
Ok((state, _)) => state,
Err(_) => return,
};
continue;
}
break Self::expire_if_current(&mut state, &key, generation, now_epoch_seconds);
}
let mut state = match inner.state.lock() {
Ok(state) => state,
Err(_) => return,
};
if let Some(access) = expired_access.and_then(|access| access.upgrade()) {
access.stop();
loop {
if state.shutdown {
return;
}
let Some((working_directory_id, generation, expires_at_epoch_seconds)) =
Self::next_expiry(&state)
else {
state = match inner.wake.wait(state) {
Ok(state) => state,
Err(_) => return,
};
continue;
};
let now_epoch_seconds = repository_access_now_epoch_seconds();
if expires_at_epoch_seconds > now_epoch_seconds {
let wait = Duration::from_secs(
(expires_at_epoch_seconds - now_epoch_seconds)
.min(Self::MAX_WAIT.as_secs()),
);
state = match inner.wake.wait_timeout(state, wait) {
Ok((state, _)) => state,
Err(_) => return,
};
continue;
}
Self::expire_if_current(
&mut state,
&working_directory_id,
generation,
now_epoch_seconds,
);
break;
}
}
}
fn cancel_active(inner: &RepositoryAccessExpirySchedulerInner, lease_id: u64, generation: u64) {
let Ok(mut state) = inner.state.lock() else {
return;
};
let should_remove = state
.active
.get(&lease_id)
.is_some_and(|lease| lease.generation == generation);
if should_remove {
state.active.remove(&lease_id);
}
drop(state);
inner.wake.notify_one();
}
#[cfg(test)]
fn pending_generation(&self, working_directory_id: &str) -> Option<u64> {
self.inner
@@ -602,7 +503,7 @@ impl RepositoryAccessExpiryScheduler {
};
Self::expire_if_current(
&mut state,
&RepositoryAccessExpiryKey::Pending(working_directory_id.to_string()),
working_directory_id,
generation,
now_epoch_seconds,
);
@@ -617,15 +518,6 @@ impl RepositoryAccessExpiryScheduler {
.unwrap_or_default()
}
#[cfg(test)]
fn active_count(&self) -> usize {
self.inner
.state
.lock()
.map(|state| state.active.len())
.unwrap_or_default()
}
#[cfg(test)]
fn worker_start_count(&self) -> usize {
self.inner.worker_starts.load(Ordering::Relaxed)
@@ -634,25 +526,15 @@ impl RepositoryAccessExpiryScheduler {
impl Drop for RepositoryAccessExpirySchedulerLifecycle {
fn drop(&mut self) {
let active = {
{
let mut state = match self.inner.state.lock() {
Ok(state) => state,
Err(poisoned) => poisoned.into_inner(),
};
state.shutdown = true;
state.pending.clear();
state
.active
.drain()
.map(|(_, lease)| lease.access)
.collect::<Vec<_>>()
};
self.inner.wake.notify_all();
for access in active {
if let Some(access) = access.upgrade() {
access.stop();
}
}
self.inner.wake.notify_all();
let worker = match self.worker.lock() {
Ok(mut worker) => worker.take(),
Err(mut poisoned) => poisoned.get_mut().take(),
@@ -663,25 +545,6 @@ impl Drop for RepositoryAccessExpirySchedulerLifecycle {
}
}
#[derive(Debug)]
struct RepositoryAccessExpiryRegistration {
scheduler: Weak<RepositoryAccessExpirySchedulerInner>,
lease_id: u64,
generation: u64,
}
impl Drop for RepositoryAccessExpiryRegistration {
fn drop(&mut self) {
if let Some(scheduler) = self.scheduler.upgrade() {
RepositoryAccessExpiryScheduler::cancel_active(
&scheduler,
self.lease_id,
self.generation,
);
}
}
}
fn repository_access_now_epoch_seconds() -> u64 {
SystemTime::now()
.duration_since(UNIX_EPOCH)
@@ -831,9 +694,6 @@ impl RuntimeGitMaterializer {
&binding.working_directory.repository_id,
&access,
)?);
let expiry_registration = self
.repository_access
.register_active(&command_access, access.expires_at_epoch_seconds)?;
binding
.command_environment
.insert("SSH_AUTH_SOCK".to_string(), "/dev/null".to_string());
@@ -849,9 +709,9 @@ impl RuntimeGitMaterializer {
}
.to_string(),
);
binding
.session_resources
.push(Arc::new(expiry_registration));
// The expiry bounds one-shot delivery before binding. Once bound, the
// brokered key stays Runtime-internal and is held by the Workdir session;
// closing the session drops this resource and revokes access.
binding.session_resources.push(command_access);
Ok(binding)
}
@@ -3134,26 +2994,6 @@ mod tests {
}
}
fn fake_repository_command_access(root: &Path) -> (Arc<RepositoryCommandAccess>, PathBuf) {
let access_root = root.join("command-access");
let agent_root = root.join("agent");
fs::create_dir_all(&access_root).unwrap();
fs::create_dir_all(&agent_root).unwrap();
let socket = agent_root.join("agent.sock");
fs::write(&socket, b"socket marker").unwrap();
let access = Arc::new(RepositoryCommandAccess {
root: access_root.clone(),
ssh_command: access_root.join("ssh-command"),
agent: Arc::new(RepositorySshAgent {
root: agent_root,
socket,
child: Mutex::new(None),
}),
ssh_broker: None,
});
(access, access_root)
}
#[test]
fn repository_access_expiry_fences_a_stale_pending_generation_after_refresh() {
let scheduler = RepositoryAccessExpiryScheduler::new();
@@ -3226,22 +3066,7 @@ mod tests {
}
#[test]
fn repository_access_expiry_unregisters_active_leases_on_session_close() {
let scheduler = RepositoryAccessExpiryScheduler::new();
let root = tempfile::tempdir().unwrap();
let (access, access_root) = fake_repository_command_access(root.path());
let registration = scheduler.register_active(&access, u64::MAX).unwrap();
assert_eq!(scheduler.active_count(), 1);
drop(registration);
assert_eq!(scheduler.active_count(), 0);
assert!(access_root.exists());
drop(access);
assert!(!access_root.exists());
}
#[test]
fn repository_access_expiry_expires_pending_and_active_leases() {
fn repository_access_expiry_discards_unused_pending_authority() {
let scheduler = RepositoryAccessExpiryScheduler::new();
let expiry = repository_access_now_epoch_seconds() + 1;
let mut pending = repository_ssh_access();
@@ -3249,38 +3074,24 @@ mod tests {
scheduler
.store_pending("working-directory-expiring", pending)
.unwrap();
let root = tempfile::tempdir().unwrap();
let (access, access_root) = fake_repository_command_access(root.path());
let registration = scheduler.register_active(&access, expiry).unwrap();
let deadline = std::time::Instant::now() + Duration::from_secs(3);
while (scheduler.pending_count() != 0
|| scheduler.active_count() != 0
|| access_root.exists())
&& std::time::Instant::now() < deadline
{
while scheduler.pending_count() != 0 && std::time::Instant::now() < deadline {
std::thread::sleep(Duration::from_millis(10));
}
assert_eq!(scheduler.pending_count(), 0);
assert_eq!(scheduler.active_count(), 0);
assert!(!access_root.exists());
drop(registration);
drop(access);
}
#[test]
fn repository_access_expiry_shutdown_revokes_and_joins() {
fn repository_access_expiry_shutdown_discards_pending_and_joins() {
let scheduler = RepositoryAccessExpiryScheduler::new();
let scheduler_inner = Arc::downgrade(&scheduler.inner);
let root = tempfile::tempdir().unwrap();
let (access, access_root) = fake_repository_command_access(root.path());
let registration = scheduler.register_active(&access, u64::MAX).unwrap();
scheduler
.store_pending("working-directory-pending", repository_ssh_access())
.unwrap();
drop(scheduler);
assert!(scheduler_inner.upgrade().is_none());
assert!(!access_root.exists());
drop(registration);
drop(access);
}
fn git(path: &Path, args: &[&str]) {
@@ -3867,10 +3678,17 @@ mod tests {
.evidence
.host_trust_revision = Some(1);
materializer.write_record(&ssh_backed_binding).unwrap();
let mut session_materialization = initial_materialization.clone();
let session_delivery_expiry = repository_access_now_epoch_seconds() + 1;
session_materialization
.ssh
.as_mut()
.expect("SSH materialization")
.expires_at_epoch_seconds = session_delivery_expiry;
materializer
.authorize_repository_access(&WorkingDirectoryRepositoryAccessRequest {
working_directory_id: id.clone(),
materialization: initial_materialization.clone(),
materialization: session_materialization,
})
.unwrap();
assert_eq!(materializer.list_working_directories().unwrap().len(), 1);
@@ -3895,6 +3713,24 @@ mod tests {
1
);
assert_eq!(environment["YOI_REPOSITORY_ACCESS"], "read_write");
let delivery_expiry_deadline = Instant::now() + Duration::from_secs(3);
while repository_access_now_epoch_seconds() <= session_delivery_expiry
&& Instant::now() < delivery_expiry_deadline
{
std::thread::sleep(Duration::from_millis(10));
}
std::thread::sleep(Duration::from_millis(50));
assert!(
ssh_command.exists(),
"consumed delivery expiry must not revoke a live Workdir session"
);
assert_eq!(
fs::read_dir(runtime_root.path().join(".repository-agents"))
.map(|entries| entries.count())
.unwrap_or_default(),
1,
"live Workdir session must retain its Runtime-internal agent"
);
drop(binding);
assert!(!ssh_command.exists());
assert_eq!(
docs/report/2026-09-23-remote-worker-repository-access-restore.md
+153
View File
@@ -0,0 +1,153 @@
# Repository secret delivery expiry deletes a live Workdir helper and blocks restore
## Symptom
The `workspace:wip` T-3 Coder completed a clean local commit but could not publish it:
```text
branch=work/T-3-wip-protocol
head=742442f17417bfe0828291addd0249bb8a754ae9
origin=git@gitea.hareworks.net:Hare/wip-rs.git
```
The non-force push failed before contacting the Git remote:
```text
fatal: cannot exec '/home/hare/.local/share/yoi/workdirs/.repository-access/f536517181e31ae9/ssh-command': No such file or directory
fatal: unable to fork
```
Stopping and restoring the Coder to reconstruct Repository access then failed twice with:
```text
worker_workdir_attachments_replace_failed: Remote Runtime 'arcadia' rejected request
(HTTP 401 Unauthorized): invalid Workspace capability token:
Workspace capability does not authorize this operation
```
No branch was pushed and no Merge Request was opened.
## Preserved work
The implementation was not lost:
- Runtime Worker: `arcadia:01a0cfe9-2763-7070-8098-2088696ec9d5`
- implementation Workdir: `001a0cfe90e4e000003`
- read-only reference Workdir: `001a0cfe90c9a000002`
- branch: `work/T-3-wip-protocol`
- commit: `742442f17417bfe0828291addd0249bb8a754ae9`
- observed checkout: clean
The Worker is stopped, but its persisted restore request, logical attachments, execution metadata, and checkout remain. Cleanup, recreation, manual credential injection, and direct repository mutation are unnecessary.
## Cause 1: a delivery deadline was reused as the bound helper lifetime
The Workspace Server makes Repository SSH secret delivery available for 300 seconds. It stores the serialized private-key candidates and pinned host entry behind a Runtime-bound `FetchOnce` resource handle. Runtime consumes that resource once before binding the Workdir; successful fetch removes the Server-side resource.
This deadline has a meaningful role only while the one-shot secret has not been consumed. It limits how long an unused delivery handle remains valid. It is not a lifetime intrinsic to the SSH key, Repository permission, or Git operation.
The Runtime nevertheless copied the same absolute deadline into an active expiry scheduler after Workdir bind. At expiry the scheduler stopped and removed the already-bound Repository command access while the Workdir session remained live.
The observed timing matches that behavior:
| Event | Timestamp |
| ----------------------------- | ----------------------: |
| T-3 Workdir linked to Coder | 2026-09-23 20:15:59 UTC |
| push failed on missing helper | 2026-09-23 20:29:42 UTC |
| elapsed | 13 minutes 43 seconds |
The push happened well after the five-minute delivery deadline.
### What the missing helper was
`GIT_SSH_COMMAND` did not point to a private key. It pointed to a Runtime-generated executable script:
```text
.repository-access/f536517181e31ae9/ssh-command
```
A bound access consists of:
```text
.repository-access/<access-id>/
├── ssh-command # invokes yoi-runtime __repository-ssh <b.sock> "$@"
├── known_hosts # pinned host key
└── b.sock # policy-enforcing Runtime broker socket
.repository-agents/<agent-id>/
└── agent.sock # Runtime-internal ssh-agent socket
```
Runtime loads the fetched key into its internal `ssh-agent` through `ssh-add -`. The Worker receives neither the raw key nor the agent socket: its ordinary `SSH_AUTH_SOCK` is `/dev/null`. A Git SSH invocation reaches only `ssh-command`, whose broker validates the destination, port, Repository path, Git service, and read/write ceiling before starting `ssh` with the internal agent.
Thus the lending boundary is the brokered Git operation, not a five-minute key lifetime.
At the erroneous deadline, `RepositoryCommandAccess::stop`:
1. stopped the broker and removed `b.sock`;
2. killed the internal `ssh-agent` and removed its directory;
3. removed the complete `.repository-access/<access-id>` directory, including `ssh-command` and `known_hosts`.
The live `LocalWorkdirSession` retained the original `GIT_SSH_COMMAND` string, leaving a stale path. Git therefore failed at `exec`; it never reached Gitea. This is not evidence of a rejected key or missing remote write permission.
## Cause 2: Server and Runtime disagreed on attachment-replacement permission
Restore reacquires Repository SSH authority before rebinding persisted Workdirs. In `WorkspaceApi::restore_workspace_worker`, the relevant order is:
1. send fresh Repository access for each SSH-backed Workdir;
2. replace the Worker's Workspace API binding;
3. replace the Worker's logical Workdir attachments;
4. restore the Runtime Worker and bind the Workdirs.
The observed diagnostic is from step 3, at:
```text
POST /v1/workers/{worker_id}/workdir-attachments
```
The Server capability classifier recognized paths containing `/attachments`, but `/workdir-attachments` does not contain that literal substring. Server therefore fell through to `runtime:read`. Runtime explicitly classified `/workdir-attachments` as `workers:input`.
```text
Server-issued operation: runtime:read
Runtime expectation: workers:input
```
The exact 401 text is `WorkspaceCapabilityVerificationError::WrongOperation`. Verification had already accepted token shape, Workspace trust/signature, Workspace, Runtime, binding revision, and Worker identity. Method, path, body-digest, and expiry checks occur later and were not reached.
Retrying the same restore without correcting the Server classifier deterministically returns the same 401.
## Failure chain
1. Runtime consumed the one-shot Repository secret and bound a policy broker to the Workdir session.
2. Five minutes later, the delivery deadline incorrectly stopped that live broker and deleted its helper files.
3. The Worker retained a stale `GIT_SSH_COMMAND`.
4. A later push failed before remote contact.
5. The Orchestrator stopped the Worker to reconstruct the binding.
6. Restore reacquired Repository access but signed attachment replacement as `runtime:read`.
7. Runtime required `workers:input` and rejected restore before Workdir bind.
The two defects are distinct. The first breaks long-running Workdir sessions; the second blocks the normal recovery path.
## Implemented correction
The correction preserves the existing security boundary without inventing a credential lifetime:
- the 300-second expiry remains on unconsumed one-shot secret delivery and pending pre-bind authority;
- binding validates and consumes that authority before its deadline;
- after bind, `RepositoryCommandAccess` is retained only as a Workdir session resource;
- every Worker Git SSH invocation still passes through the Runtime broker's host, Repository, service, and read/write policy checks;
- closing the Workdir session drops the resource, stops the broker and agent, and removes all helper files;
- expiry scheduler state and tests for active bound access are removed;
- a regression test waits beyond the consumed delivery deadline and proves the live helper remains until binding drop.
The restore correction adds `/workdir-attachments` to the Server's `workers:input` classifier and locks both Server issuance and Runtime verification with focused POST-route assertions.
## Safe recovery boundary
After deployment through the normal operator-controlled process:
1. restore the same stopped Worker;
2. confirm that the existing implementation Workdir remains clean at `742442f17417bfe0828291addd0249bb8a754ae9`;
3. let the assigned Coder perform the normal non-force push, remote-ref verification, Merge Request, and review flow.
No Runtime trust reprovisioning, credential rotation, manual helper construction, force push, Workdir copy, or Worker deletion is supported by the evidence.