Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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 .devflow/features/mds-fmt/KNOWLEDGE.md
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,7 @@ Notable divergences worth knowing before touching this file:
- **Partials are reformatted but `is_partial` still gates output emission elsewhere.** Don't conflate the two meanings of "partial" across `fmt` vs. `build`/`check`.
- **Hidden FILES are still collected by the walker.** `is_default_excluded_dir` excludes hidden *directories* from recursion, not hidden `.mds` files at the traversed level. A `.dotfile.mds` at the root of a traversed directory is collected and formatted.
- **`atomic_write_file` is the write primitive — not `std::fs::write`.** `fmt.rs` calls `atomic_write_file` from `output.rs` (shared with `lint.rs`). It provides a TOCTOU guard, Unix permission preservation (`mode & 0o7777`), and `sync_all()` + atomic rename. This means a failed write leaves the original file intact — it does NOT leave a partially-written file. And, since #227, `write_output`/dir-mode build/`.map` sidecars route through it too, with `Durability::RenameOnly` (fmt/lint pass `Fsync`).
- **`crates/mds-cli/tests/write_funnel.rs` fails CI on any raw `fs::write(` / `File::create(` in `crates/mds-cli/src` outside the two allow-listed sites** (`mds init` in main.rs; the test-only readiness marker in watch.rs).
- **`crates/mds-cli/tests/write_funnel.rs` fails CI on any raw `fs::write(` / `File::create(` in `crates/mds-cli/src` outside the one allow-listed site** (the test-only readiness marker in watch.rs; `mds init` joined the funnel in #386).

## Deferred follow-ups (recorded to avoid re-flagging as new debt)

Expand Down
2 changes: 1 addition & 1 deletion .devflow/features/mds-lint/KNOWLEDGE.md
Original file line number Diff line number Diff line change
Expand Up @@ -549,7 +549,7 @@ LintDiagnostic.fix_removals (FixLineSpan) OR .fix_edits (TextEdit)

**atomic_write_file temp prefix**: The temp file prefix is `.mds-tmp-`. Both lint and fmt share the same `atomic_write_file` from `output.rs`. Shared with `build`/`watch` since #227; third parameter `Durability` (`Fsync` for lint/fmt, `RenameOnly` for build/watch).

**`crates/mds-cli/tests/write_funnel.rs` fails CI on any raw `fs::write(` / `File::create(` in `crates/mds-cli/src`** outside the two allow-listed sites (`mds init` in main.rs; the test-only readiness marker in watch.rs).
**`crates/mds-cli/tests/write_funnel.rs` fails CI on any raw `fs::write(` / `File::create(` in `crates/mds-cli/src`** outside the one allow-listed site (the test-only readiness marker in watch.rs; `mds init` joined the funnel in #386).

**Python `LintDiagnostic.fix_edits` getter vs `#[pyo3(get)]`**: `Vec<serde_json::Value>` does not implement `IntoPy`. Use the custom `#[getter]` which calls `value_to_py`. Stored internally as `Option<Vec<serde_json::Value>>`.

Expand Down
13 changes: 12 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
`cannot get metadata` message that #240 emitted on every first write of a
not-yet-existing file is gone; a stat failure other than "not found" is now a hard
error rather than a warning (#225). A new test, `write_funnel.rs`, fails CI on any raw
`fs::write`/`File::create` in the CLI outside the two justified sites.
`fs::write`/`File::create` in the CLI outside the one justified site (the test-only
readiness marker in `watch.rs`; `mds init` joined the funnel in #386).
- **Fix stale `lint_str` rustdoc and lint-rule Tier tables (#329).** `mds-core`'s
`lint_str` rustdoc said "applies the 9 lint rules" after a 10th rule
(`legacy-interpolation`) had shipped; the Tier tables in `lint/tier.rs` and
Expand All @@ -134,6 +135,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
triggered by template input, only by a defect, and the messages carry no source text.
On the CLI a tripped invariant is a Rust panic (exit code 101); the napi, WASM and
Python bindings convert it to `mds::internal` as before.
- **`mds init` no longer writes through a symlink at the target path (#386).** The
starter file is written by the same replace-by-rename primitive as every other CLI
write (`atomic_write_file`): a symlink at the target — live under `--force`, or
dangling without it — is refused with `cannot write <path>: refusing to replace a
symlink` (exit 1) and neither the link nor its target is touched. Previously a
dangling link was written through (creating its target) and `--force` truncated a
live link's target in place. `--force` on a regular file still replaces it, now
atomically with its permission bits preserved; a new file keeps the umask default
mode. The `init` entry is gone from the `write_funnel.rs` allow-list, so the CLI's
only remaining raw write is the test-only readiness marker in `watch.rs`.
- **`mds lint <dir>` fails closed on a path it cannot name (#217).** The directory-mode
`files[].file` key and the sort key are the entry's path relative to the lint root. A
path that is not valid UTF-8, or that is not under the lint root, previously produced
Expand Down
6 changes: 3 additions & 3 deletions SECURITY.md
Original file line number Diff line number Diff line change
Expand Up @@ -51,9 +51,9 @@ input. The compiler enforces several defense-in-depth controls:
derived from a lossy string. A root that is empty or not valid UTF-8 is treated
as "no root" — entries degrade to basenames — so it can never make the
containment check vacuous.
- **Replace-by-rename writes**: `mds fmt`, `mds lint --fix`, and `mds build`/`mds
watch` outputs and `.map` sidecars are written to a same-directory temp file and
renamed over the target after a final symlink re-check (`mds-cli/src/output.rs`,
- **Replace-by-rename writes**: `mds fmt`, `mds lint --fix`, `mds init`, and `mds
build`/`mds watch` outputs and `.map` sidecars are written to a same-directory
temp file and renamed over the target after a final symlink re-check (`mds-cli/src/output.rs`,
`atomic_write_file`; enforced by `crates/mds-cli/tests/write_funnel.rs`), so a
crash never leaves a truncated target and a symlinked output path is refused.
Consequence: hard links, ACLs, xattrs, and owner/group of a pre-existing target
Expand Down
15 changes: 6 additions & 9 deletions crates/mds-cli/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -418,15 +418,12 @@ Your items:
- {{item}}
@end
";
// Raw std::fs::write is deliberate (#227). What it is NOT justified by: "init only
// ever creates a file". `--force` skips the exists-check above and truncates in
// place, and both `exists()` and `write` follow a symlink, so a link at `filename`
// — dangling, or live under `--force` — is written through to its target. The raw
// write stays because the blast radius is small and entirely user-directed:
// `filename` is typed on the command line and `starter` is a fixed public template
// that a re-run reproduces. Allow-listed in tests/write_funnel.rs.
std::fs::write(&filename, starter)
.map_err(|e| miette::miette!("cannot write {}: {e}", filename.display()))?;
// #386: the same replace-by-rename primitive as every other CLI write — a symlink
// at `filename` (live under `--force`, dangling without it) is refused instead of
// written through; `--force` replaces a regular file by rename with its mode
// preserved. `RenameOnly` because the starter is a fixed public template a re-run
// reproduces.
output::atomic_write_file(&filename, starter, output::Durability::RenameOnly)?;
if !quiet {
eprintln!(
"Created {}\n Try: mds build {}",
Expand Down
165 changes: 165 additions & 0 deletions crates/mds-cli/tests/cli_commands.rs
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,171 @@ fn init_does_not_overwrite_existing_file() {
assert_eq!(content, "original content");
}

/// T-D1-1 (#386): `mds init --force` at a live symlink is refused; the link and its
/// target survive untouched. Positive control in the same test: `mds init` on a plain
/// (non-symlink) path still succeeds normally.
#[cfg(unix)]
#[test]
fn init_force_symlink_target_refused_plain_path_created() {
let dir = tempfile::tempdir().unwrap();
let real = dir.path().join("real.txt");
std::fs::write(&real, "REAL").unwrap();
let link = dir.path().join("link.mds");
std::os::unix::fs::symlink(&real, &link).unwrap();

let refused = mds_bin()
.args(["init", link.to_str().unwrap(), "--force"])
.output()
.unwrap();

let stderr = String::from_utf8_lossy(&refused.stderr);
assert_eq!(
refused.status.code(),
Some(1),
"init --force onto a symlink must exit 1; stderr: {stderr}"
);
assert!(
stderr.contains("refusing to replace a symlink"),
"the refusal must say why; got: {stderr}"
);
assert_eq!(
std::fs::read_to_string(&real).unwrap(),
"REAL",
"the symlink target must not be written through"
);
assert!(
std::fs::symlink_metadata(&link)
.unwrap()
.file_type()
.is_symlink(),
"the symlink itself must survive the refusal"
);

// Positive control: init on a plain (non-symlink) path still succeeds.
let plain = dir.path().join("plain.mds");
let ok = mds_bin()
.args(["init", plain.to_str().unwrap()])
.output()
.unwrap();
assert_eq!(
ok.status.code(),
Some(0),
"init on a plain path must still succeed; stderr: {}",
String::from_utf8_lossy(&ok.stderr)
);
assert!(
std::fs::read_to_string(&plain)
.unwrap()
.contains("Hello {{name}}!"),
"init must write the starter template"
);
}

/// T-D1-2 (#386): `mds init` (no `--force`) at a dangling symlink is refused; the
/// target is never created and the link survives. Positive control in the same test:
/// `mds init` on a plain (non-symlink) path still succeeds normally.
#[cfg(unix)]
#[test]
fn init_dangling_symlink_refused_without_force() {
let dir = tempfile::tempdir().unwrap();
let victim = dir.path().join("victim.txt");
let link = dir.path().join("link.mds");
std::os::unix::fs::symlink(&victim, &link).unwrap();

let refused = mds_bin()
.args(["init", link.to_str().unwrap()])
.output()
.unwrap();

let stderr = String::from_utf8_lossy(&refused.stderr);
assert_eq!(
refused.status.code(),
Some(1),
"init onto a dangling symlink must exit 1; stderr: {stderr}"
);
assert!(
stderr.contains("refusing to replace a symlink"),
"the refusal must say why; got: {stderr}"
);
assert_eq!(
victim.symlink_metadata().unwrap_err().kind(),
std::io::ErrorKind::NotFound,
"the dangling link's target must not be created"
);
assert!(
std::fs::symlink_metadata(&link)
.unwrap()
.file_type()
.is_symlink(),
"the symlink itself must survive the refusal"
);

// Positive control: init on a plain (non-symlink) path still succeeds.
let plain = dir.path().join("plain.mds");
let ok = mds_bin()
.args(["init", plain.to_str().unwrap()])
.output()
.unwrap();
assert_eq!(
ok.status.code(),
Some(0),
"init on a plain path must still succeed; stderr: {}",
String::from_utf8_lossy(&ok.stderr)
);
assert!(
std::fs::read_to_string(&plain)
.unwrap()
.contains("Hello {{name}}!"),
"init must write the starter template"
);
}

/// T-D1-3 (#386, pin): `mds init --force` on a regular file replaces it atomically via
/// rename, preserving its permission bits, and leaves no `.mds-tmp-` temp file behind.
/// Already green on `main` before this PR — this test pins the contract stated in the
/// PR body so a future regression is caught.
#[cfg(unix)]
#[test]
fn init_force_regular_file_replaced_atomically_mode_preserved() {
use std::os::unix::fs::PermissionsExt as _;

let dir = tempfile::tempdir().unwrap();
let target = dir.path().join("keep.mds");
std::fs::write(&target, "original content").unwrap();
std::fs::set_permissions(&target, std::fs::Permissions::from_mode(0o600)).unwrap();

let output = mds_bin()
.args(["init", target.to_str().unwrap(), "--force"])
.output()
.unwrap();

assert_eq!(
output.status.code(),
Some(0),
"init --force on a regular file must succeed; stderr: {}",
String::from_utf8_lossy(&output.stderr)
);
let content = std::fs::read_to_string(&target).unwrap();
assert!(
content.contains("Hello {{name}}!"),
"the overwritten file must contain the starter template; got: {content:?}"
);
let mode = std::fs::metadata(&target).unwrap().permissions().mode();
assert_eq!(
mode & 0o777,
0o600,
"the replaced file must preserve the original permission bits"
);
let leftover_tmp = std::fs::read_dir(dir.path())
.unwrap()
.filter_map(|e| e.ok())
.any(|e| e.file_name().to_string_lossy().starts_with(".mds-tmp-"));
assert!(
!leftover_tmp,
"no .mds-tmp- temp file should remain in the directory"
);
}

#[test]
fn set_flag_cli_overrides() {
// --set name=Test should override the frontmatter variable 'name'
Expand Down
22 changes: 6 additions & 16 deletions crates/mds-cli/tests/write_funnel.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,22 +40,12 @@ const NEEDLES: &[&str] = &["fs::write(", "File::create("];
/// reported as dead by [`write_sites_are_funnelled`] and by
/// [`every_allowlist_entry_is_live`], so a removed site cannot leave a stale licence
/// behind for a future raw write to hide under.
const ALLOWED_RAW_WRITES: &[(&str, &str, usize, &str)] = &[
(
"main.rs",
"fs::write(",
1,
"mds init scaffolds a fixed public template at a user-typed path, reproducible by \
a re-run — NOT because it only ever creates: --force truncates in place and the \
write follows a symlink at the target (#227)",
),
(
"watch.rs",
"fs::write(",
1,
"test-only readiness marker: written to <path>.tmp then renamed — already atomic",
),
];
const ALLOWED_RAW_WRITES: &[(&str, &str, usize, &str)] = &[(
"watch.rs",
"fs::write(",
1,
"test-only readiness marker: written to <path>.tmp then renamed — already atomic",
)];

#[test]
fn write_sites_are_funnelled() {
Expand Down
4 changes: 2 additions & 2 deletions spec.md
Original file line number Diff line number Diff line change
Expand Up @@ -926,7 +926,7 @@ mds build src/ --out-dir dist # Mirror subtree: src/a/b.mds → dis

In all paths, `<ext>` is `md` for Markdown templates and `json` for messages templates.

**Output writing.** Compiled outputs and `.map` sidecars written by `mds build` and `mds watch`, and `.mds` sources rewritten by `mds fmt` and `mds lint --fix`, are written to a temporary file in the target's directory and then renamed over the target, after a final symlink re-check of the target: a crash, kill or full disk never leaves a truncated file behind, and a destination path that is a symlink is refused. Because the output is created as a sibling temporary file and renamed into place, the destination must be a regular-file path inside a writable directory: device files such as `/dev/null` and FIFOs are not supported as `-o` targets (write to stdout instead). Source rewrites (`fmt`, `lint --fix`) are additionally fsynced before the rename; compiled outputs and sidecars rely on the rename alone — they are regenerable, and an unconditional fsync made directory-mode startup several times slower on macOS. Because the rename gives the target a new inode, a pre-existing target's hard links (other links keep the old content), ACLs, extended attributes, and owner/group are not preserved; permission bits are preserved on Unix. This is enforced for the CLI's write sites by `crates/mds-cli/tests/write_funnel.rs`.
**Output writing.** Compiled outputs and `.map` sidecars written by `mds build` and `mds watch`, `.mds` sources rewritten by `mds fmt` and `mds lint --fix`, and the starter file written by `mds init`, are written to a temporary file in the target's directory and then renamed over the target, after a final symlink re-check of the target: a crash, kill or full disk never leaves a truncated file behind, and a destination path that is a symlink is refused. Because the output is created as a sibling temporary file and renamed into place, the destination must be a regular-file path inside a writable directory: device files such as `/dev/null` and FIFOs are not supported as `-o` targets (write to stdout instead). Source rewrites (`fmt`, `lint --fix`) are additionally fsynced before the rename; compiled outputs and sidecars rely on the rename alone — they are regenerable, and an unconditional fsync made directory-mode startup several times slower on macOS. Because the rename gives the target a new inode, a pre-existing target's hard links (other links keep the old content), ACLs, extended attributes, and owner/group are not preserved; permission bits are preserved on Unix. This is enforced for the CLI's write sites by `crates/mds-cli/tests/write_funnel.rs`.

### 7.3 `mds check`

Expand Down Expand Up @@ -1274,7 +1274,7 @@ mds init my-prompt.mds # Creates my-prompt.mds
mds init my-prompt.mds --force # Overwrite if file already exists
```

Creates a compilable starter template. Path traversal (e.g. `../escaped.mds`) is rejected.
Creates a compilable starter template. Path traversal (e.g. `../escaped.mds`) is rejected. The file is written through the replace-by-rename primitive of §7.2 "Output writing": a symlink at the target — live or dangling — is refused (exit 1, `cannot write <path>: refusing to replace a symlink`) rather than written through; `--force` replaces a regular file atomically, preserving its permission bits.

### 7.7 Auto-Detection

Expand Down
Loading