Resolve host paths before mounting them, and stop reading stored data as choice
Four fixes that share a shape: a value already on disk, or one spelled
around a check, being taken at face value.
`/..` bind-mounted the entire host filesystem read-write. `is_filesystem_root`
was purely lexical — trim trailing separators, refuse what was left only if it
was empty or a bare `C:` — and nothing in the file ever called `canonicalize`,
so `/..`, `/./`, `/home/..`, `/etc/../` and `C:\..` all passed. The daemon
resolves them: `docker run -v /..:/mnt/probe` mounts the host root, and the app
mounts read-*write* into a container whose agent has passwordless sudo. It is
the escalation `check_mount_name_stays_under_workspace` exists to close,
reached through the host-path half of the mount instead of the mount-name half.
`classify_mount_source` replaces it and asks the OS: `canonicalize` applies
`..`, follows symlinks, and resolves 8.3 aliases and UNC spellings on Windows.
A path that cannot be resolved — `projects.json` synced from another machine,
a folder not created yet — falls back to a lexical collapse rather than being
refused, because refusing would make such a project unsavable; the gap is
bounded, since what resolution adds is a property of paths that exist. A path
that names no location at all (`C:x`, a relative path) is refused rather than
guessed at. Same check now guards `ssh_key_path` and `ca_cert_path`, whose
read-only mounts were whole-host disclosure at /tmp/.host-ssh.
Custom env var names had no charset check anywhere, so `BASH_FUNC_stat%%` —
bash's wire format for an exported shell function, body in the value — reached
the container environment verbatim. Latent today because the image's /bin/sh is
dash, but the pre-commit scrub runs `/bin/sh -c` as root and nothing pins that.
Keys are now shell identifiers, on the project and the global list both, with
the same grandfathering the folder rows get: a stored key is admitted, a new or
edited one is not.
The blank workspace row was persisted. The comment said it was dropped on save;
the code computed the filtered list and then saved the unfiltered one, so
"+ Add folder" plus a blur stored `{"Target": "/workspace/", "Source": ""}` and
the project could never be started or recreated again. Every save in the
section now goes through one filter, and a blur that changed nothing saves
nothing.
Widening the five `ClaudeCodeSettings` booleans to `Option<bool>` reinterpreted
every stored record. They were plain `bool`s that always serialised, so every
project ever saved carries an explicit `"env_scrub": false` that nobody chose —
and under the new merge that `Some(false)` beats a global `Some(true)`, where
the old rule let the global win. Upgrading silently turned five settings off,
"strip credentials from subprocess environments" among them. Deserialisation
now goes through a shim that dates the record by the presence of the
pre-widening `enable_session_recap` key and reads its `false`s as unset. The
fields skip serialising when unset, so an older binary can still parse
`projects.json` after a downgrade — a `null` would fail to parse and take the
whole list down, since `ProjectsStore` parses all-or-nothing.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GBq2rGum6GX7xXgsas1fDc
This commit is contained in:
@@ -195,7 +195,9 @@ pub(crate) fn load_secrets_for_project(project: &mut Project) {
|
||||
/// entire host filesystem read-write into a container whose agent has
|
||||
/// passwordless sudo. Anything short of a filesystem root is the user choosing
|
||||
/// a folder — the Browse button and the free-text field lead to the same place
|
||||
/// — so only the roots themselves are refused.
|
||||
/// — so only the roots themselves are refused, and *root* is answered by
|
||||
/// resolving the path rather than by reading it: `/..` is spelled like a folder
|
||||
/// and is the root. See [`classify_mount_source`].
|
||||
///
|
||||
/// This is the **whole** rule set, and it belongs to `add_project`, where every
|
||||
/// row is new by definition. `update_project` runs
|
||||
@@ -228,17 +230,31 @@ fn validate_one_path(p: &ProjectPath) -> Result<(), String> {
|
||||
return Err(format!("Mount name '{}' contains invalid characters. Use alphanumeric, dash, underscore, or dot.", p.mount_name));
|
||||
}
|
||||
check_mount_name_stays_under_workspace(&p.mount_name)?;
|
||||
if p.host_path.is_empty() {
|
||||
// Trimmed: a host path of spaces is not a folder, and `classify_mount_source`
|
||||
// is deliberately silent about a path with nothing in it — this is the
|
||||
// message that names the mount it belongs to.
|
||||
if p.host_path.trim().is_empty() {
|
||||
return Err(format!(
|
||||
"Folder mounted at '/workspace/{}' has no host path.",
|
||||
p.mount_name
|
||||
));
|
||||
}
|
||||
if is_filesystem_root(&p.host_path) {
|
||||
return Err(format!(
|
||||
"'{}' is a filesystem root. Choose the project folder itself — mounting the whole drive gives the container everything on it.",
|
||||
p.host_path
|
||||
));
|
||||
match classify_mount_source(&p.host_path) {
|
||||
None => {}
|
||||
Some(UnmountableHostPath::FilesystemRoot { resolved }) => {
|
||||
return Err(filesystem_root_message(
|
||||
&p.host_path,
|
||||
&resolved,
|
||||
"using it as a project folder",
|
||||
));
|
||||
}
|
||||
Some(UnmountableHostPath::NotAbsolute) => {
|
||||
return Err(format!(
|
||||
"'{}' is not a full path to a folder — where it lands is decided by wherever \
|
||||
Triple-C is running from rather than by you. Give the whole path.",
|
||||
p.host_path
|
||||
));
|
||||
}
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
@@ -264,6 +280,23 @@ fn check_mount_name_stays_under_workspace(mount_name: &str) -> Result<(), String
|
||||
mount_name
|
||||
));
|
||||
}
|
||||
// **An empty name is not refused here, and that is a live residual.** It
|
||||
// makes the target `/workspace/`, which the daemon normalises to
|
||||
// `/workspace` — so the host folder shadows the directory the other mounts
|
||||
// land in, and Docker then creates their mount points *inside it*, on the
|
||||
// host. It is not refused because it cannot be: an empty name is what a
|
||||
// half-filled row holds, `legacy_rows` shows those are already in
|
||||
// `projects.json`, and this function runs on grandfathered rows too, so
|
||||
// refusing it would make every such project unsavable — the exact
|
||||
// regression [`validate_project_paths_update`] exists to prevent.
|
||||
//
|
||||
// Introducing one is closed at both ends: `validate_one_path` refuses an
|
||||
// empty name on any new or edited row, and `WorkspaceSection` no longer
|
||||
// sends half-filled or blank ones. What remains is the *stored* row, and it
|
||||
// belongs where the mount is built — `docker::create_container` should skip
|
||||
// a row with no mount name or no host path, which is the same filter that
|
||||
// stops a stored blank row failing the create with
|
||||
// `field Source must not be empty`.
|
||||
if !mount_name.is_empty() && mount_name.chars().all(|c| c == '.') {
|
||||
return Err(format!(
|
||||
"Mount name '{}' is not a folder name — it names the directory the mount would sit in.",
|
||||
@@ -363,7 +396,8 @@ fn validate_project_paths_update(
|
||||
/// `/tmp/.host-ssh` and `/tmp/.host-ca` — and neither had any check at all, so
|
||||
/// `/` handed the whole host filesystem to the agent to read. Read-only, so
|
||||
/// this is disclosure rather than the read-write hole a `/` project folder is,
|
||||
/// but the fix is the same one line.
|
||||
/// but it is the same check: [`classify_mount_source`], resolved rather than
|
||||
/// spelled, so `/..` and `/home/..` are refused here too.
|
||||
///
|
||||
/// Same grandfathering as the folder list, for the same reason: a value already
|
||||
/// stored is already mounted on every start, and refusing an unrelated save
|
||||
@@ -379,29 +413,239 @@ fn validate_mounted_host_path(
|
||||
if stored.map(str::trim) == Some(value) {
|
||||
return Ok(());
|
||||
}
|
||||
if is_filesystem_root(value) {
|
||||
return Err(format!(
|
||||
"'{}' is a filesystem root, so setting it as {} would mount the whole drive into the \
|
||||
container. Choose the folder itself.",
|
||||
value, label
|
||||
));
|
||||
match classify_mount_source(value) {
|
||||
None => {}
|
||||
Some(UnmountableHostPath::FilesystemRoot { resolved }) => {
|
||||
return Err(filesystem_root_message(
|
||||
value,
|
||||
&resolved,
|
||||
&format!("setting it as {}", label),
|
||||
));
|
||||
}
|
||||
Some(UnmountableHostPath::NotAbsolute) => {
|
||||
return Err(format!(
|
||||
"'{}' is not a full path, so it cannot be used as {}. Give the whole path.",
|
||||
value, label
|
||||
));
|
||||
}
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Whether a host path is the root of a filesystem, in any spelling the three
|
||||
/// desktop platforms produce: `/`, a Windows drive root, or a bare UNC/share
|
||||
/// prefix. Trailing separators are ignored, so `C:\\` and `C:/` are the same
|
||||
/// answer.
|
||||
fn is_filesystem_root(host_path: &str) -> bool {
|
||||
let trimmed = host_path.trim_end_matches(['/', '\\']);
|
||||
if trimmed.is_empty() {
|
||||
// Nothing but separators: `/`, `\\`, `//`.
|
||||
return true;
|
||||
/// Why a host path may not be used as the source of a bind mount.
|
||||
///
|
||||
/// Two answers rather than a `bool` because they need different sentences, and
|
||||
/// because "is a root" is no longer a question about how the path is *spelled*
|
||||
/// — the refusal has to be able to say where the path actually landed.
|
||||
#[derive(Debug, PartialEq)]
|
||||
enum UnmountableHostPath {
|
||||
/// The path is, or resolves to, the root of a filesystem. `resolved` is
|
||||
/// what it lands on, which is the same string only when a root was typed
|
||||
/// outright.
|
||||
FilesystemRoot { resolved: String },
|
||||
/// The path does not name a location at all. Where it lands is decided by
|
||||
/// whatever directory Triple-C happens to be running from, so it can be a
|
||||
/// root tomorrow and a folder today, and nothing here can judge it.
|
||||
NotAbsolute,
|
||||
}
|
||||
|
||||
/// Length of a `C:` drive prefix at the head of `path`, or 0.
|
||||
///
|
||||
/// Duplicated from `commands::file_commands::drive_prefix_len`, together with
|
||||
/// [`is_windows_style_path`] and [`normalize_host_path`] below. Those are
|
||||
/// private to that module and it is not this branch's file to change; if the
|
||||
/// two copies are ever merged, that one is the original and carries the wider
|
||||
/// test coverage.
|
||||
fn drive_prefix_len(path: &str) -> usize {
|
||||
let b = path.as_bytes();
|
||||
if b.len() >= 2 && b[0].is_ascii_alphabetic() && b[1] == b':' {
|
||||
2
|
||||
} else {
|
||||
0
|
||||
}
|
||||
// `C:` — a drive with no path on it.
|
||||
let bytes = trimmed.as_bytes();
|
||||
bytes.len() == 2 && bytes[0].is_ascii_alphabetic() && bytes[1] == b':'
|
||||
}
|
||||
|
||||
/// Whether `path` is written in Windows form, and so whether `\` separates its
|
||||
/// components. On Linux a backslash is an ordinary filename character, which is
|
||||
/// why this is a question rather than an unconditional substitution.
|
||||
///
|
||||
/// Copy of `file_commands::is_windows_style_path` — see [`drive_prefix_len`].
|
||||
fn is_windows_style_path(path: &str) -> bool {
|
||||
cfg!(windows) || path.starts_with("\\\\") || drive_prefix_len(path) > 0
|
||||
}
|
||||
|
||||
/// `path` with its separators unified and any Win32 verbatim/device prefix
|
||||
/// removed — the form every rule below is expressed against.
|
||||
///
|
||||
/// `\\?\C:\Windows` and `\\?\UNC\server\share` name the *same locations* as
|
||||
/// `C:\Windows` and `\\server\share`; the prefix only turns off Win32 path
|
||||
/// parsing. Stripping it is what stops four characters being a bypass — and it
|
||||
/// has to run on our own output as well, because `std::fs::canonicalize` hands
|
||||
/// back exactly that spelling on Windows.
|
||||
///
|
||||
/// Copy of `file_commands::normalize_host_path` — see [`drive_prefix_len`].
|
||||
fn normalize_host_path(path: &str) -> String {
|
||||
let mut s = if is_windows_style_path(path) {
|
||||
path.replace('\\', "/")
|
||||
} else {
|
||||
path.to_string()
|
||||
};
|
||||
// Slicing by byte index is safe here only because a prefix matched
|
||||
// case-insensitively as ASCII is ASCII, so its end is a char boundary.
|
||||
for prefix in ["//?/unc/", "//./unc/"] {
|
||||
if s.len() >= prefix.len()
|
||||
&& s.as_bytes()[..prefix.len()].eq_ignore_ascii_case(prefix.as_bytes())
|
||||
{
|
||||
return format!("//{}", &s[prefix.len()..]);
|
||||
}
|
||||
}
|
||||
for prefix in ["//?/", "//./"] {
|
||||
if s.len() >= prefix.len()
|
||||
&& s.as_bytes()[..prefix.len()].eq_ignore_ascii_case(prefix.as_bytes())
|
||||
{
|
||||
s = s[prefix.len()..].to_string();
|
||||
break;
|
||||
}
|
||||
}
|
||||
s
|
||||
}
|
||||
|
||||
/// A normalised absolute path split into the root it hangs off and the part
|
||||
/// below it, or `None` when it names no location at all.
|
||||
///
|
||||
/// The three roots the desktop platforms have: `/`, a drive (`C:/`), and a UNC
|
||||
/// share (`//server/share` — the share *is* the root; `//server` alone names a
|
||||
/// machine and nothing on it).
|
||||
fn split_host_root(norm: &str) -> Option<(&str, &str)> {
|
||||
if let Some(rest) = norm.strip_prefix("//") {
|
||||
let mut parts = rest.splitn(3, '/');
|
||||
let server = parts.next().unwrap_or("");
|
||||
let share = parts.next().unwrap_or("");
|
||||
if server.is_empty() || share.is_empty() {
|
||||
// `//server`, `//server/`: no share, so nothing under it is named.
|
||||
return Some((norm, ""));
|
||||
}
|
||||
let root_len = 2 + server.len() + 1 + share.len();
|
||||
return Some((&norm[..root_len], &norm[root_len..]));
|
||||
}
|
||||
let drive = drive_prefix_len(norm);
|
||||
if drive > 0 {
|
||||
// `C:x` is drive-*relative* — it means "x under the current directory
|
||||
// on C:", which is a location only the process's own state decides.
|
||||
return match norm[drive..].strip_prefix('/') {
|
||||
Some(tail) => Some((&norm[..drive + 1], tail)),
|
||||
None if norm.len() == drive => Some((norm, "")),
|
||||
None => None,
|
||||
};
|
||||
}
|
||||
norm.strip_prefix('/').map(|tail| (&norm[..1], tail))
|
||||
}
|
||||
|
||||
/// How many named components deep `tail` ends up, with `.` dropped and `..`
|
||||
/// applied — clamped at the root, because `/..` is `/` and not an error.
|
||||
fn depth_below_root(tail: &str) -> usize {
|
||||
let mut depth = 0usize;
|
||||
for segment in tail.split('/') {
|
||||
match segment {
|
||||
"" | "." => {}
|
||||
".." => depth = depth.saturating_sub(1),
|
||||
_ => depth += 1,
|
||||
}
|
||||
}
|
||||
depth
|
||||
}
|
||||
|
||||
/// Whether a host path can be handed to Docker as a bind-mount source, and if
|
||||
/// not, why.
|
||||
///
|
||||
/// ## Resolved, not spelled
|
||||
///
|
||||
/// This used to be `is_filesystem_root`, and it was purely lexical: trim the
|
||||
/// trailing separators, say yes to what was left over only if it was empty or a
|
||||
/// bare `C:`. Nothing in this file called `canonicalize`, so `/..`, `/./`,
|
||||
/// `/home/..`, `/etc/../` and `C:\..` all sailed through and were passed
|
||||
/// verbatim to `docker::create_container`, which builds a
|
||||
/// `Mount { source, read_only: Some(false) }` out of them. Verified against the
|
||||
/// daemon: `-v /..:/mnt/probe` mounts the host root. That is the whole host
|
||||
/// filesystem, read-write, in a container whose agent has passwordless sudo —
|
||||
/// the same escalation `check_mount_name_stays_under_workspace` exists to
|
||||
/// close, reached through the host-path half of the mount instead of the
|
||||
/// mount-name half.
|
||||
///
|
||||
/// So the answer comes from the OS where the OS can give one: `canonicalize`
|
||||
/// applies `..`, follows every symlink in the path, and on Windows returns the
|
||||
/// long name for an 8.3 alias and the verbatim spelling of a UNC share — all
|
||||
/// things a string comparison cannot see.
|
||||
///
|
||||
/// ## When the path cannot be resolved
|
||||
///
|
||||
/// `canonicalize` fails on a path that does not exist *here*, which is an
|
||||
/// ordinary state rather than an attack: `projects.json` syncs between machines
|
||||
/// and names `C:\Users\jo\code` on a box that has never had a `C:`, and a
|
||||
/// folder can be created after the project is. Refusing outright would make
|
||||
/// every such project unsavable, which is the exact failure
|
||||
/// [`validate_project_paths_update`] exists to avoid — so an unresolvable path
|
||||
/// falls back to the lexical answer, with `.` and `..` collapsed by
|
||||
/// [`depth_below_root`] rather than ignored.
|
||||
///
|
||||
/// That is a weaker guarantee, not a wrong one, and the gap is bounded: what
|
||||
/// resolution adds over the lexical rule is symlinks and 8.3 aliases, and both
|
||||
/// of those are properties of a path that *exists* — precisely the case where
|
||||
/// `canonicalize` answers. What is left is a path that does not exist at save
|
||||
/// time and is a symlink to the root by the time the container starts, i.e. the
|
||||
/// user doing it to themselves after being asked.
|
||||
///
|
||||
/// Blocking on the filesystem here is deliberate: this runs on a save, once per
|
||||
/// row, and is a single `realpath` walk.
|
||||
fn classify_mount_source(host_path: &str) -> Option<UnmountableHostPath> {
|
||||
let raw = host_path.trim();
|
||||
if raw.is_empty() {
|
||||
// Emptiness is somebody else's error message — see
|
||||
// `validate_one_path`, which names the mount it belongs to.
|
||||
return None;
|
||||
}
|
||||
|
||||
// Nothing but separators, in either spelling: `/`, `//`, `\`, `\\`. Taken
|
||||
// first because a lone `\` is a *relative* name on Linux, and reporting a
|
||||
// Windows root as "not a full path" would be answering a question the user
|
||||
// did not ask.
|
||||
if raw.chars().all(|c| c == '/' || c == '\\') {
|
||||
return Some(UnmountableHostPath::FilesystemRoot {
|
||||
resolved: raw.to_string(),
|
||||
});
|
||||
}
|
||||
|
||||
let canonical = std::fs::canonicalize(raw)
|
||||
.ok()
|
||||
.map(|p| p.to_string_lossy().into_owned());
|
||||
let judged = canonical.as_deref().unwrap_or(raw);
|
||||
let norm = normalize_host_path(judged);
|
||||
|
||||
let Some((root, tail)) = split_host_root(&norm) else {
|
||||
return Some(UnmountableHostPath::NotAbsolute);
|
||||
};
|
||||
if depth_below_root(tail) == 0 {
|
||||
return Some(UnmountableHostPath::FilesystemRoot {
|
||||
resolved: root.to_string(),
|
||||
});
|
||||
}
|
||||
None
|
||||
}
|
||||
|
||||
/// The refusal for a host path that lands on a filesystem root, naming the
|
||||
/// resolved location as well as what was typed when those differ. `/..` reads
|
||||
/// as a folder; `/..` *is* `/`, and a message that only quoted it back would
|
||||
/// leave the user with nothing to act on.
|
||||
fn filesystem_root_message(typed: &str, resolved: &str, use_for: &str) -> String {
|
||||
let where_it_lands = if typed.trim() == resolved {
|
||||
format!("'{}' is a filesystem root", typed)
|
||||
} else {
|
||||
format!("'{}' resolves to '{}', which is a filesystem root", typed, resolved)
|
||||
};
|
||||
format!(
|
||||
"{}, so {} would mount the whole drive into the container. Choose the folder itself.",
|
||||
where_it_lands, use_for
|
||||
)
|
||||
}
|
||||
|
||||
#[tauri::command]
|
||||
@@ -551,6 +795,12 @@ pub async fn update_project(
|
||||
project.ca_cert_path.as_deref(),
|
||||
)?;
|
||||
|
||||
// Custom env var names had no charset check anywhere, so a key like
|
||||
// `BASH_FUNC_stat%%` reached the container environment verbatim. Same
|
||||
// grandfathering as the folder list, for the same reason — see
|
||||
// [`crate::models::validate_env_vars_update`].
|
||||
crate::models::validate_env_vars_update(&stored.custom_env_vars, &project.custom_env_vars)?;
|
||||
|
||||
project.container_id = stored.container_id;
|
||||
project.status = stored.status;
|
||||
project.created_at = stored.created_at;
|
||||
@@ -1159,6 +1409,130 @@ mod tests {
|
||||
assert!(validate_project_paths(&[path("C:\\Users\\u\\project", "project")]).is_ok());
|
||||
}
|
||||
|
||||
/// Every spelling of a root that is not *spelled* like one.
|
||||
///
|
||||
/// The predicate this replaces trimmed trailing separators and compared
|
||||
/// what was left, so `/..` — which the daemon mounts as the host root,
|
||||
/// verified with `docker run -v /..:/mnt/probe` — was indistinguishable
|
||||
/// from a project folder called `..`. No test in the repo contained a `.`
|
||||
/// or a `..` in a host path, which is why it shipped.
|
||||
#[test]
|
||||
fn a_host_path_that_resolves_to_a_root_is_refused() {
|
||||
let escapes = [
|
||||
"/..",
|
||||
"/../",
|
||||
"/./",
|
||||
"/.",
|
||||
"/home/..",
|
||||
"/etc/../",
|
||||
"/tmp/../..",
|
||||
// Deliberately not present on any machine, so this is the
|
||||
// unresolvable path taking the lexical route.
|
||||
"/no-such-dir-here/../..",
|
||||
"C:\\..",
|
||||
"C:\\Users\\..",
|
||||
"c:/foo/..",
|
||||
// Win32 verbatim spelling of a drive root.
|
||||
"\\\\?\\C:\\",
|
||||
"\\\\?\\C:\\..",
|
||||
// A UNC share root is the root of everything on that share, which
|
||||
// the old predicate accepted despite its doc comment claiming
|
||||
// otherwise.
|
||||
"\\\\server\\share",
|
||||
"//server/share/",
|
||||
];
|
||||
for escape in escapes {
|
||||
assert!(
|
||||
validate_project_paths(&[path(escape, "everything")]).is_err(),
|
||||
"host path '{}' was accepted as a project folder, which bind-mounts a whole \
|
||||
filesystem read-write into a container with passwordless sudo",
|
||||
escape
|
||||
);
|
||||
// The same value must not be reachable through the editor either.
|
||||
assert!(
|
||||
validate_project_paths_update(&[], &[path(escape, "everything")]).is_err(),
|
||||
"host path '{}' was accepted through update_project",
|
||||
escape
|
||||
);
|
||||
// And the two read-only mounts are the same check.
|
||||
assert!(
|
||||
validate_mounted_host_path("the SSH key folder", None, Some(escape)).is_err(),
|
||||
"'{}' was accepted as an SSH key path, which read-only bind-mounts a whole \
|
||||
filesystem at /tmp/.host-ssh",
|
||||
escape
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
/// A drive-relative path (`C:x`, no separator) means "x under whatever the
|
||||
/// current directory on C: happens to be" — a location decided by the
|
||||
/// process rather than by the user, so it may be the drive root.
|
||||
#[test]
|
||||
fn a_path_that_names_no_location_is_refused_rather_than_guessed_at() {
|
||||
for relative in ["C:x", "C:Users\\jo", "relative/path", "./project"] {
|
||||
assert!(
|
||||
validate_project_paths(&[path(relative, "project")]).is_err(),
|
||||
"'{}' was accepted, though where it lands depends on Triple-C's own \
|
||||
working directory",
|
||||
relative
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
/// The dots that are *not* an escape have to keep working — a folder can
|
||||
/// legitimately be reached through `.` or a `..` that goes back down again,
|
||||
/// and the Browse button produces paths on machines this list is not
|
||||
/// running on.
|
||||
#[test]
|
||||
fn an_ordinary_folder_is_still_accepted_however_it_is_spelled() {
|
||||
for ok in [
|
||||
"/home/u/./project",
|
||||
"/home/u/x/../project",
|
||||
"/home/u/..project",
|
||||
"/home/u/project/..hidden",
|
||||
"C:\\Users\\u\\x\\..\\project",
|
||||
"\\\\server\\share\\project",
|
||||
"\\\\?\\C:\\Users\\u\\project",
|
||||
] {
|
||||
assert!(
|
||||
validate_project_paths(&[path(ok, "project")]).is_ok(),
|
||||
"host path '{}' should be usable",
|
||||
ok
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
/// The half of this that only resolution can answer.
|
||||
///
|
||||
/// A lexical check sees a two-component path under `/tmp` and stops. The
|
||||
/// container is what plants the link — `/proc/self/mountinfo` inside a
|
||||
/// Triple-C container spells the host's project paths out verbatim — so the
|
||||
/// symlink is reachable, and the mount that follows it is read-write.
|
||||
#[cfg(unix)]
|
||||
#[test]
|
||||
fn a_symlink_to_the_root_is_refused_because_resolution_is_what_answers() {
|
||||
let dir = std::env::temp_dir().join(format!(
|
||||
"triple-c-root-link-{}-{}",
|
||||
std::process::id(),
|
||||
std::time::SystemTime::now()
|
||||
.duration_since(std::time::UNIX_EPOCH)
|
||||
.unwrap()
|
||||
.as_nanos()
|
||||
));
|
||||
std::fs::create_dir_all(&dir).unwrap();
|
||||
let link = dir.join("innocent");
|
||||
std::os::unix::fs::symlink("/", &link).unwrap();
|
||||
|
||||
let verdict = validate_project_paths(&[path(&link.to_string_lossy(), "project")]);
|
||||
std::fs::remove_file(&link).ok();
|
||||
std::fs::remove_dir(&dir).ok();
|
||||
|
||||
assert!(
|
||||
verdict.is_err(),
|
||||
"a symlink to / was accepted as a project folder; only canonicalisation can see it"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn duplicate_and_half_filled_rows_are_refused_but_the_blank_row_is_not() {
|
||||
assert!(validate_project_paths(&[
|
||||
|
||||
@@ -16,6 +16,15 @@ pub async fn update_settings(
|
||||
state: State<'_, AppState>,
|
||||
) -> Result<AppSettings, String> {
|
||||
let before = state.settings_store.get();
|
||||
|
||||
// The global half of the same rule the project half gets in
|
||||
// `update_project`: a global custom env var is merged into every project's
|
||||
// container environment, so an unchecked name here reaches all of them.
|
||||
crate::models::validate_env_vars_update(
|
||||
&before.global_custom_env_vars,
|
||||
&settings.global_custom_env_vars,
|
||||
)?;
|
||||
|
||||
let saved = state.settings_store.update(settings)?;
|
||||
|
||||
// Persisting a setting is not the same as applying it. The gateway is the
|
||||
|
||||
@@ -8,6 +8,100 @@ pub struct EnvVar {
|
||||
pub value: String,
|
||||
}
|
||||
|
||||
/// Whether `key` is a name a shell will read back as an ordinary variable:
|
||||
/// `[A-Za-z_][A-Za-z0-9_]*`.
|
||||
///
|
||||
/// ## Why a charset rule, and not just the reserved-name list
|
||||
///
|
||||
/// `docker::container::is_reserved_env_key` answers a different question — "is
|
||||
/// this one of the names Triple-C manages itself" — and nothing anywhere asked
|
||||
/// what the *characters* were. A key is joined into `KEY=VALUE` and handed to
|
||||
/// the daemon, which puts it in the container's environment verbatim, so a name
|
||||
/// that is not an identifier travels through unchallenged.
|
||||
///
|
||||
/// The one that matters is `BASH_FUNC_name%%`, bash's wire format for an
|
||||
/// exported shell function: bash imports those at startup and the *body* is the
|
||||
/// value. Today that is latent rather than live — the image's `/bin/sh` is
|
||||
/// dash, which does not import them, and an auditor confirmed the vector fires
|
||||
/// under `bash -c` and not under `sh -c` in the shipped image. But the
|
||||
/// pre-commit scrub runs `/bin/sh -c` **as root**, `/bin/sh` is whatever
|
||||
/// `ubuntu:24.04` points it at, and nothing pins that. One base-image change,
|
||||
/// or one call site spelled `bash`, turns a stored project setting into root
|
||||
/// code execution inside the container at commit time.
|
||||
///
|
||||
/// So the rule is the shape of the thing rather than a list of the names that
|
||||
/// are known to be dangerous: `IFS`, `LD_PRELOAD` and `PATH` are all perfectly
|
||||
/// good identifiers and are the user's business, while nothing legitimate needs
|
||||
/// a `%`, a `(` or a space in an environment variable name.
|
||||
///
|
||||
/// The key is judged **trimmed**, because that is what `create_container` sends
|
||||
/// — ` FOO ` already reaches the container as `FOO`, and refusing it here would
|
||||
/// break a setting that works.
|
||||
pub fn is_valid_env_key(key: &str) -> bool {
|
||||
let mut chars = key.trim().chars();
|
||||
match chars.next() {
|
||||
Some(c) if c.is_ascii_alphabetic() || c == '_' => {}
|
||||
_ => return false,
|
||||
}
|
||||
chars.all(|c| c.is_ascii_alphanumeric() || c == '_')
|
||||
}
|
||||
|
||||
/// Validate a custom environment variable list that is about to be stored,
|
||||
/// admitting the entries it is already stored with.
|
||||
///
|
||||
/// Same shape, and the same reasoning, as
|
||||
/// `commands::project_commands::validate_project_paths_update`: nothing ever
|
||||
/// checked these keys, so `projects.json` and `settings.json` in the field can
|
||||
/// hold whatever was typed. Holding every save to the new rule would make such
|
||||
/// a project unsavable *entirely* — `update_project` is the single command
|
||||
/// behind the whole Config tab — and would buy nothing, because the stored key
|
||||
/// is already being handed to every container that starts. An entry carried
|
||||
/// over verbatim is admitted; a new or edited one is held to the rule, which is
|
||||
/// what keeps the escalation closed, since escalation means *introducing* a bad
|
||||
/// key through this command.
|
||||
///
|
||||
/// Counted rather than set-tested, for the same reason as the folder rows: a
|
||||
/// second copy of an existing entry is a new entry.
|
||||
///
|
||||
/// The blank entry is not a violation. "+ Add variable" appends
|
||||
/// `{key: "", value: ""}` and saves the list immediately, so refusing it would
|
||||
/// turn the button itself into an error toast; `create_container` skips an
|
||||
/// empty key, so it reaches nothing.
|
||||
pub fn validate_env_vars_update(stored: &[EnvVar], incoming: &[EnvVar]) -> Result<(), String> {
|
||||
// An entry with no key is the placeholder, whatever is in its value:
|
||||
// `create_container` skips it, so it reaches nothing and there is nothing
|
||||
// to refuse. The editor saves on every blur, and typing the value before
|
||||
// the name is an ordinary way to fill a row in.
|
||||
let is_blank = |v: &EnvVar| v.key.trim().is_empty();
|
||||
|
||||
let mut carried: std::collections::HashMap<(&str, &str), usize> =
|
||||
std::collections::HashMap::new();
|
||||
for v in stored.iter().filter(|v| !is_blank(v)) {
|
||||
*carried
|
||||
.entry((v.key.as_str(), v.value.as_str()))
|
||||
.or_insert(0) += 1;
|
||||
}
|
||||
|
||||
for v in incoming.iter().filter(|v| !is_blank(v)) {
|
||||
match carried.get_mut(&(v.key.as_str(), v.value.as_str())) {
|
||||
Some(remaining) if *remaining > 0 => {
|
||||
*remaining -= 1;
|
||||
}
|
||||
_ => {
|
||||
if !is_valid_env_key(&v.key) {
|
||||
return Err(format!(
|
||||
"'{}' is not a usable environment variable name. Use a letter or \
|
||||
underscore followed by letters, digits or underscores.",
|
||||
v.key
|
||||
));
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)]
|
||||
pub struct ProjectPath {
|
||||
pub host_path: String,
|
||||
@@ -85,6 +179,7 @@ impl PermissionMode {
|
||||
/// Settings for Claude Code CLI behavior inside the container.
|
||||
/// These map to Claude Code env vars and ~/.claude/settings.json entries.
|
||||
#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Default)]
|
||||
#[serde(from = "StoredClaudeCodeSettings")]
|
||||
/// Every field is three-state, and the third state is load-bearing.
|
||||
///
|
||||
/// `None` means "not set at this level". For a *project* that is "inherit
|
||||
@@ -98,24 +193,24 @@ pub struct ClaudeCodeSettings {
|
||||
/// what lets Claude Code pick the renderer itself; `Some("default")` pins
|
||||
/// the classic main-screen renderer and `Some("fullscreen")` the alt-screen
|
||||
/// one. All three are distinct — "let it choose" is not "classic".
|
||||
#[serde(default)]
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub tui_mode: Option<String>,
|
||||
/// Saved `/effort` level: `None` = unset, otherwise one of
|
||||
/// `"low" | "medium" | "high" | "xhigh"`. Written to settings.json as
|
||||
/// `effortLevel` (**not** `effort`, which Claude Code has never read).
|
||||
#[serde(default)]
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub effort: Option<String>,
|
||||
/// Disable auto-scroll in fullscreen TUI mode. Held in the *disabled* sense
|
||||
/// because Claude Code's `autoScrollEnabled` defaults to `true`, so the
|
||||
/// zero value of this field has to mean "leave it on".
|
||||
#[serde(default)]
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub auto_scroll_disabled: Option<bool>,
|
||||
/// Collapse tool output to one-line summaries. Written to settings.json as
|
||||
/// `viewMode: "focus"`; there is no `focusMode` key in Claude Code.
|
||||
#[serde(default)]
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub focus_mode: Option<bool>,
|
||||
/// Show thinking summaries in responses
|
||||
#[serde(default)]
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub show_thinking_summaries: Option<bool>,
|
||||
/// Turn the session recap **off**.
|
||||
///
|
||||
@@ -128,16 +223,99 @@ pub struct ClaudeCodeSettings {
|
||||
/// never touched the control holds — as "the user turned the recap off" and
|
||||
/// silently disabled it for all of them. A new name lets the old key be
|
||||
/// ignored, which lands every existing project on the correct default.
|
||||
#[serde(default)]
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub session_recap_disabled: Option<bool>,
|
||||
/// Strip credentials from subprocess environments
|
||||
#[serde(default)]
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub env_scrub: Option<bool>,
|
||||
/// Enable 1-hour prompt cache TTL (vs default 5-minute)
|
||||
#[serde(default)]
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub prompt_caching_1h: Option<bool>,
|
||||
}
|
||||
|
||||
/// `ClaudeCodeSettings` in every shape `projects.json` and `settings.json` can
|
||||
/// be holding, which is what [`ClaudeCodeSettings`] is actually deserialised
|
||||
/// through.
|
||||
///
|
||||
/// ## The upgrade this exists to survive
|
||||
///
|
||||
/// Before the widening, the five booleans were plain `bool`s with
|
||||
/// `#[serde(default)]` and no `skip_serializing_if`, so **every** settings
|
||||
/// object ever written carries an explicit `"env_scrub": false` — not because
|
||||
/// anyone chose it, but because that is what a `bool` serialises to. Under the
|
||||
/// old merge (`if p.x { true } else { g.x }`) that `false` carried no
|
||||
/// information at all: it was the only value an unset switch could produce, and
|
||||
/// the global always won.
|
||||
///
|
||||
/// Read as `Some(false)` by the new code it becomes a *deliberate off* that
|
||||
/// beats a global `Some(true)` — so upgrading silently turned five settings off
|
||||
/// for every project that had ever opened this editor, `env_scrub` ("strip
|
||||
/// credentials from subprocess environments") among them. There is no store
|
||||
/// migration anywhere: `projects_store` parses these structs directly.
|
||||
///
|
||||
/// ## How an old record is told apart from a new one
|
||||
///
|
||||
/// By `enable_session_recap`. It was in the struct from the day it existed and
|
||||
/// was a plain `bool`, so its key is present in every pre-widening record and
|
||||
/// in no other — the field was *renamed* to `session_recap_disabled` precisely
|
||||
/// so the old key could be ignored (see the doc on that field), and the new
|
||||
/// code has never written it. Its presence is therefore an exact statement that
|
||||
/// these bytes were written by a binary in which `false` meant "unset", and the
|
||||
/// booleans are read back that way: `true` is a real choice and survives,
|
||||
/// `false` becomes `None` and inherits again.
|
||||
///
|
||||
/// Nothing marks a *new* record, and nothing needs to: absent is `None` (the
|
||||
/// fields skip serialising when unset) and a present `false` is the deliberate
|
||||
/// off the widening was for. That is also what keeps a downgrade survivable —
|
||||
/// an older binary reads an absent key as `false` through its own
|
||||
/// `#[serde(default)]`, where a `null` would fail to parse and take the whole
|
||||
/// of `projects.json` down with it, since `ProjectsStore` parses all-or-nothing
|
||||
/// and starts empty on an error.
|
||||
#[derive(Deserialize)]
|
||||
struct StoredClaudeCodeSettings {
|
||||
#[serde(default)]
|
||||
tui_mode: Option<String>,
|
||||
#[serde(default)]
|
||||
effort: Option<String>,
|
||||
#[serde(default)]
|
||||
auto_scroll_disabled: Option<bool>,
|
||||
#[serde(default)]
|
||||
focus_mode: Option<bool>,
|
||||
#[serde(default)]
|
||||
show_thinking_summaries: Option<bool>,
|
||||
#[serde(default)]
|
||||
session_recap_disabled: Option<bool>,
|
||||
#[serde(default)]
|
||||
env_scrub: Option<bool>,
|
||||
#[serde(default)]
|
||||
prompt_caching_1h: Option<bool>,
|
||||
/// The pre-widening spelling of `session_recap_disabled`, and the *only*
|
||||
/// use of its value: presence dates the record. Its meaning was inverted
|
||||
/// and it never worked, so it is read for the marker and discarded.
|
||||
#[serde(default)]
|
||||
enable_session_recap: Option<bool>,
|
||||
}
|
||||
|
||||
impl From<StoredClaudeCodeSettings> for ClaudeCodeSettings {
|
||||
fn from(stored: StoredClaudeCodeSettings) -> Self {
|
||||
let pre_widening = stored.enable_session_recap.is_some();
|
||||
// On a pre-widening record `false` is what an untouched switch wrote,
|
||||
// so it means "not set at this level" and must inherit. A `true` was a
|
||||
// real choice either way.
|
||||
let read = |v: Option<bool>| if pre_widening { v.filter(|on| *on) } else { v };
|
||||
ClaudeCodeSettings {
|
||||
tui_mode: stored.tui_mode,
|
||||
effort: stored.effort,
|
||||
auto_scroll_disabled: read(stored.auto_scroll_disabled),
|
||||
focus_mode: read(stored.focus_mode),
|
||||
show_thinking_summaries: read(stored.show_thinking_summaries),
|
||||
session_recap_disabled: read(stored.session_recap_disabled),
|
||||
env_scrub: read(stored.env_scrub),
|
||||
prompt_caching_1h: read(stored.prompt_caching_1h),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, Serialize, Deserialize)]
|
||||
pub struct Project {
|
||||
pub id: String,
|
||||
@@ -467,3 +645,170 @@ impl Project {
|
||||
val
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
||||
// ── Custom environment variable names ─────────────────────────────────
|
||||
|
||||
#[test]
|
||||
fn an_env_var_name_has_to_be_a_shell_identifier() {
|
||||
for ok in ["PATH", "_", "_x", "MY_VAR2", "a", " SPACED_BY_THE_EDITOR "] {
|
||||
assert!(is_valid_env_key(ok), "'{}' should be a usable name", ok);
|
||||
}
|
||||
for bad in [
|
||||
// bash's wire format for an exported shell function: the value is
|
||||
// the body, and a `bash` that imports it runs it. The scrub exec is
|
||||
// `/bin/sh -c` as root, and nothing pins `/bin/sh` to dash.
|
||||
"BASH_FUNC_stat%%",
|
||||
"BASH_FUNC_ls()",
|
||||
"MY VAR",
|
||||
"2FAST",
|
||||
"WITH-DASH",
|
||||
"WITH.DOT",
|
||||
"",
|
||||
" ",
|
||||
"$(id)",
|
||||
"A=B",
|
||||
] {
|
||||
assert!(!is_valid_env_key(bad), "'{}' should be refused", bad);
|
||||
}
|
||||
}
|
||||
|
||||
fn env(key: &str, value: &str) -> EnvVar {
|
||||
EnvVar { key: key.to_string(), value: value.to_string() }
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_bad_env_var_name_cannot_be_introduced_but_a_stored_one_does_not_brick_the_editor() {
|
||||
let bad = [env("BASH_FUNC_stat%%", "() { id; }")];
|
||||
// Introducing it through the Config tab is the escalation.
|
||||
assert!(validate_env_vars_update(&[], &bad).is_err());
|
||||
// Already stored: it is handed to every container that starts whether
|
||||
// or not an unrelated save is allowed through, and refusing the save
|
||||
// would make every toggle on the Config tab fail.
|
||||
assert!(validate_env_vars_update(&bad, &bad).is_ok());
|
||||
// Editing its value is a new entry, and refused again.
|
||||
assert!(
|
||||
validate_env_vars_update(&bad, &[env("BASH_FUNC_stat%%", "() { rm -rf /; }")]).is_err()
|
||||
);
|
||||
// Fixing the name is what the message asks for, and it saves.
|
||||
assert!(validate_env_vars_update(&bad, &[env("STAT", "() { id; }")]).is_ok());
|
||||
// Dropping it entirely is always fine.
|
||||
assert!(validate_env_vars_update(&bad, &[]).is_ok());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn the_blank_row_the_add_button_saves_is_not_an_error() {
|
||||
// "+ Add variable" appends an empty entry and saves the list at once,
|
||||
// so this is the button, not an attempt at anything.
|
||||
assert!(validate_env_vars_update(&[], &[env("", "")]).is_ok());
|
||||
// Typing the value before the name is an ordinary way to fill it in,
|
||||
// and an entry with no name reaches no container either way.
|
||||
assert!(validate_env_vars_update(&[], &[env("", "value-first")]).is_ok());
|
||||
assert!(validate_env_vars_update(&[], &[env("GOOD", "v"), env("", "")]).is_ok());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_stored_entry_may_be_kept_but_not_multiplied() {
|
||||
let stored = [env("BAD NAME", "v")];
|
||||
assert!(validate_env_vars_update(&stored, &stored).is_ok());
|
||||
// A second copy is a new entry, and held to the rule.
|
||||
assert!(
|
||||
validate_env_vars_update(&stored, &[env("BAD NAME", "v"), env("BAD NAME", "v")])
|
||||
.is_err()
|
||||
);
|
||||
}
|
||||
|
||||
// ── Claude Code settings written before the fields were widened ───────
|
||||
|
||||
/// `projects.json` exactly as the shipped `main` binary wrote it: the five
|
||||
/// booleans were plain `bool`s that always serialised, so every project
|
||||
/// that ever opened the editor carries `false` for the ones it never
|
||||
/// touched.
|
||||
const MAIN_SHAPE_PROJECT: &str = r#"{
|
||||
"id": "p1",
|
||||
"name": "demo",
|
||||
"paths": [{ "host_path": "/home/u/demo", "mount_name": "demo" }],
|
||||
"container_id": null,
|
||||
"status": "stopped",
|
||||
"backend": "anthropic",
|
||||
"bedrock_config": null,
|
||||
"ollama_config": null,
|
||||
"openai_compatible_config": null,
|
||||
"allow_docker_access": false,
|
||||
"ssh_key_path": null,
|
||||
"git_user_name": null,
|
||||
"git_user_email": null,
|
||||
"claude_code_settings": {
|
||||
"tui_mode": "fullscreen",
|
||||
"effort": null,
|
||||
"auto_scroll_disabled": false,
|
||||
"focus_mode": false,
|
||||
"show_thinking_summaries": false,
|
||||
"enable_session_recap": false,
|
||||
"env_scrub": false,
|
||||
"prompt_caching_1h": false
|
||||
},
|
||||
"created_at": "2026-01-01T00:00:00Z",
|
||||
"updated_at": "2026-01-01T00:00:00Z"
|
||||
}"#;
|
||||
|
||||
#[test]
|
||||
fn a_setting_stored_as_false_by_the_old_binary_still_inherits_the_global() {
|
||||
let project: Project = serde_json::from_str(MAIN_SHAPE_PROJECT).unwrap();
|
||||
let stored = project.claude_code_settings.expect("settings should parse");
|
||||
|
||||
// Read verbatim these would be `Some(false)`, which under
|
||||
// `docker::container::merge_claude_code_settings` beats the global.
|
||||
assert_eq!(stored.env_scrub, None);
|
||||
assert_eq!(stored.auto_scroll_disabled, None);
|
||||
assert_eq!(stored.focus_mode, None);
|
||||
assert_eq!(stored.show_thinking_summaries, None);
|
||||
assert_eq!(stored.prompt_caching_1h, None);
|
||||
assert_eq!(stored.session_recap_disabled, None);
|
||||
// A value the user did choose is untouched.
|
||||
assert_eq!(stored.tui_mode.as_deref(), Some("fullscreen"));
|
||||
|
||||
// The merge rule itself, spelled the way
|
||||
// `merge_claude_code_settings` spells it. `main` resolved this with
|
||||
// `if p.env_scrub { true } else { g.env_scrub }`, i.e. the global won —
|
||||
// and it has to go on winning, because the user never turned this off.
|
||||
let global = ClaudeCodeSettings { env_scrub: Some(true), ..Default::default() };
|
||||
assert_eq!(
|
||||
stored.env_scrub.or(global.env_scrub),
|
||||
Some(true),
|
||||
"upgrading silently turned off 'strip credentials from subprocess environments'"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn an_off_chosen_in_the_new_editor_still_beats_a_global_on() {
|
||||
// Same record without the pre-widening key: this `false` is the
|
||||
// deliberate off the widening exists to make expressible.
|
||||
let json = r#"{ "env_scrub": false }"#;
|
||||
let chosen: ClaudeCodeSettings = serde_json::from_str(json).unwrap();
|
||||
assert_eq!(chosen.env_scrub, Some(false));
|
||||
let global = ClaudeCodeSettings { env_scrub: Some(true), ..Default::default() };
|
||||
assert_eq!(chosen.env_scrub.or(global.env_scrub), Some(false));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn an_unset_setting_is_written_as_absent_rather_than_null() {
|
||||
// A downgrade parses these fields as plain `bool` with
|
||||
// `#[serde(default)]`: an absent key is `false`, a `null` is a parse
|
||||
// error — and `ProjectsStore` parses all-or-nothing, so one project
|
||||
// with one null empties the whole list and the next save persists that.
|
||||
let json = serde_json::to_string(&ClaudeCodeSettings::default()).unwrap();
|
||||
assert_eq!(json, "{}");
|
||||
assert!(!json.contains("null"));
|
||||
|
||||
let partial = ClaudeCodeSettings { env_scrub: Some(false), ..Default::default() };
|
||||
let json = serde_json::to_string(&partial).unwrap();
|
||||
assert_eq!(json, r#"{"env_scrub":false}"#);
|
||||
// And it reads back as what it is.
|
||||
let round_tripped: ClaudeCodeSettings = serde_json::from_str(&json).unwrap();
|
||||
assert_eq!(round_tripped, partial);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -193,4 +193,27 @@ describe("ClaudeCodeSettingsEditor", () => {
|
||||
expect(screen.getByRole("switch", { name: label })).toBeChecked();
|
||||
});
|
||||
});
|
||||
|
||||
/**
|
||||
* A settings object with nothing set at this level arrives as `{}`: the Rust
|
||||
* struct skips serialising a field it has no value for, which is what keeps
|
||||
* an older binary able to parse `projects.json` after a downgrade. It is also
|
||||
* the exact shape a project stored before the fields were widened is read
|
||||
* back as — every one of its `false`s meant "unset" — so reading absent as
|
||||
* "off" would show a switch the user never touched as a deliberate choice.
|
||||
*/
|
||||
it("reads an absent field as Global rather than as Off", () => {
|
||||
renderEditor({} as ClaudeCodeSettings, "project");
|
||||
expect((screen.getByLabelText("Env scrub") as HTMLSelectElement).value).toBe("global");
|
||||
expect((screen.getByLabelText("Session recap") as HTMLSelectElement).value).toBe("global");
|
||||
});
|
||||
|
||||
it("still collapses to null when an absent-field object is edited back", () => {
|
||||
const onSave = renderEditor({} as ClaudeCodeSettings, "global");
|
||||
// Off and straight back on: the round trip has to land on `null`, or an
|
||||
// untouched global stops being indistinguishable from one never opened.
|
||||
fireEvent.click(screen.getByRole("switch", { name: "Session recap" }));
|
||||
fireEvent.click(screen.getByRole("switch", { name: "Session recap" }));
|
||||
expect(onSave).toHaveBeenLastCalledWith(null);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -37,15 +37,20 @@ export const CLAUDE_CODE_DEFAULTS: ClaudeCodeSettings = {
|
||||
* overrides a global on, so a settings object holding one has to be persisted.
|
||||
*/
|
||||
function isAllDefaults(s: ClaudeCodeSettings): boolean {
|
||||
// `== null`, not `===`: an unset field is *absent* on the wire, not null.
|
||||
// The Rust struct skips serialising one it has no value for, so a project
|
||||
// whose stored settings were all "unset" arrives here as `{}` — and reading
|
||||
// that as "off" is exactly the mistake the three-state control exists to
|
||||
// avoid. See the note on `ClaudeCodeSettings` in `lib/types.ts`.
|
||||
return (
|
||||
s.tui_mode === null &&
|
||||
s.effort === null &&
|
||||
s.auto_scroll_disabled === null &&
|
||||
s.focus_mode === null &&
|
||||
s.show_thinking_summaries === null &&
|
||||
s.session_recap_disabled === null &&
|
||||
s.env_scrub === null &&
|
||||
s.prompt_caching_1h === null
|
||||
s.tui_mode == null &&
|
||||
s.effort == null &&
|
||||
s.auto_scroll_disabled == null &&
|
||||
s.focus_mode == null &&
|
||||
s.show_thinking_summaries == null &&
|
||||
s.session_recap_disabled == null &&
|
||||
s.env_scrub == null &&
|
||||
s.prompt_caching_1h == null
|
||||
);
|
||||
}
|
||||
|
||||
@@ -204,7 +209,7 @@ export default function ClaudeCodeSettingsEditor({
|
||||
// `stored` holds the deviation from Claude Code's default, so an
|
||||
// inverted field reads back the other way round — see BOOLEAN_FIELDS.
|
||||
const selected =
|
||||
stored === null ? "global" : (invert ? !stored : stored) ? "on" : "off";
|
||||
stored == null ? "global" : (invert ? !stored : stored) ? "on" : "off";
|
||||
|
||||
return (
|
||||
<SwitchRow
|
||||
|
||||
@@ -0,0 +1,177 @@
|
||||
import { describe, it, expect, vi, beforeEach } from "vitest";
|
||||
import { render, screen, fireEvent, act } from "@testing-library/react";
|
||||
import WorkspaceSection from "./WorkspaceSection";
|
||||
import type { Project } from "../../../../lib/types";
|
||||
|
||||
// The Browse button is the OS folder picker.
|
||||
const open = vi.fn();
|
||||
vi.mock("@tauri-apps/plugin-dialog", () => ({
|
||||
open: (...args: unknown[]) => open(...args),
|
||||
}));
|
||||
|
||||
const baseProject: Project = {
|
||||
id: "p1",
|
||||
name: "api-server",
|
||||
paths: [{ host_path: "/src/api", mount_name: "api" }],
|
||||
container_id: null,
|
||||
status: "stopped",
|
||||
backend: "anthropic",
|
||||
bedrock_config: null,
|
||||
ollama_config: null,
|
||||
llamacpp_config: null,
|
||||
openai_compatible_config: null,
|
||||
allow_docker_access: false,
|
||||
sandbox_mode_enabled: true,
|
||||
mission_control_enabled: false,
|
||||
auth_bridge_enabled: false,
|
||||
browser_view_enabled: false,
|
||||
vpn_support_enabled: false,
|
||||
use_shared_auth_token: true,
|
||||
full_permissions: false,
|
||||
permission_mode: null,
|
||||
ssh_key_path: null,
|
||||
ca_cert_path: null,
|
||||
git_token: null,
|
||||
git_user_name: null,
|
||||
git_user_email: null,
|
||||
custom_env_vars: [],
|
||||
port_mappings: [],
|
||||
claude_instructions: null,
|
||||
claude_code_settings: null,
|
||||
renamed_session_names: {},
|
||||
created_at: "2026-01-01T00:00:00Z",
|
||||
updated_at: "2026-01-01T00:00:00Z",
|
||||
};
|
||||
|
||||
const save = vi.fn().mockResolvedValue(true);
|
||||
|
||||
function renderSection(over: Partial<Project> = {}, disabled = false) {
|
||||
return render(
|
||||
<WorkspaceSection
|
||||
project={{ ...baseProject, ...over }}
|
||||
save={save}
|
||||
disabled={disabled}
|
||||
/>,
|
||||
);
|
||||
}
|
||||
|
||||
/** Every folder list this component has sent to `update_project`. */
|
||||
function savedLists() {
|
||||
return save.mock.calls
|
||||
.filter(([patch]) => "paths" in patch)
|
||||
.map(([patch]) => patch.paths);
|
||||
}
|
||||
|
||||
describe("WorkspaceSection — the blank row is never stored", () => {
|
||||
beforeEach(() => vi.clearAllMocks());
|
||||
|
||||
/**
|
||||
* The bug this file exists for. `create_container` mounts every stored row
|
||||
* unfiltered, so a persisted `{host_path: "", mount_name: ""}` becomes
|
||||
* `{"Target": "/workspace/", "Source": ""}` and the daemon refuses the whole
|
||||
* container with `field Source must not be empty` — the project can never be
|
||||
* started or recreated again. Click "+ Add folder", blur a field, and it is
|
||||
* bricked.
|
||||
*/
|
||||
it("drops the placeholder row when a real edit is saved", () => {
|
||||
renderSection();
|
||||
fireEvent.click(screen.getByRole("button", { name: "+ Add folder" }));
|
||||
|
||||
const hostPath = screen.getByLabelText("Folder 1 host path");
|
||||
fireEvent.change(hostPath, { target: { value: "/src/api-v2" } });
|
||||
fireEvent.blur(hostPath);
|
||||
|
||||
expect(save).toHaveBeenCalledTimes(1);
|
||||
expect(savedLists()[0]).toEqual([{ host_path: "/src/api-v2", mount_name: "api" }]);
|
||||
});
|
||||
|
||||
it("drops it when Browse fills a different row in", async () => {
|
||||
open.mockResolvedValueOnce("/src/api-v2");
|
||||
renderSection();
|
||||
fireEvent.click(screen.getByRole("button", { name: "+ Add folder" }));
|
||||
|
||||
// The picker is awaited inside the handler, so the state update that
|
||||
// follows it lands outside the click.
|
||||
await act(async () => {
|
||||
fireEvent.click(screen.getAllByRole("button", { name: "Browse" })[0]);
|
||||
});
|
||||
|
||||
expect(savedLists()[0]).toEqual([{ host_path: "/src/api-v2", mount_name: "api" }]);
|
||||
});
|
||||
|
||||
it("drops it when a row is removed", () => {
|
||||
renderSection({
|
||||
paths: [
|
||||
{ host_path: "/src/api", mount_name: "api" },
|
||||
{ host_path: "/src/web", mount_name: "web" },
|
||||
],
|
||||
});
|
||||
fireEvent.click(screen.getByRole("button", { name: "+ Add folder" }));
|
||||
fireEvent.click(screen.getByRole("button", { name: "Remove folder 2" }));
|
||||
|
||||
expect(savedLists()[0]).toEqual([{ host_path: "/src/api", mount_name: "api" }]);
|
||||
});
|
||||
|
||||
it("never sends a row with an empty host path, whatever the route", () => {
|
||||
renderSection();
|
||||
fireEvent.click(screen.getByRole("button", { name: "+ Add folder" }));
|
||||
const hostPath = screen.getByLabelText("Folder 1 host path");
|
||||
fireEvent.change(hostPath, { target: { value: "/src/api-v2" } });
|
||||
fireEvent.blur(hostPath);
|
||||
|
||||
for (const list of savedLists()) {
|
||||
for (const row of list) {
|
||||
expect(row.host_path).not.toBe("");
|
||||
expect(row.mount_name).not.toBe("");
|
||||
}
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe("WorkspaceSection — what a blur is allowed to save", () => {
|
||||
beforeEach(() => vi.clearAllMocks());
|
||||
|
||||
/**
|
||||
* Both inputs save on blur, so tabbing from the host path to the mount name
|
||||
* fires a save with the name still empty — which `update_project` refuses,
|
||||
* turning an ordinary keystroke into an error toast.
|
||||
*/
|
||||
it("holds a half-filled row back until it is complete", () => {
|
||||
renderSection();
|
||||
fireEvent.click(screen.getByRole("button", { name: "+ Add folder" }));
|
||||
|
||||
const newHostPath = screen.getByLabelText("Folder 2 host path");
|
||||
fireEvent.change(newHostPath, { target: { value: "/src/web" } });
|
||||
fireEvent.blur(newHostPath);
|
||||
expect(save).not.toHaveBeenCalled();
|
||||
|
||||
const newMountName = screen.getByLabelText("Folder 2 mount name");
|
||||
fireEvent.change(newMountName, { target: { value: "web" } });
|
||||
fireEvent.blur(newMountName);
|
||||
expect(savedLists()[0]).toEqual([
|
||||
{ host_path: "/src/api", mount_name: "api" },
|
||||
{ host_path: "/src/web", mount_name: "web" },
|
||||
]);
|
||||
});
|
||||
|
||||
/**
|
||||
* Blurring out of an untouched field is not an edit. Saving anyway would
|
||||
* round-trip the filtered list through `project` and take the empty row away
|
||||
* while the user was still filling it in.
|
||||
*/
|
||||
it("saves nothing when the blur changed nothing", () => {
|
||||
renderSection();
|
||||
fireEvent.click(screen.getByRole("button", { name: "+ Add folder" }));
|
||||
fireEvent.blur(screen.getByLabelText("Folder 1 mount name"));
|
||||
expect(save).not.toHaveBeenCalled();
|
||||
expect(screen.getByLabelText("Folder 2 host path")).toBeTruthy();
|
||||
});
|
||||
|
||||
it("still saves a rename, which does not go through the folder list", () => {
|
||||
renderSection();
|
||||
const name = screen.getByDisplayValue("api-server");
|
||||
fireEvent.change(name, { target: { value: "api-v2" } });
|
||||
fireEvent.blur(name);
|
||||
expect(save).toHaveBeenCalledWith({ name: "api-v2" });
|
||||
});
|
||||
});
|
||||
@@ -10,6 +10,14 @@ interface Props {
|
||||
disabled: boolean;
|
||||
}
|
||||
|
||||
/** Whether two folder lists are the same rows in the same order. */
|
||||
function sameRows(a: ProjectPath[], b: ProjectPath[]): boolean {
|
||||
return (
|
||||
a.length === b.length &&
|
||||
a.every((row, i) => row.host_path === b[i].host_path && row.mount_name === b[i].mount_name)
|
||||
);
|
||||
}
|
||||
|
||||
export default function WorkspaceSection({ project, save, disabled }: Props) {
|
||||
const [name, setName] = useState(project.name);
|
||||
const [paths, setPaths] = useState<ProjectPath[]>(project.paths ?? []);
|
||||
@@ -19,6 +27,27 @@ export default function WorkspaceSection({ project, save, disabled }: Props) {
|
||||
setPaths(project.paths ?? []);
|
||||
}, [project]);
|
||||
|
||||
/**
|
||||
* Persist a folder list, minus the rows that are only in it because the UI
|
||||
* put them there.
|
||||
*
|
||||
* **The blank row must never reach the store.** "+ Add folder" inserts
|
||||
* `{host_path: "", mount_name: ""}` deliberately, and `create_container`
|
||||
* mounts every stored row unfiltered — a stored blank one becomes
|
||||
* `{"Target": "/workspace/", "Source": ""}`, which the daemon rejects with
|
||||
* `field Source must not be empty`. The project then cannot be started or
|
||||
* recreated at all, from a click and a blur. `AddProjectDialog` has always
|
||||
* filtered this; this section computed the filtered list and then saved the
|
||||
* unfiltered one.
|
||||
*
|
||||
* Every save goes through here for that reason — Browse and Remove write the
|
||||
* list too, and either can be holding a blank row from an earlier click.
|
||||
*/
|
||||
const persist = (rows: ProjectPath[]) => {
|
||||
const filled = rows.filter((p) => p.host_path.trim() || p.mount_name.trim());
|
||||
return save({ paths: filled });
|
||||
};
|
||||
|
||||
/**
|
||||
* Save only when every row is fully filled in.
|
||||
*
|
||||
@@ -27,12 +56,18 @@ export default function WorkspaceSection({ project, save, disabled }: Props) {
|
||||
* a half-filled row is refused — so the unconditional save turned an ordinary
|
||||
* keystroke into an error toast. A blank row is *not* incomplete: the
|
||||
* "+ Add folder" button adds one deliberately, and it is dropped on save.
|
||||
*
|
||||
* A blur that changed nothing saves nothing, which is what keeps the blank
|
||||
* row on screen while it is being filled in: persisting the filtered list
|
||||
* would round-trip through `project` and take the empty row away under the
|
||||
* cursor.
|
||||
*/
|
||||
const saveIfComplete = () => {
|
||||
const filled = paths.filter((p) => p.host_path.trim() || p.mount_name.trim());
|
||||
const halfFilled = filled.some((p) => !p.host_path.trim() || !p.mount_name.trim());
|
||||
if (halfFilled) return;
|
||||
return save({ paths });
|
||||
if (sameRows(filled, project.paths ?? [])) return;
|
||||
return persist(paths);
|
||||
};
|
||||
|
||||
return (
|
||||
@@ -106,7 +141,7 @@ export default function WorkspaceSection({ project, save, disabled }: Props) {
|
||||
mount_name: updated[i].mount_name || basename,
|
||||
};
|
||||
setPaths(updated);
|
||||
save({ paths: updated });
|
||||
persist(updated);
|
||||
}
|
||||
}}
|
||||
>
|
||||
@@ -137,7 +172,7 @@ export default function WorkspaceSection({ project, save, disabled }: Props) {
|
||||
onClick={() => {
|
||||
const updated = paths.filter((_, j) => j !== i);
|
||||
setPaths(updated);
|
||||
save({ paths: updated });
|
||||
persist(updated);
|
||||
}}
|
||||
>
|
||||
Remove
|
||||
|
||||
+13
-8
@@ -162,23 +162,28 @@ export interface OpenAiCompatibleConfig {
|
||||
* project that is "inherit the global value", and on the global settings it is
|
||||
* "leave Claude Code's own default alone". `false` is a deliberate off, which
|
||||
* is what lets a project turn a globally-enabled setting back off.
|
||||
*
|
||||
* Every field is optional as well as nullable: the Rust struct skips
|
||||
* serialising a field it has no value for, so an object with nothing set at
|
||||
* this level arrives as `{}`. Absent and `null` mean the same thing, which is
|
||||
* why every read of one of these has to use `== null` rather than `=== null`.
|
||||
*/
|
||||
export interface ClaudeCodeSettings {
|
||||
/** `null` = let Claude Code choose the renderer; `"default"` = classic, `"fullscreen"` = alt-screen. */
|
||||
tui_mode: string | null;
|
||||
tui_mode?: string | null;
|
||||
/** `null` = unset, else `"low" | "medium" | "high" | "xhigh"`. Written as `effortLevel`. */
|
||||
effort: string | null;
|
||||
auto_scroll_disabled: boolean | null;
|
||||
effort?: string | null;
|
||||
auto_scroll_disabled?: boolean | null;
|
||||
/** Written as `viewMode: "focus"`. */
|
||||
focus_mode: boolean | null;
|
||||
show_thinking_summaries: boolean | null;
|
||||
focus_mode?: boolean | null;
|
||||
show_thinking_summaries?: boolean | null;
|
||||
/**
|
||||
* Turns the session recap **off**. Held in the disabled sense because Claude
|
||||
* Code's recap is on by default — see the Rust doc on `ClaudeCodeSettings`.
|
||||
*/
|
||||
session_recap_disabled: boolean | null;
|
||||
env_scrub: boolean | null;
|
||||
prompt_caching_1h: boolean | null;
|
||||
session_recap_disabled?: boolean | null;
|
||||
env_scrub?: boolean | null;
|
||||
prompt_caching_1h?: boolean | null;
|
||||
}
|
||||
|
||||
export interface ContainerInfo {
|
||||
|
||||
Reference in New Issue
Block a user