Skip to content
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion coder/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions coder/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,7 @@ All errors return as JSON objects of the form `{"code":"C2xx","message":"..."}`.
| `C215` | Path is outside every allowed root (lexically or through a symlink); names all roots + recovery |
| `C216` | Underlying I/O error |
| `C217` | `coder::create-file` or `coder::move` saw an existing file with `overwrite=false` |
| `C218` | A per-call `base_dir` was set and the path is inside an allowed root but outside the session directory; names the session dir + recovery (distinct from `C215` so it does not contradict the allowed-roots list) |

## Configuration

Expand Down
16 changes: 14 additions & 2 deletions coder/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,14 @@ pub enum CoderError {
#[error("C217: {0}")]
#[serde(rename = "C217")]
AlreadyExists(String),

/// Path canonicalises inside a configured root but OUTSIDE the
/// per-call `base_dir` the session is scoped to. Distinct from `C215`
/// (outside EVERY root) so the rejection can name the session
/// directory rather than contradict `coder::info`'s allowed-roots list.
#[error("C218: {0}")]
#[serde(rename = "C218")]
OutsideSession(String),
}

impl CoderError {
Expand All @@ -93,6 +101,7 @@ impl CoderError {
CoderError::OutsideBase(_) => "C215",
CoderError::Io(_) => "C216",
CoderError::AlreadyExists(_) => "C217",
CoderError::OutsideSession(_) => "C218",
}
}

Expand All @@ -106,7 +115,8 @@ impl CoderError {
| CoderError::TooLarge(m)
| CoderError::OutsideBase(m)
| CoderError::Io(m)
| CoderError::AlreadyExists(m) => m,
| CoderError::AlreadyExists(m)
| CoderError::OutsideSession(m) => m,
}
}

Expand Down Expand Up @@ -277,10 +287,11 @@ mod tests {
CoderError::OutsideBase("a".into()).code(),
CoderError::Io("a".into()).code(),
CoderError::AlreadyExists("a".into()).code(),
CoderError::OutsideSession("a".into()).code(),
]
.into_iter()
.collect();
assert_eq!(codes.len(), 6);
assert_eq!(codes.len(), 7);
}

/// DRIFT PREVENTION: `to_wire_error()` (structured per-entry form)
Expand All @@ -296,6 +307,7 @@ mod tests {
CoderError::OutsideBase("outside base msg".into()),
CoderError::Io("io msg".into()),
CoderError::AlreadyExists("already exists msg".into()),
CoderError::OutsideSession("outside session msg".into()),
];
for v in &variants {
let wire = v.to_wire_error();
Expand Down
21 changes: 19 additions & 2 deletions coder/src/functions/create_file.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,13 @@ use crate::path::PathResolver;
#[schemars(example = "example_create_file_input")]
pub struct CreateFileInput {
pub files: Vec<CreateFileSpec>,
/// Optional per-call session working directory. When set, relative
/// `path`s anchor here instead of the primary allowed root, and every
/// resolved path must stay inside it. `base_dir` itself must canonicalize
/// inside an allowed root (`coder::info` lists them). Omit to resolve
/// against the primary allowed root exactly as before.
#[serde(default)]
pub base_dir: Option<String>,
}

#[derive(Debug, Deserialize, JsonSchema)]
Expand Down Expand Up @@ -94,23 +101,25 @@ pub async fn handle(
"`files` must not be empty".into(),
)));
}
let base_dir = req.base_dir.as_deref();
let mut results = Vec::with_capacity(req.files.len());
for spec in req.files {
results.push(create_one(&resolver, &cfg, spec));
results.push(create_one(&resolver, &cfg, base_dir, spec));
}
Ok(CreateFileOutput { results })
}

fn create_one(
resolver: &PathResolver,
cfg: &CoderConfig,
base_dir: Option<&str>,
spec: CreateFileSpec,
) -> CreateFileResult {
// Resolve up front: from here on every filesystem operation uses ONLY
// the resolver-returned path (never re-derived from the raw request),
// and the result echoes that canonical absolute path. When resolution
// fails there is no canonical path, so the input is echoed verbatim.
let abs = match resolver.require_writable(&spec.path) {
let abs = match resolver.require_writable_opt(base_dir, &spec.path) {
Ok(abs) => abs,
Err(e) => {
return CreateFileResult {
Expand Down Expand Up @@ -214,6 +223,7 @@ mod tests {
parents: true,
overwrite: false,
}],
base_dir: None,
},
)
.await
Expand Down Expand Up @@ -249,6 +259,7 @@ mod tests {
parents: true,
overwrite: false,
}],
base_dir: None,
},
)
.await
Expand All @@ -272,6 +283,7 @@ mod tests {
parents: true,
overwrite: false,
}],
base_dir: None,
},
)
.await
Expand Down Expand Up @@ -300,6 +312,7 @@ mod tests {
parents: true,
overwrite: true,
}],
base_dir: None,
},
)
.await
Expand All @@ -325,6 +338,7 @@ mod tests {
parents: true,
overwrite: true,
}],
base_dir: None,
},
)
.await
Expand Down Expand Up @@ -353,6 +367,7 @@ mod tests {
parents: true,
overwrite: false,
}],
base_dir: None,
},
)
.await
Expand Down Expand Up @@ -384,6 +399,7 @@ mod tests {
overwrite: false,
},
],
base_dir: None,
},
)
.await
Expand Down Expand Up @@ -412,6 +428,7 @@ mod tests {
parents: true,
overwrite: false,
}],
base_dir: None,
},
)
.await
Expand Down
27 changes: 24 additions & 3 deletions coder/src/functions/delete_file.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,13 @@ pub struct DeleteFileInput {
/// Required for non-empty directories. Files and empty dirs ignore it.
#[serde(default)]
pub recursive: bool,
/// Optional per-call session working directory. When set, relative
/// `paths` anchor here instead of the primary allowed root, and every
/// resolved path must stay inside it. `base_dir` itself must canonicalize
/// inside an allowed root (`coder::info` lists them). Omit to resolve
/// against the primary allowed root exactly as before.
#[serde(default)]
pub base_dir: Option<String>,
}

// examples are wire-contract; goldens pin them.
Expand Down Expand Up @@ -64,19 +71,25 @@ pub async fn handle(
"`paths` must not be empty".into(),
)));
}
let base_dir = req.base_dir.as_deref();
let mut results = Vec::with_capacity(req.paths.len());
for p in req.paths {
results.push(delete_one(&resolver, &p, req.recursive));
results.push(delete_one(&resolver, base_dir, &p, req.recursive));
}
Ok(DeleteFileOutput { results })
}

fn delete_one(resolver: &PathResolver, rel: &str, recursive: bool) -> DeleteFileResult {
fn delete_one(
resolver: &PathResolver,
base_dir: Option<&str>,
rel: &str,
recursive: bool,
) -> DeleteFileResult {
// Resolve up front: deletion operates ONLY on the resolver-returned
// path, and the result echoes that canonical absolute path. When
// resolution fails there is no canonical path, so the caller's input
// is echoed verbatim.
let abs = match resolver.require_writable(rel) {
let abs = match resolver.require_writable_opt(base_dir, rel) {
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved
Ok(abs) => abs,
Err(e) => {
return DeleteFileResult {
Expand Down Expand Up @@ -182,6 +195,7 @@ mod tests {
DeleteFileInput {
paths: vec!["a.txt".into()],
recursive: false,
base_dir: None,
},
)
.await
Expand All @@ -199,6 +213,7 @@ mod tests {
DeleteFileInput {
paths: vec!["nope.txt".into()],
recursive: false,
base_dir: None,
},
)
.await
Expand All @@ -216,6 +231,7 @@ mod tests {
DeleteFileInput {
paths: vec![".env".into()],
recursive: false,
base_dir: None,
},
)
.await
Expand All @@ -235,6 +251,7 @@ mod tests {
DeleteFileInput {
paths: vec!["d".into()],
recursive: false,
base_dir: None,
},
)
.await
Expand All @@ -252,6 +269,7 @@ mod tests {
DeleteFileInput {
paths: vec!["d".into()],
recursive: true,
base_dir: None,
},
)
.await
Expand All @@ -270,6 +288,7 @@ mod tests {
DeleteFileInput {
paths: vec!["d".into()],
recursive: true,
base_dir: None,
},
)
.await
Expand All @@ -293,6 +312,7 @@ mod tests {
DeleteFileInput {
paths: vec!["secrets".into()],
recursive: true,
base_dir: None,
},
)
.await
Expand Down Expand Up @@ -324,6 +344,7 @@ mod tests {
DeleteFileInput {
paths: vec![".".into()],
recursive: true,
base_dir: None,
},
)
.await
Expand Down
14 changes: 13 additions & 1 deletion coder/src/functions/list_folder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,13 @@ pub struct ListFolderInput {
/// `config.list_default_page_size` when omitted.
#[serde(default)]
pub page_size: Option<u32>,
/// Optional per-call session working directory. When set, a relative
/// `path` anchors here instead of the primary allowed root, and the
/// resolved folder must stay inside it. `base_dir` itself must canonicalize
/// inside an allowed root (`coder::info` lists them). Omit to resolve
/// against the primary allowed root exactly as before.
#[serde(default)]
pub base_dir: Option<String>,
}

fn default_path() -> String {
Expand Down Expand Up @@ -99,7 +106,7 @@ fn inner(
// list-folder uses `resolve`, not `require_writable`, so callers can
// list a directory that contains non-accessible *children* even if the
// directory itself happened to match a glob.
let abs = resolver.resolve(&req.path)?;
let abs = resolver.resolve_opt(req.base_dir.as_deref(), &req.path)?;
// NotFound is intercepted with the wire path in scope so the C211
// message names the path the caller supplied (standardized wording —
// REDACTION INVARIANT: identical to the glob-denied message).
Expand Down Expand Up @@ -227,6 +234,7 @@ mod tests {
path: ".".into(),
page: 1,
page_size: None,
base_dir: None,
},
)
.await
Expand Down Expand Up @@ -258,6 +266,7 @@ mod tests {
path: ".".into(),
page: 2,
page_size: Some(2),
base_dir: None,
},
)
.await
Expand Down Expand Up @@ -285,6 +294,7 @@ mod tests {
path: ".".into(),
page: 1,
page_size: Some(9999),
base_dir: None,
},
)
.await
Expand All @@ -304,6 +314,7 @@ mod tests {
path: ".".into(),
page: 1,
page_size: None,
base_dir: None,
},
)
.await
Expand Down Expand Up @@ -333,6 +344,7 @@ mod tests {
path: "a.txt".into(),
page: 1,
page_size: None,
base_dir: None,
},
)
.await
Expand Down
Loading
Loading