diff --git a/docs/algorithm/index.md b/docs/algorithm/index.md index 1cf0373..971e9d6 100644 --- a/docs/algorithm/index.md +++ b/docs/algorithm/index.md @@ -122,7 +122,7 @@ file_type = "file" * perms * owner * `state`: entry for this file in the state-file (if it exists). This represents the values of the target file - at the time of the last successful sync (which is the same as the values for `source` at that time. + at the time of the last successful sync (which is the same as the values for `source` at that time). * mtime * hash * type (file, symlink) @@ -202,8 +202,12 @@ Verify that the actions can be executed * Check that the state file can be written * Check that the source file can be written or created. * Check that missing parent directories can be created. - * Check that the correct file owner can be set. - (i.e., either `cfgsync` is running as root or as the intended owner of the **config file**) + * Check that the owner of the target file matches the owner configuration for that file. + * Check that the permissions of the target file are a valid value from the target column of + the [permission mapping](./permissions-and-owner.md#permissions) for the configured permissions + (i.e., 600 or 700 if permissions are set to "private"; 644 or 755 if permissions are set to "public" or are + omitted) + * Check that either `cfgsync` is running as root or as the owner of the **config file** * `DeleteSource`: * Check that the state file can be written * Check the source file can be deleted @@ -217,7 +221,8 @@ Verify that the actions can be executed * `Conflict`: * Check if any of `CopyToTarget` or `CopyToSource` are possible. -All failed checks are immediately logged as warnings. For the next steps, a file is only considered "valid" if EVERY check for that file passes. +All failed checks are immediately logged as warnings. For the next steps, a file is only considered "valid" if EVERY +check for that file passes. # 5. Execute changes diff --git a/e2e-tests/checked/1-config-edge-cases/absolute-target.test.ts b/e2e-tests/checked/1-config-edge-cases/absolute-target.test.ts index 2103cd5..f4cfee3 100644 --- a/e2e-tests/checked/1-config-edge-cases/absolute-target.test.ts +++ b/e2e-tests/checked/1-config-edge-cases/absolute-target.test.ts @@ -18,7 +18,18 @@ Deno.test("copy data to absolute target path", async (t) => { ], })); - await testbed.run({ args: ["--config", "config.toml", "sync"] }); + await testbed.testSync("config.toml", { + code: 0, + stdout: deindent` + copied .subdir/subsub/new.txt -> target + + source -> target: 1 + target -> source: 0 + deleted target: 0 + deleted source: 0 + `, + stderr: "", + }); await testbed.assertTestDir([ `user:user | 644 | 0 | config.cfgsync.state | ${STATE_FILE}`, @@ -32,16 +43,4 @@ Deno.test("copy data to absolute target path", async (t) => { "user:user | 755 | 0 | target/.subdir/subsub/", "user:user | 644 | 0 | target/.subdir/subsub/new.txt | newfile", ]); - testbed.assertOutput({ - code: 0, - stdout: deindent` - copied .subdir/subsub/new.txt -> target - - source -> target: 1 - target -> source: 0 - deleted target: 0 - deleted source: 0 - `, - stderr: "", - }); }); diff --git a/e2e-tests/checked/3-classifications/0-glob-matching/ignore-non-matching.test.ts b/e2e-tests/checked/3-classifications/0-glob-matching/ignore-non-matching.test.ts index a914162..e843673 100644 --- a/e2e-tests/checked/3-classifications/0-glob-matching/ignore-non-matching.test.ts +++ b/e2e-tests/checked/3-classifications/0-glob-matching/ignore-non-matching.test.ts @@ -19,7 +19,19 @@ Deno.test("ignore-non-matching", async (t) => { ], }); - await testbed.run({ args: ["--config", "config.toml", "sync"] }); + await testbed.testSync("config.toml", { + code: 0, + stdout: deindent` + copied file.txt -> target + copied target -> unmatched-target.txt + + source -> target: 1 + target -> source: 1 + deleted target: 0 + deleted source: 0 + `, + stderr: "", + }); await testbed.assertTestDir([ `user:user | 644 | 0 | config.cfgsync.state | ${STATE_FILE}`, @@ -33,17 +45,4 @@ Deno.test("ignore-non-matching", async (t) => { "user:user | 644 | 0 | target/not-matched.conf | should not be synced", "user:user | 644 | 0 | target/unmatched-target.txt | target file content", ]); - testbed.assertOutput({ - code: 0, - stdout: deindent` - copied file.txt -> target - copied target -> unmatched-target.txt - - source -> target: 1 - target -> source: 1 - deleted target: 0 - deleted source: 0 - `, - stderr: "", - }); }); diff --git a/e2e-tests/checked/3-classifications/0-glob-matching/multi-group-independent.test.ts b/e2e-tests/checked/3-classifications/0-glob-matching/multi-group-independent.test.ts index 8105cd1..0547bcf 100644 --- a/e2e-tests/checked/3-classifications/0-glob-matching/multi-group-independent.test.ts +++ b/e2e-tests/checked/3-classifications/0-glob-matching/multi-group-independent.test.ts @@ -26,52 +26,38 @@ Deno.test("multi-group-independent", async (t) => { }); // Status and diff - await testbed.run({ args: ["--config", "config.toml", "status", "--short"] }); - testbed.assertOutput({ - code: 0, - stdout: deindent` + await testbed.testStatus("config.toml", { + short: deindent` 2→ `, - stderr: "", - }); - - await testbed.run({ args: ["--config", "config.toml", "status"] }); - testbed.assertOutput({ - code: 0, - stdout: deindent` + normal: deindent` source -> target: 2 target -> source: 0 `, - stderr: "", }); - await testbed.run({ args: ["--config", "config.toml", "diff"] }); - testbed.assertOutput({ - code: 0, - stdout: deindent` - === file.txt (source -> target) === - --- ${testDir}/source-a/file.txt${"\t"}2020-01-01 00:00:00.000000000 +0000 - +++ ${testDir}/target-a/file.txt${"\t"} - @@ -1 +1 @@ - -content from group a - \ No newline at end of file - +(file missing) - \ No newline at end of file - - === file.conf (source -> target) === - --- ${testDir}/source-b/file.conf${"\t"}2020-01-01 00:00:00.000000000 +0000 - +++ ${testDir}/target-b/file.conf${"\t"} - @@ -1 +1 @@ - -content from group b - \ No newline at end of file - +(file missing) - \ No newline at end of file`, - stderr: "", - }); + await testbed.testDiff("config.toml", deindent` + === file.txt (source -> target) === + --- ${testDir}/source-a/file.txt${"\t"}2020-01-01 00:00:00.000000000 +0000 + +++ ${testDir}/target-a/file.txt${"\t"} + @@ -1 +1 @@ + -content from group a + \ No newline at end of file + +(file missing) + \ No newline at end of file + + === file.conf (source -> target) === + --- ${testDir}/source-b/file.conf${"\t"}2020-01-01 00:00:00.000000000 +0000 + +++ ${testDir}/target-b/file.conf${"\t"} + @@ -1 +1 @@ + -content from group b + \ No newline at end of file + +(file missing) + \ No newline at end of file`, + ); // First sync: both groups copy to target - await testbed.run({ args: ["--config", "config.toml", "sync"] }); - testbed.assertOutput({ + await testbed.testSync("config.toml", { code: 0, stdout: deindent` copied file.txt -> target diff --git a/e2e-tests/checked/3-classifications/0-glob-matching/relative-paths.test.ts b/e2e-tests/checked/3-classifications/0-glob-matching/relative-paths.test.ts index fdf088f..06d9060 100644 --- a/e2e-tests/checked/3-classifications/0-glob-matching/relative-paths.test.ts +++ b/e2e-tests/checked/3-classifications/0-glob-matching/relative-paths.test.ts @@ -19,46 +19,31 @@ Deno.test("relative-paths", async (t) => { }); // Status shows pending copy before sync - await testbed.run({ args: ["--config", "subdir/config.toml", "status"] }); - testbed.assertOutput({ - code: 0, - stdout: deindent` + await testbed.testStatus("subdir/config.toml", { + short: deindent` + 1→ + `, + normal: deindent` source -> target: 1 target -> source: 0 `, - stderr: "", - }); - - await testbed.run({ args: ["--config", "subdir/config.toml", "status", "--short"] }); - testbed.assertOutput({ - code: 0, - stdout: deindent` - 1→ - `, - stderr: "", }); // Diff shows pending change with relative paths resolved absolutely. // The target file doesn't exist yet, so its mtime is empty (trailing tab). - await testbed.run({ args: ["--config", "subdir/config.toml", "diff"] }); - testbed.assertOutput({ - code: 0, - stdout: deindent` - === same.txt (source -> target) === - --- ${testDir}/subdir/source/same.txt${"\t"}2020-01-01 00:00:00.000000000 +0000 - +++ ${testDir}/target/same.txt - @@ -1 +1 @@ - -identical content - \ No newline at end of file - +(file missing) - \ No newline at end of file - `, - stderr: "", - }); + await testbed.testDiff("subdir/config.toml", deindent` + === same.txt (source -> target) === + --- ${testDir}/subdir/source/same.txt${"\t"}2020-01-01 00:00:00.000000000 +0000 + +++ ${testDir}/target/same.txt + @@ -1 +1 @@ + -identical content + \ No newline at end of file + +(file missing) + \ No newline at end of file + `); // Sync copies the file - await testbed.run({ args: ["--config", "subdir/config.toml", "sync"] }); - testbed.assertOutput({ + await testbed.testSync("subdir/config.toml", { code: 0, stdout: deindent` copied same.txt -> target diff --git a/e2e-tests/checked/3-classifications/0-glob-matching/resolve-tilde.test.ts b/e2e-tests/checked/3-classifications/0-glob-matching/resolve-tilde.test.ts index e3434cc..c0a3218 100644 --- a/e2e-tests/checked/3-classifications/0-glob-matching/resolve-tilde.test.ts +++ b/e2e-tests/checked/3-classifications/0-glob-matching/resolve-tilde.test.ts @@ -38,53 +38,37 @@ Deno.test({ name: "resolve tilde in target path", ignore: runningOutsideDocker } }); // Status shows both copies before sync - await testbed.run({ args: ["--config", "config.toml", "status", "--short"] }); - testbed.assertOutput({ - code: 0, - stdout: deindent` + await testbed.testStatus("config.toml", { + short: deindent` 1→ 1← `, - stderr: "", - }); - - await testbed.run({ args: ["--config", "config.toml", "status"] }); - testbed.assertOutput({ - code: 0, - stdout: deindent` + normal: deindent` source -> target: 1 target -> source: 1 `, - stderr: "", }); // Diff with tilde-expanded target paths - await testbed.run({ args: ["--config", "config.toml", "diff"] }); - testbed.assertOutput({ - code: 0, - stdout: deindent` - === cfgsync-test-subdir/data-source.txt (source -> target) === - --- ${testDir}/source/cfgsync-test-subdir/data-source.txt${"\t"}2020-01-01 00:00:00.000000000 +0000 - +++ /home/user/cfgsync-test-subdir/data-source.txt - @@ -1 +1 @@ - -My data - \ No newline at end of file - +(file missing) - \ No newline at end of file - === cfgsync-test-subdir/subdir/data-home.txt (target -> source) === - --- /home/user/cfgsync-test-subdir/subdir/data-home.txt${"\t"}2020-01-01 00:00:00.000000000 +0000 - +++ ${testDir}/source/cfgsync-test-subdir/subdir/data-home.txt - @@ -1 +1 @@ - -My data - \ No newline at end of file - +(file missing) - \ No newline at end of file - `, - stderr: "", - }); - - await testbed.run({ args: ["--config", "config.toml", "sync"] }); + await testbed.testDiff("config.toml", deindent` + === cfgsync-test-subdir/data-source.txt (source -> target) === + --- ${testDir}/source/cfgsync-test-subdir/data-source.txt${"\t"}2020-01-01 00:00:00.000000000 +0000 + +++ /home/user/cfgsync-test-subdir/data-source.txt + @@ -1 +1 @@ + -My data + \ No newline at end of file + +(file missing) + \ No newline at end of file + === cfgsync-test-subdir/subdir/data-home.txt (target -> source) === + --- /home/user/cfgsync-test-subdir/subdir/data-home.txt${"\t"}2020-01-01 00:00:00.000000000 +0000 + +++ ${testDir}/source/cfgsync-test-subdir/subdir/data-home.txt + @@ -1 +1 @@ + -My data + \ No newline at end of file + +(file missing) + \ No newline at end of file + `); - testbed.assertOutput({ + await testbed.testSync("config.toml", { code: 0, stdout: deindent` copied cfgsync-test-subdir/data-source.txt -> target diff --git a/e2e-tests/checked/3-classifications/1-no-state-file/files-equal.test.ts b/e2e-tests/checked/3-classifications/1-no-state-file/files-equal.test.ts index dd2b974..f18b291 100644 --- a/e2e-tests/checked/3-classifications/1-no-state-file/files-equal.test.ts +++ b/e2e-tests/checked/3-classifications/1-no-state-file/files-equal.test.ts @@ -18,40 +18,24 @@ Deno.test("new-file-update-state", async (t) => { }); // Test short status - await testbed.run({ args: ["--config", "config.toml", "status", "--short"] }); - testbed.assertOutput({ - code: 0, - stdout: deindent` + await testbed.testStatus("config.toml", { + short: deindent` ↺1 `, - stderr: "", - }); - - // status: both sides have same content and no state entry → UpdateState - await testbed.run({ args: ["--config", "config.toml", "status"] }); - testbed.assertOutput({ - code: 0, - stdout: deindent` + normal: deindent` source -> target: 0 target -> source: 0 state update: 1 `, - stderr: "", }); // TODO: Test short status (new icon to show that everything is fine, but sync should be run anyway) // diff: UpdateState produces no output - await testbed.run({ args: ["--config", "config.toml", "diff"] }); - testbed.assertOutput({ - code: 0, - stdout: "", - stderr: "", - }); + await testbed.testDiff("config.toml", ""); // sync: no files copied, state file created with hash for tracking - await testbed.run({ args: ["--config", "config.toml", "sync"] }); - testbed.assertOutput({ + await testbed.testSync("config.toml", { code: 0, stdout: deindent` source -> target: 0 diff --git a/e2e-tests/checked/3-classifications/1-no-state-file/sync-symlink-forward.test.ts b/e2e-tests/checked/3-classifications/1-no-state-file/sync-symlink-forward.test.ts index e19ecba..a1f82e5 100644 --- a/e2e-tests/checked/3-classifications/1-no-state-file/sync-symlink-forward.test.ts +++ b/e2e-tests/checked/3-classifications/1-no-state-file/sync-symlink-forward.test.ts @@ -16,8 +16,7 @@ Deno.test("sync-symlink-forward", async (t) => { ], }); - await testbed.run({ args: ["--config", "config.toml", "sync"] }); - testbed.assertOutput({ + await testbed.testSync("config.toml", { code: 0, stdout: deindent` copied link.txt -> target diff --git a/e2e-tests/checked/3-classifications/3-changed-files/both-files-equal-different-mtime.test.ts b/e2e-tests/checked/3-classifications/3-changed-files/both-files-equal-different-mtime.test.ts index acd6662..193155f 100644 --- a/e2e-tests/checked/3-classifications/3-changed-files/both-files-equal-different-mtime.test.ts +++ b/e2e-tests/checked/3-classifications/3-changed-files/both-files-equal-different-mtime.test.ts @@ -25,38 +25,22 @@ Deno.test("both-exist-same-content-different-mtime", async (t) => { await testbed.writeTextFile("target/file.txt", "bye"); // Test status - await testbed.run({ args: ["--config", "config.toml", "status"] }); - testbed.assertOutput({ - code: 0, - stdout: deindent` + await testbed.testStatus("config.toml", { + short: deindent` + ↺1 + `, + normal: deindent` source -> target: 0 target -> source: 0 state update: 1 `, - stderr: "", - }); - - // Test short status - await testbed.run({ args: ["--config", "config.toml", "status", "--short"] }); - testbed.assertOutput({ - code: 0, - stdout: deindent` - ↺1 - `, - stderr: "", }); // Test diff (no changes = empty output) - await testbed.run({ args: ["--config", "config.toml", "diff"] }); - testbed.assertOutput({ - code: 0, - stdout: "", - stderr: "", - }); + await testbed.testDiff("config.toml", ""); // Test sync - await testbed.run({ args: ["--config", "config.toml", "sync"] }); - testbed.assertOutput({ + await testbed.testSync("config.toml", { code: 0, stdout: deindent` source -> target: 0 diff --git a/e2e-tests/checked/3-classifications/3-changed-files/both-files-equal.test.ts b/e2e-tests/checked/3-classifications/3-changed-files/both-files-equal.test.ts index 4a63627..3c6628d 100644 --- a/e2e-tests/checked/3-classifications/3-changed-files/both-files-equal.test.ts +++ b/e2e-tests/checked/3-classifications/3-changed-files/both-files-equal.test.ts @@ -19,37 +19,21 @@ Deno.test("both-exist-clean", async (t) => { }); // Test status - await testbed.run({ args: ["--config", "config.toml", "status"] }); - testbed.assertOutput({ - code: 0, - stdout: deindent` + await testbed.testStatus("config.toml", { + short: deindent` + ✓ + `, + normal: deindent` source -> target: 0 target -> source: 0 `, - stderr: "", - }); - - // Test short status - await testbed.run({ args: ["--config", "config.toml", "status", "--short"] }); - testbed.assertOutput({ - code: 0, - stdout: deindent` - ✓ - `, - stderr: "", }); // Test diff (no changes = empty output) - await testbed.run({ args: ["--config", "config.toml", "diff"] }); - testbed.assertOutput({ - code: 0, - stdout: "", - stderr: "", - }); + await testbed.testDiff("config.toml", ""); // Test sync - await testbed.run({ args: ["--config", "config.toml", "sync"] }); - testbed.assertOutput({ + await testbed.testSync("config.toml", { code: 0, stdout: deindent` source -> target: 0 diff --git a/e2e-tests/checked/3-classifications/3-changed-files/both-owner-changed-same.test.ts b/e2e-tests/checked/3-classifications/3-changed-files/both-owner-changed-same.test.ts new file mode 100644 index 0000000..efa5964 --- /dev/null +++ b/e2e-tests/checked/3-classifications/3-changed-files/both-owner-changed-same.test.ts @@ -0,0 +1,66 @@ +import { + CONFIG_TOML, + deindent, + rootOwner, + runningOutsideDocker, + STATE_FILE, + TestBed, +} from "@/lib/index.ts"; + +Deno.test({ + name: "both-owner-changed-same-update-state", + ignore: runningOutsideDocker, +}, async (t) => { + const { testbed } = await TestBed.create(t, { + configToml: deindent` + [[sync]] + source = "./source" + target = "./target" + globs = ["**/*.txt"] + `, + files: [ + `user:user | 644 | 0 | config.toml | ${CONFIG_TOML}`, + `user:user | 644 | 0 | config.cfgsync.state | ${STATE_FILE}`, + "user:user | 755 | 0 | source/", + "user:user | 644 | 0 | source/file.txt | hello", + "user:user | 755 | 0 | target/", + "user:user | 644 | 0 | target/file.txt | hello", + ], + faketime: "2020-01-01T00:00:00Z", + }); + + testbed.advance("1 sec"); + await testbed.chown("source/file.txt", rootOwner); + await testbed.chown("target/file.txt", rootOwner); + + await testbed.testStatus("config.toml", { + short: deindent` + ↺1 + `, + normal: deindent` + source -> target: 0 + target -> source: 0 + state update: 1 + `, + }); + + await testbed.testSync("config.toml", { + code: 0, + stdout: deindent` + source -> target: 0 + target -> source: 0 + deleted target: 0 + deleted source: 0 + `, + stderr: "", + }); + + await testbed.assertTestDir([ + `user:user | 644 | 0 | config.cfgsync.state | ${STATE_FILE}`, + `user:user | 644 | 0 | config.toml | ${CONFIG_TOML}`, + "user:user | 755 | 0 | source/", + `root:root | 644 | 0 | source/file.txt | hello`, + "user:user | 755 | 0 | target/", + `root:root | 644 | 0 | target/file.txt | hello`, + ]); +}); diff --git a/e2e-tests/checked/3-classifications/3-changed-files/both-perms-changed-same.test.ts b/e2e-tests/checked/3-classifications/3-changed-files/both-perms-changed-same.test.ts new file mode 100644 index 0000000..5dc4b60 --- /dev/null +++ b/e2e-tests/checked/3-classifications/3-changed-files/both-perms-changed-same.test.ts @@ -0,0 +1,58 @@ +import { CONFIG_TOML, deindent, STATE_FILE, TestBed } from "@/lib/index.ts"; + +Deno.test("both-perms-changed-same-update-state", async (t) => { + const { testbed } = await TestBed.create(t, { + configToml: deindent` + [[sync]] + source = "./source" + target = "./target" + globs = ["**/*.txt"] + `, + files: [ + `user:user | 644 | 0 | config.toml | ${CONFIG_TOML}`, + `user:user | 644 | 0 | config.cfgsync.state | ${STATE_FILE}`, + "user:user | 755 | 0 | source/", + "user:user | 644 | 0 | source/file.txt | hello", + "user:user | 755 | 0 | target/", + "user:user | 644 | 0 | target/file.txt | hello", + ], + faketime: "2020-01-01T00:00:00Z", + }); + + testbed.advance("1 sec"); + await testbed.chmod("source/file.txt", 0o600); + await testbed.chmod("target/file.txt", 0o600); + + await testbed.testStatus("config.toml", { + short: deindent` + ↺1 + `, + normal: deindent` + source -> target: 0 + target -> source: 0 + state update: 1 + `, + }); + + await testbed.testDiff("config.toml", ""); + + await testbed.testSync("config.toml", { + code: 0, + stdout: deindent` + source -> target: 0 + target -> source: 0 + deleted target: 0 + deleted source: 0 + `, + stderr: "", + }); + + await testbed.assertTestDir([ + `user:user | 644 | 0 | config.cfgsync.state | ${STATE_FILE}`, + `user:user | 644 | 0 | config.toml | ${CONFIG_TOML}`, + "user:user | 755 | 0 | source/", + "user:user | 600 | 0 | source/file.txt | hello", + "user:user | 755 | 0 | target/", + "user:user | 600 | 0 | target/file.txt | hello", + ]); +}); diff --git a/e2e-tests/checked/3-classifications/3-changed-files/source-file-changed.test.ts b/e2e-tests/checked/3-classifications/3-changed-files/source-file-changed.test.ts index adf4ba0..4dc7646 100644 --- a/e2e-tests/checked/3-classifications/3-changed-files/source-file-changed.test.ts +++ b/e2e-tests/checked/3-classifications/3-changed-files/source-file-changed.test.ts @@ -22,43 +22,28 @@ Deno.test("both-exist-copy-to-target", async (t) => { testbed.advance("1 sec"); await testbed.writeTextFile("source/file.txt", "v2"); - await testbed.run({ args: ["--config", "config.toml", "status"] }); - testbed.assertOutput({ - code: 0, - stdout: deindent` + await testbed.testStatus("config.toml", { + short: deindent` + 1→ + `, + normal: deindent` source -> target: 1 target -> source: 0 `, - stderr: "", }); - await testbed.run({ args: ["--config", "config.toml", "status", "--short"] }); - testbed.assertOutput({ - code: 0, - stdout: deindent` - 1→ - `, - stderr: "", - }); - - await testbed.run({ args: ["--config", "config.toml", "diff"] }); - testbed.assertOutput({ - code: 0, - stderr: "", - stdout: deindent` - === file.txt (source -> target) === - --- ${testDir}/source/file.txt${"\t"}2020-01-01 00:00:01.000000000 +0000 - +++ ${testDir}/target/file.txt${"\t"}2020-01-01 00:00:00.000000000 +0000 - @@ -1 +1 @@ - -v2 - \ No newline at end of file - +v1 - \ No newline at end of file - `, - }); + await testbed.testDiff("config.toml", deindent` + === file.txt (source -> target) === + --- ${testDir}/source/file.txt${"\t"}2020-01-01 00:00:01.000000000 +0000 + +++ ${testDir}/target/file.txt${"\t"}2020-01-01 00:00:00.000000000 +0000 + @@ -1 +1 @@ + -v2 + \ No newline at end of file + +v1 + \ No newline at end of file + `); - await testbed.run({ args: ["--config", "config.toml", "sync"] }); - testbed.assertOutput({ + await testbed.testSync("config.toml", { code: 0, stdout: deindent` copied file.txt -> target diff --git a/e2e-tests/checked/3-classifications/3-changed-files/source-owner-changed.test.ts b/e2e-tests/checked/3-classifications/3-changed-files/source-owner-changed.test.ts new file mode 100644 index 0000000..c7adb5a --- /dev/null +++ b/e2e-tests/checked/3-classifications/3-changed-files/source-owner-changed.test.ts @@ -0,0 +1,74 @@ +import { + CONFIG_TOML, + deindent, + rootOwner, + runningOutsideDocker, + STATE_FILE, + TestBed, +} from "@/lib/index.ts"; + +Deno.test({ + name: "source-owner-changed-copy-to-target", + ignore: runningOutsideDocker, +}, async (t) => { + const { testbed } = await TestBed.create(t, { + configToml: deindent` + [[sync]] + source = "./source" + target = "./target" + globs = ["**/*.txt"] + `, + files: [ + `user:user | 644 | 0 | config.toml | ${CONFIG_TOML}`, + `user:user | 644 | 0 | config.cfgsync.state | ${STATE_FILE}`, + "user:user | 755 | 0 | source/", + "user:user | 644 | 0 | source/file.txt | hello", + "user:user | 755 | 0 | target/", + "user:user | 644 | 0 | target/file.txt | hello", + ], + faketime: "2020-01-01T00:00:00Z", + }); + + testbed.advance("1 sec"); + await testbed.updateConfig(deindent` + [[sync]] + source = "./source" + target = "./target" + owner = "${rootOwner}" + globs = ["**/*.txt"] + `); + await testbed.writeTextFile("source/file.txt", "v2"); + + await testbed.run({ args: ["--config", "config.toml", "status"] }); + testbed.assertOutput({ + code: 0, + stdout: deindent` + source -> target: 1 + target -> source: 0 + `, + stderr: "", + }); + + await testbed.run({ args: ["--config", "config.toml", "sync"], sudo: true }); + testbed.assertOutput({ + code: 0, + stdout: deindent` + copied file.txt -> target + + source -> target: 1 + target -> source: 0 + deleted target: 0 + deleted source: 0 + `, + stderr: "", + }); + + await testbed.assertTestDir([ + `user:user | 644 | 0 | config.cfgsync.state | ${STATE_FILE}`, + `user:user | 644 | 0 | config.toml | ${CONFIG_TOML}`, + "user:user | 755 | 0 | source/", + "user:user | 644 | 0 | source/file.txt | v2", + "user:user | 755 | 0 | target/", + `root:root | 644 | 0 | target/file.txt | v2`, + ]); +}); diff --git a/e2e-tests/checked/3-classifications/3-changed-files/source-perms-changed.test.ts b/e2e-tests/checked/3-classifications/3-changed-files/source-perms-changed.test.ts new file mode 100644 index 0000000..626d1ac --- /dev/null +++ b/e2e-tests/checked/3-classifications/3-changed-files/source-perms-changed.test.ts @@ -0,0 +1,63 @@ +import { CONFIG_TOML, deindent, STATE_FILE, TestBed } from "@/lib/index.ts"; + +Deno.test("source-perms-changed-copy-to-target", async (t) => { + const { testbed } = await TestBed.create(t, { + configToml: deindent` + [[sync]] + source = "./source" + target = "./target" + globs = ["**/*.txt"] + `, + files: [ + `user:user | 644 | 0 | config.toml | ${CONFIG_TOML}`, + `user:user | 644 | 0 | config.cfgsync.state | ${STATE_FILE}`, + "user:user | 755 | 0 | source/", + "user:user | 644 | 0 | source/file.txt | hello", + "user:user | 755 | 0 | target/", + "user:user | 644 | 0 | target/file.txt | hello", + ], + faketime: "2020-01-01T00:00:00Z", + }); + + testbed.advance("1 sec"); + await testbed.updateConfig(deindent` + [[sync]] + source = "./source" + target = "./target" + file_perms = "private" + globs = ["**/*.txt"] + `); + + await testbed.run({ args: ["--config", "config.toml", "status"] }); + testbed.assertOutput({ + code: 0, + stdout: deindent` + source -> target: 1 + target -> source: 0 + `, + stderr: "", + }); + + await testbed.run({ args: ["--config", "config.toml", "sync"] }); + testbed.assertOutput({ + code: 0, + stdout: deindent` + copied file.txt -> target + + source -> target: 1 + target -> source: 0 + deleted target: 0 + deleted source: 0 + `, + stderr: "", + }); + + await testbed.assertTestDir([ + `user:user | 644 | 0 | config.cfgsync.state | ${STATE_FILE}`, + `user:user | 644 | 0 | config.toml | ${CONFIG_TOML}`, + "user:user | 755 | 0 | source/", + "user:user | 644 | 0 | source/file.txt | hello", + "user:user | 755 | 0 | target/", + "user:user | 600 | 0 | target/file.txt | hello", + ]); +}); diff --git a/e2e-tests/checked/3-classifications/3-changed-files/source-perms-valid.test.ts b/e2e-tests/checked/3-classifications/3-changed-files/source-perms-valid.test.ts new file mode 100644 index 0000000..e391815 --- /dev/null +++ b/e2e-tests/checked/3-classifications/3-changed-files/source-perms-valid.test.ts @@ -0,0 +1,62 @@ +import { CONFIG_TOML, deindent, STATE_FILE, TestBed } from "@/lib/index.ts"; + +Deno.test("source-perms-valid-copy-to-target", async (t) => { + const { testbed, testDir } = await TestBed.create(t, { + configToml: deindent` + [[sync]] + source = "./source" + target = "./target" + globs = ["**/*.txt"] + `, + files: [ + `user:user | 644 | 0 | config.toml | ${CONFIG_TOML}`, + `user:user | 644 | 0 | config.cfgsync.state | ${STATE_FILE}`, + "user:user | 755 | 0 | source/", + "user:user | 644 | 0 | source/file.txt | hello", + "user:user | 755 | 0 | target/", + "user:user | 644 | 0 | target/file.txt | hello", + ], + faketime: "2020-01-01T00:00:00Z", + }); + + testbed.advance("1 sec"); + await testbed.chmod("source/file.txt", 0o755); + + await testbed.testStatus("config.toml", { + short: deindent` + 1→ + `, + normal: deindent` + source -> target: 1 + target -> source: 0 + `, + }); + + await testbed.testDiff("config.toml", deindent` + === file.txt (source -> target) === + --- ${testDir}/source/file.txt${"\t"}2020-01-01 00:00:01.000000000 +0000 + +++ ${testDir}/target/file.txt${"\t"}2020-01-01 00:00:00.000000000 +0000 + `); + + await testbed.testSync("config.toml", { + code: 0, + stdout: deindent` + copied file.txt -> target + + source -> target: 1 + target -> source: 0 + deleted target: 0 + deleted source: 0 + `, + stderr: "", + }); + + await testbed.assertTestDir([ + `user:user | 644 | 0 | config.cfgsync.state | ${STATE_FILE}`, + `user:user | 644 | 0 | config.toml | ${CONFIG_TOML}`, + "user:user | 755 | 0 | source/", + "user:user | 755 | 0 | source/file.txt | hello", + "user:user | 755 | 0 | target/", + "user:user | 755 | 0 | target/file.txt | hello", + ]); +}); diff --git a/e2e-tests/checked/3-classifications/3-changed-files/source-symlink-changed.test.ts b/e2e-tests/checked/3-classifications/3-changed-files/source-symlink-changed.test.ts index d5ed79f..cfcd0cc 100644 --- a/e2e-tests/checked/3-classifications/3-changed-files/source-symlink-changed.test.ts +++ b/e2e-tests/checked/3-classifications/3-changed-files/source-symlink-changed.test.ts @@ -26,8 +26,7 @@ Deno.test("sync-changed-symlink-replaces-symlink", async (t) => { await Deno.symlink("hello-new.txt", `${testDir}/source/link.txt`); // Sync detects the type change (file -> symlink) and copies to target - await testbed.run({ args: ["--config", "config.toml", "sync"] }); - testbed.assertOutput({ + await testbed.testSync("config.toml", { code: 0, stdout: deindent` copied link.txt -> target @@ -77,8 +76,7 @@ Deno.test("sync-changed-symlink-replaces-file", async (t) => { await Deno.symlink("hello", `${testDir}/source/link.txt`); // Sync detects the type change (file -> symlink) and copies to target - await testbed.run({ args: ["--config", "config.toml", "sync"] }); - testbed.assertOutput({ + await testbed.testSync("config.toml", { code: 0, stdout: deindent` copied link.txt -> target @@ -126,8 +124,7 @@ Deno.test("sync-changed-file-replaces-symlink", async (t) => { await testbed.writeTextFile("source/link.txt", "file contents"); // Sync detects the type change (file -> symlink) and copies to target - await testbed.run({ args: ["--config", "config.toml", "sync"] }); - testbed.assertOutput({ + await testbed.testSync("config.toml", { code: 0, stdout: deindent` copied link.txt -> target @@ -175,8 +172,7 @@ Deno.test("sync-changed-broken-symlink-replaces-broken-symlink", async (t) => { await Deno.symlink("hello-new.txt", `${testDir}/source/link.txt`); // Sync detects the type change (file -> symlink) and copies to target - await testbed.run({ args: ["--config", "config.toml", "sync"] }); - testbed.assertOutput({ + await testbed.testSync("config.toml", { code: 0, stdout: deindent` copied link.txt -> target diff --git a/e2e-tests/checked/3-classifications/3-changed-files/target-file-changed.test.ts b/e2e-tests/checked/3-classifications/3-changed-files/target-file-changed.test.ts index c26a3e7..d8975c0 100644 --- a/e2e-tests/checked/3-classifications/3-changed-files/target-file-changed.test.ts +++ b/e2e-tests/checked/3-classifications/3-changed-files/target-file-changed.test.ts @@ -22,43 +22,28 @@ Deno.test("both-exist-copy-to-source", async (t) => { testbed.advance("1 sec"); await testbed.writeTextFile("target/file.txt", "v2"); - await testbed.run({ args: ["--config", "config.toml", "status"] }); - testbed.assertOutput({ - code: 0, - stdout: deindent` + await testbed.testStatus("config.toml", { + short: deindent` + 1← + `, + normal: deindent` source -> target: 0 target -> source: 1 `, - stderr: "", }); - await testbed.run({ args: ["--config", "config.toml", "status", "--short"] }); - testbed.assertOutput({ - code: 0, - stdout: deindent` - 1← - `, - stderr: "", - }); - - await testbed.run({ args: ["--config", "config.toml", "diff"] }); - testbed.assertOutput({ - code: 0, - stdout: deindent` - === file.txt (target -> source) === - --- ${testDir}/target/file.txt${"\t"}2020-01-01 00:00:01.000000000 +0000 - +++ ${testDir}/source/file.txt${"\t"}2020-01-01 00:00:00.000000000 +0000 - @@ -1 +1 @@ - -v2 - \ No newline at end of file - +v1 - \ No newline at end of file - `, - stderr: "", - }); + await testbed.testDiff("config.toml", deindent` + === file.txt (target -> source) === + --- ${testDir}/target/file.txt${"\t"}2020-01-01 00:00:01.000000000 +0000 + +++ ${testDir}/source/file.txt${"\t"}2020-01-01 00:00:00.000000000 +0000 + @@ -1 +1 @@ + -v2 + \ No newline at end of file + +v1 + \ No newline at end of file + `); - await testbed.run({ args: ["--config", "config.toml", "sync"] }); - testbed.assertOutput({ + await testbed.testSync("config.toml", { code: 0, stdout: deindent` copied target -> file.txt diff --git a/e2e-tests/checked/3-classifications/3-changed-files/target-owner-changed.test.ts b/e2e-tests/checked/3-classifications/3-changed-files/target-owner-changed.test.ts new file mode 100644 index 0000000..3dddca2 --- /dev/null +++ b/e2e-tests/checked/3-classifications/3-changed-files/target-owner-changed.test.ts @@ -0,0 +1,69 @@ +import { + CONFIG_TOML, + deindent, + rootOwner, + runningOutsideDocker, + STATE_FILE, + TestBed, +} from "@/lib/index.ts"; + +Deno.test({ + name: "target-owner-invalid-skipped-by-validation", + ignore: runningOutsideDocker, +}, async (t) => { + const { testbed, username, groupname } = await TestBed.create(t, { + configToml: deindent` + [[sync]] + source = "./source" + target = "./target" + globs = ["**/*.txt"] + `, + files: [ + `user:user | 644 | 0 | config.toml | ${CONFIG_TOML}`, + `user:user | 644 | 0 | config.cfgsync.state | ${STATE_FILE}`, + "user:user | 755 | 0 | source/", + "user:user | 644 | 0 | source/file.txt | hello", + "user:user | 755 | 0 | target/", + "user:user | 644 | 0 | target/file.txt | hello", + ], + faketime: "2020-01-01T00:00:00Z", + }); + + testbed.advance("1 sec"); + await testbed.chown("target/file.txt", rootOwner); + + await testbed.run({ args: ["--config", "config.toml", "status"] }); + testbed.assertOutput({ + code: 0, + stdout: deindent` + source -> target: 0 + target -> source: 0 + failed: 1 + `, + stderr: "", + }); + + await testbed.run({ args: ["--config", "config.toml", "sync"] }); + testbed.assertOutput({ + code: 0, + stdout: deindent` + source -> target: 0 + target -> source: 0 + deleted target: 0 + deleted source: 0 + permission skips: 1 + `, + stderr: deindent` + Warning: skipping 'file.txt': target file 'file.txt' is owned by ${rootOwner}, expected '${username}:${groupname}' + `, + }); + + await testbed.assertTestDir([ + `user:user | 644 | 0 | config.cfgsync.state | ${STATE_FILE}`, + `user:user | 644 | 0 | config.toml | ${CONFIG_TOML}`, + "user:user | 755 | 0 | source/", + "user:user | 644 | 0 | source/file.txt | hello", + "user:user | 755 | 0 | target/", + `root:root | 644 | 0 | target/file.txt | hello`, + ]); +}); diff --git a/e2e-tests/checked/3-classifications/3-changed-files/target-perms-changed.test.ts b/e2e-tests/checked/3-classifications/3-changed-files/target-perms-changed.test.ts new file mode 100644 index 0000000..b466474 --- /dev/null +++ b/e2e-tests/checked/3-classifications/3-changed-files/target-perms-changed.test.ts @@ -0,0 +1,59 @@ +import { CONFIG_TOML, deindent, STATE_FILE, TestBed } from "@/lib/index.ts"; + +Deno.test("target-perms-invalid-skipped-by-validation", async (t) => { + const { testbed } = await TestBed.create(t, { + configToml: deindent` + [[sync]] + source = "./source" + target = "./target" + globs = ["**/*.txt"] + `, + files: [ + `user:user | 644 | 0 | config.toml | ${CONFIG_TOML}`, + `user:user | 644 | 0 | config.cfgsync.state | ${STATE_FILE}`, + "user:user | 755 | 0 | source/", + "user:user | 644 | 0 | source/file.txt | hello", + "user:user | 755 | 0 | target/", + "user:user | 644 | 0 | target/file.txt | hello", + ], + faketime: "2020-01-01T00:00:00Z", + }); + + testbed.advance("1 sec"); + await testbed.chmod("target/file.txt", 0o600); + + await testbed.run({ args: ["--config", "config.toml", "status"] }); + testbed.assertOutput({ + code: 0, + stdout: deindent` + source -> target: 0 + target -> source: 0 + failed: 1 + `, + stderr: "", + }); + + await testbed.run({ args: ["--config", "config.toml", "sync"] }); + testbed.assertOutput({ + code: 0, + stdout: deindent` + source -> target: 0 + target -> source: 0 + deleted target: 0 + deleted source: 0 + permission skips: 1 + `, + stderr: deindent` + Warning: skipping 'file.txt': target file 'file.txt' has permissions 600, must be 644 or 755 when no file_perms is configured + `, + }); + + await testbed.assertTestDir([ + `user:user | 644 | 0 | config.cfgsync.state | ${STATE_FILE}`, + `user:user | 644 | 0 | config.toml | ${CONFIG_TOML}`, + "user:user | 755 | 0 | source/", + "user:user | 644 | 0 | source/file.txt | hello", + "user:user | 755 | 0 | target/", + "user:user | 600 | 0 | target/file.txt | hello", + ]); +}); diff --git a/e2e-tests/checked/3-classifications/3-changed-files/target-perms-valid.test.ts b/e2e-tests/checked/3-classifications/3-changed-files/target-perms-valid.test.ts new file mode 100644 index 0000000..e0ad997 --- /dev/null +++ b/e2e-tests/checked/3-classifications/3-changed-files/target-perms-valid.test.ts @@ -0,0 +1,62 @@ +import { CONFIG_TOML, deindent, STATE_FILE, TestBed } from "@/lib/index.ts"; + +Deno.test("target-perms-valid-copy-to-source", async (t) => { + const { testbed, testDir } = await TestBed.create(t, { + configToml: deindent` + [[sync]] + source = "./source" + target = "./target" + globs = ["**/*.txt"] + `, + files: [ + `user:user | 644 | 0 | config.toml | ${CONFIG_TOML}`, + `user:user | 644 | 0 | config.cfgsync.state | ${STATE_FILE}`, + "user:user | 755 | 0 | source/", + "user:user | 644 | 0 | source/file.txt | hello", + "user:user | 755 | 0 | target/", + "user:user | 644 | 0 | target/file.txt | hello", + ], + faketime: "2020-01-01T00:00:00Z", + }); + + testbed.advance("1 sec"); + await testbed.chmod("target/file.txt", 0o755); + + await testbed.testStatus("config.toml", { + short: deindent` + 1← + `, + normal: deindent` + source -> target: 0 + target -> source: 1 + `, + }); + + await testbed.testDiff("config.toml", deindent` + === file.txt (target -> source) === + --- ${testDir}/target/file.txt${"\t"}2020-01-01 00:00:01.000000000 +0000 + +++ ${testDir}/source/file.txt${"\t"}2020-01-01 00:00:00.000000000 +0000 + `); + + await testbed.testSync("config.toml", { + code: 0, + stdout: deindent` + copied target -> file.txt + + source -> target: 0 + target -> source: 1 + deleted target: 0 + deleted source: 0 + `, + stderr: "", + }); + + await testbed.assertTestDir([ + `user:user | 644 | 0 | config.cfgsync.state | ${STATE_FILE}`, + `user:user | 644 | 0 | config.toml | ${CONFIG_TOML}`, + "user:user | 755 | 0 | source/", + "user:user | 755 | 0 | source/file.txt | hello", + "user:user | 755 | 0 | target/", + "user:user | 755 | 0 | target/file.txt | hello", + ]); +}); diff --git a/e2e-tests/lib/TestBed.ts b/e2e-tests/lib/TestBed.ts index 3e763f3..c5df77e 100644 --- a/e2e-tests/lib/TestBed.ts +++ b/e2e-tests/lib/TestBed.ts @@ -71,6 +71,12 @@ export class TestBed { await Deno.utime(path, this.mtime(), this.mtime()); } + async updateConfig(newConfigToml: string) { + this.spec.configToml = newConfigToml; + await Deno.writeTextFile(new URL("config.toml", this.testDir), newConfigToml); + await Deno.utime(new URL("config.toml", this.testDir), this.mtime(), this.mtime()); + } + async readTextFile(relativePath: string): Promise { const path = new URL(relativePath, this.testDir); return await Deno.readTextFile(path); @@ -86,6 +92,38 @@ export class TestBed { await Deno.utime(path, this.mtime(), this.mtime()); } + async chmod(relativePath: string, mode: number) { + const path = new URL(relativePath, this.testDir); + await Deno.chmod(path, mode); + await Deno.utime(path, this.mtime(), this.mtime()); + } + + async chown(relativePath: string, owner: string) { + const path = new URL(relativePath, this.testDir); + const [user, group] = owner.split(":"); + const chown = new Deno.Command("sudo", { + args: ["chown", `${user}:${group}`, path.pathname], + }); + const chownOut = await chown.output(); + if (!chownOut.success) { + throw new Error( + `sudo chown failed: ${new TextDecoder().decode(chownOut.stderr)}`, + ); + } + const touch = new Deno.Command("sudo", { + args: ["touch", "-d", this.mtime().toISOString(), path.pathname], + }); + await touch.output(); + } + + async symlink(relativePath: string, target: string) { + const path = new URL(relativePath, this.testDir); + await Deno.symlink(target, path); + await (new Deno.Command("touch", { + args: ["-h", "-d", this.mtime().toISOString(), path.pathname], + })).output(); + } + private mtime(): Date { return this.faketime?.now ?? new Date(); } @@ -134,6 +172,23 @@ export class TestBed { return this.lastRun.code; } + async testSync(configPath: string, expectedOutput: ExecReturn) { + await this.run({ args: ["--config", configPath, "sync"] }); + this.assertOutput(expectedOutput); + } + + async testStatus(configPath: string, expectedOutput: { short: string; normal: string }) { + await this.run({ args: ["--config", configPath, "status", "--short"] }); + this.assertOutput({ code: 0, stdout: expectedOutput.short, stderr: "" }); + await this.run({ args: ["--config", configPath, "status"] }); + this.assertOutput({ code: 0, stdout: expectedOutput.normal, stderr: "" }); + } + + async testDiff(configPath: string, expectedOutput: string) { + await this.run({ args: ["--config", configPath, "diff"] }); + this.assertOutput({ code: 0, stdout: expectedOutput, stderr: "" }); + } + assertOutput(expectedOutput: ExecReturn) { if (this.lastRun == null) { throw new Error("Call 'run' before checking output"); diff --git a/e2e-tests/to-check/hooks/hooks-not-run-on-copy-to-source.test.ts b/e2e-tests/to-check/hooks/hooks-not-run-on-copy-to-source.test.ts index 352f47a..7ae910f 100644 --- a/e2e-tests/to-check/hooks/hooks-not-run-on-copy-to-source.test.ts +++ b/e2e-tests/to-check/hooks/hooks-not-run-on-copy-to-source.test.ts @@ -17,17 +17,7 @@ Deno.test("hooks-not-run-on-copy-to-source", async (t) => { ], }); - await testbed.run({ args: ["--config", "config.toml", "sync"] }); - - await testbed.assertTestDir([ - `user:user | 644 | 0 | config.cfgsync.state | ${STATE_FILE}`, - `user:user | 644 | 0 | config.toml | ${CONFIG_TOML}`, - "user:user | 755 | 0 | source/", - "user:user | 644 | 0 | source/file.txt | target only content", - "user:user | 755 | 0 | target/", - "user:user | 644 | 0 | target/file.txt | target only content", - ]); - testbed.assertOutput({ + await testbed.testSync("config.toml", { code: 0, stdout: deindent` copied target -> file.txt @@ -39,4 +29,13 @@ Deno.test("hooks-not-run-on-copy-to-source", async (t) => { `, stderr: "", }); + + await testbed.assertTestDir([ + `user:user | 644 | 0 | config.cfgsync.state | ${STATE_FILE}`, + `user:user | 644 | 0 | config.toml | ${CONFIG_TOML}`, + "user:user | 755 | 0 | source/", + "user:user | 644 | 0 | source/file.txt | target only content", + "user:user | 755 | 0 | target/", + "user:user | 644 | 0 | target/file.txt | target only content", + ]); }); diff --git a/e2e-tests/to-check/hooks/hooks-unchanged.test.ts b/e2e-tests/to-check/hooks/hooks-unchanged.test.ts index 7e593bc..8e47f1f 100644 --- a/e2e-tests/to-check/hooks/hooks-unchanged.test.ts +++ b/e2e-tests/to-check/hooks/hooks-unchanged.test.ts @@ -16,15 +16,7 @@ Deno.test("hooks-not-run-on-unchanged", async (t) => { ], }); - await testbed.run({ args: ["--config", "config.toml", "sync"] }); - - await testbed.assertTestDir([ - `user:user | 644 | 0 | config.cfgsync.state | ${STATE_FILE}`, - `user:user | 644 | 0 | config.toml | ${CONFIG_TOML}`, - "user:user | 755 | 0 | source/", - "user:user | 755 | 0 | target/", - ]); - testbed.assertOutput({ + await testbed.testSync("config.toml", { code: 0, stdout: deindent` @@ -35,4 +27,11 @@ Deno.test("hooks-not-run-on-unchanged", async (t) => { `, stderr: "", }); + + await testbed.assertTestDir([ + `user:user | 644 | 0 | config.cfgsync.state | ${STATE_FILE}`, + `user:user | 644 | 0 | config.toml | ${CONFIG_TOML}`, + "user:user | 755 | 0 | source/", + "user:user | 755 | 0 | target/", + ]); }); diff --git a/e2e-tests/to-check/hooks/hooks-working-directory.test.ts b/e2e-tests/to-check/hooks/hooks-working-directory.test.ts index df1b5c3..840e457 100644 --- a/e2e-tests/to-check/hooks/hooks-working-directory.test.ts +++ b/e2e-tests/to-check/hooks/hooks-working-directory.test.ts @@ -18,19 +18,7 @@ Deno.test("hooks-working-directory-is-config-dir", async (t) => { ], }); - await testbed.run({ args: ["--config", "subdir/config.toml", "sync"] }); - - await testbed.assertTestDir([ - "user:user | 755 | 0 | subdir/", - `user:user | 644 | 0 | subdir/config.cfgsync.state | ${STATE_FILE}`, - `user:user | 644 | 0 | subdir/config.toml | ${CONFIG_TOML}`, - "user:user | 644 | 0 | subdir/hook-marker | ", - "user:user | 755 | 0 | subdir/source/", - "user:user | 644 | 0 | subdir/source/file.txt | file content", - "user:user | 755 | 0 | subdir/target/", - "user:user | 644 | 0 | subdir/target/file.txt | file content", - ]); - testbed.assertOutput({ + await testbed.testSync("subdir/config.toml", { code: 0, stdout: deindent` copied file.txt -> target @@ -43,4 +31,15 @@ Deno.test("hooks-working-directory-is-config-dir", async (t) => { `, stderr: "", }); + + await testbed.assertTestDir([ + "user:user | 755 | 0 | subdir/", + `user:user | 644 | 0 | subdir/config.cfgsync.state | ${STATE_FILE}`, + `user:user | 644 | 0 | subdir/config.toml | ${CONFIG_TOML}`, + "user:user | 644 | 0 | subdir/hook-marker | ", + "user:user | 755 | 0 | subdir/source/", + "user:user | 644 | 0 | subdir/source/file.txt | file content", + "user:user | 755 | 0 | subdir/target/", + "user:user | 644 | 0 | subdir/target/file.txt | file content", + ]); }); diff --git a/e2e-tests/to-check/hooks/hooks.test.ts b/e2e-tests/to-check/hooks/hooks.test.ts index 3903885..ae04d45 100644 --- a/e2e-tests/to-check/hooks/hooks.test.ts +++ b/e2e-tests/to-check/hooks/hooks.test.ts @@ -17,18 +17,7 @@ Deno.test("hooks-basic-execution", async (t) => { ], }); - await testbed.run({ args: ["--config", "config.toml", "sync"] }); - - await testbed.assertTestDir([ - `user:user | 644 | 0 | config.cfgsync.state | ${STATE_FILE}`, - `user:user | 644 | 0 | config.toml | ${CONFIG_TOML}`, - "user:user | 755 | 0 | source/", - "user:user | 644 | 0 | source/file.txt | file content", - "user:user | 755 | 0 | target/", - "user:user | 644 | 0 | target/file.txt | file content", - "user:user | 644 | 0 | target/hook-ran | ", - ]); - testbed.assertOutput({ + await testbed.testSync("config.toml", { code: 0, stdout: deindent` copied file.txt -> target @@ -41,4 +30,14 @@ Deno.test("hooks-basic-execution", async (t) => { `, stderr: "", }); + + await testbed.assertTestDir([ + `user:user | 644 | 0 | config.cfgsync.state | ${STATE_FILE}`, + `user:user | 644 | 0 | config.toml | ${CONFIG_TOML}`, + "user:user | 755 | 0 | source/", + "user:user | 644 | 0 | source/file.txt | file content", + "user:user | 755 | 0 | target/", + "user:user | 644 | 0 | target/file.txt | file content", + "user:user | 644 | 0 | target/hook-ran | ", + ]); }); diff --git a/e2e-tests/to-check/misc/dry-run-actions.test.ts b/e2e-tests/to-check/misc/dry-run-actions.test.ts index a2811ac..30b56ee 100644 --- a/e2e-tests/to-check/misc/dry-run-actions.test.ts +++ b/e2e-tests/to-check/misc/dry-run-actions.test.ts @@ -75,8 +75,7 @@ Deno.test("dry-run-then-real-sync", async (t) => { "user:user | 755 | 0 | target/", ]); - await testbed.run({ args: ["--config", "config.toml", "sync"] }); - testbed.assertOutput({ + await testbed.testSync("config.toml", { code: 0, stdout: deindent` copied file.txt -> target diff --git a/e2e-tests/to-check/validation/CopyToSource/copy-to-source-permission-check.test.ts b/e2e-tests/to-check/validation/CopyToSource/copy-to-source-permission-check.test.ts index 50d729b..bd5ae0a 100644 --- a/e2e-tests/to-check/validation/CopyToSource/copy-to-source-permission-check.test.ts +++ b/e2e-tests/to-check/validation/CopyToSource/copy-to-source-permission-check.test.ts @@ -29,7 +29,7 @@ Deno.test("copy-to-source-permission-check", async (t) => { permission skips: 1 `, stderr: deindent` - Warning: skipping 'file.conf' (target file has unexpected permissions 644, expected 600 for this preset) + Warning: skipping 'file.conf': target file 'file.conf' has permissions 644, expected 600 `, }); }); diff --git a/e2e-tests/to-check/validation/CopyToTarget/directory-permission-warning.test.ts b/e2e-tests/to-check/validation/CopyToTarget/directory-permission-warning.test.ts index 72e90b1..1eeb2a7 100644 --- a/e2e-tests/to-check/validation/CopyToTarget/directory-permission-warning.test.ts +++ b/e2e-tests/to-check/validation/CopyToTarget/directory-permission-warning.test.ts @@ -19,9 +19,7 @@ Deno.test("directory-permission-warning", async (t) => { ], }); - await testbed.run({ args: ["--config", "config.toml", "sync"] }); - - testbed.assertOutput({ + await testbed.testSync("config.toml", { code: 0, stdout: deindent` copied subdir/file.txt -> target diff --git a/e2e-tests/to-check/validation/CopyToTarget/nonroot-permission-warning.test.ts b/e2e-tests/to-check/validation/CopyToTarget/nonroot-permission-warning.test.ts index b170a65..c261e44 100644 --- a/e2e-tests/to-check/validation/CopyToTarget/nonroot-permission-warning.test.ts +++ b/e2e-tests/to-check/validation/CopyToTarget/nonroot-permission-warning.test.ts @@ -17,17 +17,7 @@ Deno.test("nonroot-permission-warning", async (t) => { ], }); - await testbed.run({ args: ["--config", "config.toml", "sync"] }); - - await testbed.assertTestDir([ - `user:user | 644 | 0 | config.cfgsync.state | ${STATE_FILE}`, - `user:user | 644 | 0 | config.toml | ${CONFIG_TOML}`, - "user:user | 755 | 0 | source/", - "user:user | 644 | 0 | source/file.conf | my config", - "user:user | 755 | 0 | target/", - "user:user | 600 | 0 | target/file.conf | my config", - ]); - testbed.assertOutput({ + await testbed.testSync("config.toml", { code: 0, stdout: deindent` copied file.conf -> target @@ -39,4 +29,13 @@ Deno.test("nonroot-permission-warning", async (t) => { `, stderr: "", }); + + await testbed.assertTestDir([ + `user:user | 644 | 0 | config.cfgsync.state | ${STATE_FILE}`, + `user:user | 644 | 0 | config.toml | ${CONFIG_TOML}`, + "user:user | 755 | 0 | source/", + "user:user | 644 | 0 | source/file.conf | my config", + "user:user | 755 | 0 | target/", + "user:user | 600 | 0 | target/file.conf | my config", + ]); }); diff --git a/e2e-tests/to-check/validation/CopyToTarget/permission-octal-backward-compat.test.ts b/e2e-tests/to-check/validation/CopyToTarget/permission-octal-backward-compat.test.ts index b4a4303..8a83cfb 100644 --- a/e2e-tests/to-check/validation/CopyToTarget/permission-octal-backward-compat.test.ts +++ b/e2e-tests/to-check/validation/CopyToTarget/permission-octal-backward-compat.test.ts @@ -17,27 +17,17 @@ Deno.test("file-perms-public-no-change", async (t) => { ], }); - await testbed.run({ args: ["--config", "config.toml", "status"] }); - testbed.assertOutput({ - code: 0, - stdout: deindent` + await testbed.testStatus("config.toml", { + short: deindent` + 1→ + `, + normal: deindent` source -> target: 1 target -> source: 0 `, - stderr: "", - }); - - await testbed.run({ args: ["--config", "config.toml", "status", "--short"] }); - testbed.assertOutput({ - code: 0, - stdout: deindent` - 1→ - `, - stderr: "", }); - await testbed.run({ args: ["--config", "config.toml", "sync"] }); - testbed.assertOutput({ + await testbed.testSync("config.toml", { code: 0, stdout: deindent` copied file.conf -> target diff --git a/e2e-tests/to-check/validation/CopyToTarget/permission-preset-shared.test.ts b/e2e-tests/to-check/validation/CopyToTarget/permission-preset-shared.test.ts index 4ec3b91..da32a8c 100644 --- a/e2e-tests/to-check/validation/CopyToTarget/permission-preset-shared.test.ts +++ b/e2e-tests/to-check/validation/CopyToTarget/permission-preset-shared.test.ts @@ -17,27 +17,17 @@ Deno.test("permission-preset-shared", async (t) => { ], }); - await testbed.run({ args: ["--config", "config.toml", "status"] }); - testbed.assertOutput({ - code: 0, - stdout: deindent` + await testbed.testStatus("config.toml", { + short: deindent` + 1→ + `, + normal: deindent` source -> target: 1 target -> source: 0 `, - stderr: "", - }); - - await testbed.run({ args: ["--config", "config.toml", "status", "--short"] }); - testbed.assertOutput({ - code: 0, - stdout: deindent` - 1→ - `, - stderr: "", }); - await testbed.run({ args: ["--config", "config.toml", "sync"] }); - testbed.assertOutput({ + await testbed.testSync("config.toml", { code: 0, stdout: deindent` copied file.txt -> target diff --git a/e2e-tests/to-check/validation/CopyToTarget/state-not-updated-after-warning.test.ts b/e2e-tests/to-check/validation/CopyToTarget/state-not-updated-after-warning.test.ts index 8525ded..a69f379 100644 --- a/e2e-tests/to-check/validation/CopyToTarget/state-not-updated-after-warning.test.ts +++ b/e2e-tests/to-check/validation/CopyToTarget/state-not-updated-after-warning.test.ts @@ -1,6 +1,16 @@ -import { CONFIG_TOML, deindent, rootOwner, STATE_FILE, TestBed } from "@/lib/index.ts"; +import { + CONFIG_TOML, + deindent, + rootOwner, + runningOutsideDocker, + STATE_FILE, + TestBed, +} from "@/lib/index.ts"; -Deno.test("state-should-not-be-updated-after-warning", async (t) => { +Deno.test({ + name: "state-should-not-be-updated-after-warning", + ignore: runningOutsideDocker, +}, async (t) => { const { testbed } = await TestBed.create(t, { configToml: deindent` [[sync]] diff --git a/plans/003-close-spec-gap.md b/plans/003-close-spec-gap.md deleted file mode 100644 index 8ac0877..0000000 --- a/plans/003-close-spec-gap.md +++ /dev/null @@ -1,108 +0,0 @@ -# Close the Gap: Algorithm Spec vs Implementation - -## Summary - -The goal is to have the cfgsync implementation fully match the algorithm specification described in -[docs/algorithm/index.md](../docs/algorithm/index.md) and -[docs/algorithm/permissions-and-owner.md](../docs/algorithm/permissions-and-owner.md). - -Six gaps remain. This plan addresses each in dependency order. - -## Status - -closed — all 6 phases implemented. 6 new e2e tests added (total 85, 22 root-only). 55 unit tests passing. - -## Gaps overview - -| # | Gap | Spec location | Implementation location | Complexity | -|---|-----|---------------|------------------------|------------| -| 1 | Step 4 — Validate action feasibility | index.md §4 | (missing) | High | -| 2 | Permission preset mappings (incl. `dir_perms` + reverse mapping) | permissions-and-owner.md §Permissions | config.rs, sync.rs, changes.rs | Medium | -| 3 | Deviating directories validation | index.md §1, permissions-and-owner.md | config.rs (stored only) | Medium | -| 4 | Target-to-source permission/owner validation before sync | permissions-and-owner.md §Target to source | sync.rs CopyToSource arms | Medium | -| 5 | Existing directories not updated — warnings for mismatches | permissions-and-owner.md §Source to target | sync.rs enforce/check perms | Low | -| 6 | Security edge case — files without explicit owner in foreign-owned directories | permissions-and-owner.md §Source to target §Edge case | sync.rs CopyToTarget path | Medium | - -## Design decisions - -- **Feasibility checks are non-fatal warnings by default**: If an action is infeasible, it's marked as `Failed` (the enum variant already exists at `changes.rs:58`). The `status` command shows failed counts; `sync` prints warnings and skips failed files. -- **Group the work bottom-up**: Gaps 2, 5, and 6 are lower-level permission/owner improvements that the higher-level gaps (1, 4) build on. Gap 3 is independent. -- **Preserve existing behavior for working features**: Don't break the 100 passing e2e tests. Each gap gets its own e2e test per the rule "For every new feature, an e2e test must be added." - -### Phase 1: Permission preset mappings (Gap 2) - -The `PermissionPreset` enum and `map_permissions()` already exist at `config.rs:14-58` and are partially applied. What's missing: - -- [ ] **Apply `dir_perms` at runtime**: Currently `file_perms` mapping is applied in `enforce_permissions_root` (`sync.rs:892`) and `check_permissions_nonroot` (`sync.rs:1013`), but `dir_perms` is never read. This task is deferred to Phase 2, where directory handling lands alongside the directory-mismatch warnings (per the parenthetical note above). -- [x] **Fix `Private` preset mapping**: `Private::map_permissions` always returned `0o600`, ignoring whether the source is a directory (`755`). The spec table previously said `755 → 600` for `private`, but that is incorrect — removing the directory execute bit makes the directory inaccessible. The spec has been corrected to `755 → 700` for `private` directories; `map_permissions` now returns `0o700` when `owner_perm == 0o700`. -- [x] **Implement reverse mapping for CopyToSource**: The spec says "The permissions are determined by reversing the configured mapping to the original 644 or 755 permissions" (`permissions-and-owner.md:79`). Added `reverse_map_permissions(&self, target_mode: u32) -> u32` method to `PermissionPreset` that maps back (e.g., `600 → 644`, `664 → 644`, `660 → 644`, `640 → 644`, `700 → 755` if executable bit set). It is currently `#[allow(dead_code)]`; it will be used in the CopyToSource path (Gap 4 / Phase 5). -- [x] **Fix `resolve_file_perms` in sync.rs**: Now applies `map_permissions` when a glob has `file_perms` configured, so the state file records the configured target perms (e.g., `600` for `private` files), matching spec at `index.md:71-72`: "the applied permissions of the target file at the time of the last sync, which is the same as the permissions of the source file after applying the configured mapping rules." For globs without `file_perms` and for symlinks, the raw mode / `"0"` are stored as before. -- [x] **E2e test**: Added `test-permission-presets.test.ts` — configures `file_perms = "private"` on a glob, verifies the state file records `perms = "600"` (mapped value, not raw source `644`). (Root-enforced target perms `600` and source stays `644` are already covered by `test-root-permissions-enforced.test.ts`.) - -### Phase 2: Existing directories — warnings for permission/owner mismatches (Gap 5) - -Currently `enforce_permissions_root` (`sync.rs:876`) and `check_permissions_nonroot` (`sync.rs:995`) skip directories with `if !abs_path.is_file() { continue; }` and print no warnings. - -- [x] **Add directory warning in `enforce_permissions_root`**: For directories, don't apply changes (per spec: "Existing directories are NOT updated"), but print a warning when actual perms/owner differ from configured values. -- [x] **Add directory warning in `check_permissions_nonroot`**: Same — warn about mismatches but don't change. -- [x] **Use `dir_perms` from glob config**: When checking directory permissions, use the `dir_perms` preset to determine what the expected permissions are. -- [x] **E2e test**: Added `test-directory-permission-warning.test.ts` — creates a target directory with wrong perms, runs sync as non-root, verifies warning is printed and directory perms are unchanged. - -### Phase 3: Security edge case — files without explicit owner in foreign-owned directories (Gap 6) - -Per `permissions-and-owner.md:59`: "A file or directory without explicit owner configuration is never copied into a directory owned by another user. This case is treated the same as a failure to write into that directory." - -- [x] **Add check in CopyToTarget path**: Before copying a file to the target directory, if no explicit `owner` is configured for the glob/group, check whether the parent directory is owned by the config file owner. If not, treat it as a write failure (skip file, print warning). -- [x] **Determine parent directory ownership**: Uses `std::fs::metadata(parent_dir).uid()` and compares with config file owner UID. If they differ and no explicit owner is configured (via `find_matching_glob` + `has_explicit_owner` helper), the file is skipped. -- [ ] **Test as root**: This is primarily a root-level scenario. The e2e test is marked `ignore: runningOutsideDocker` and runs in Docker. -- [x] **E2e test**: Added `test-security-foreign-dir-owner.test.ts` — as root, configures a sync group with no explicit owner pointing to a target directory owned by a different user. The test runs only in Docker (root context). - -### Phase 4: Deviating directories validation (Gap 3) - -The `deviating` field is parsed and stored in `ResolvedSyncGroup` (`config.rs:157`) as `Vec` but never read at runtime. - -- [x] **Add deviating directory checks**: After sync, for each sync group's `deviating` entries, the `check_deviating_directories` function checks actual permissions and owner against configured values. -- [x] **Print warnings for mismatches**: Directories are NOT updated — only warnings are printed, with details about found vs. expected values. -- [x] **Where to add the check**: New function `check_deviating_directories` is called after the main permission enforcement in `sync::run`. -- [x] **Deviating path canonicalization**: Deviating paths are now canonicalized during config loading (matching `source_dir` and `target_dir` handling). -- [x] **E2e test**: Added `test-deviating-directories.test.ts` — configures a sync group with `deviating` entries, creates target directories with wrong perms/owner, runs sync, verifies warnings are printed and directories are NOT changed. Also updated existing `permissions-and-owner/deviating-dir-config.test.ts`. - -### Phase 5: Target-to-source permission/owner validation (Gap 4) - -Per `permissions-and-owner.md:67-75`: "Before syncing anything, the permissions and owner of the target file are validated. If they do not match the configured permissions and owner, the whole file is skipped and a warning is printed." - -- [x] **Add validation gate in `CopyToSource`**: Before `copy_file(abs_tgt, abs_src)`, `validate_target_for_copy_to_source` checks the target file's actual permissions and owner against configured values. Fails are printed as warnings and files are skipped. -- [x] **Use reverse mapping**: To validate permissions, the function reverse-maps the configured preset and re-forward-maps to check if the target's perms match valid outputs. If actual perms don't match the round-trip, the file is skipped. -- [x] **Check owner**: Compares target file owner against configured owner using `owner_spec_matches` (UID/GID-based comparison). -- [x] **Both interactive and non-interactive paths**: Added validation in both the non-interactive (`sync.rs:144`) and interactive (`sync.rs:405`) CopyToSource arms. -- [x] **Dry-run**: In dry-run mode, validation still runs — if fail, warning is printed and file is skipped (same as non-dry-run). -- [x] **E2e test**: Added `test-copy-to-source-permission-check.test.ts` — creates a target file with public (644) perms while config requires private (600), verifies the file is skipped with a warning. - -### Phase 6: Step 4 — Validate action feasibility (Gap 1) - -This is the largest gap. The algorithm describes a full validation step between classification (step 3) and execution (step 5). - -- [x] **Add `failed_checks` field to `Change` variants**: Each `Change` variant now carries `failed_checks: Vec`. Added `failed_checks()` accessor method. `classify` initializes all with empty vecs. -- [x] **Implement `validate_actions` function**: New function in `changes.rs` that takes `&mut [Change]` + `&ResolvedConfig` and checks each action: state file writability, parent directory existence, source/target existence. -- [x] **Run validation between classify and execute**: Called `changes::validate_actions` in `main.rs` for sync, status, and diff commands after `classify` and before processing. -- [x] **`status` command**: Shows failed file count (long format: "failed: N"; short format: "✗N"). -- [x] **`sync` command**: Prints warnings for failed files and skips them (at top of execution loop in `sync::run`). -- [ ] **E2e test**: Add `test-feasibility-check.test.ts` — create a scenario where a target directory is read-only, run sync, verify the file is marked as failed with a warning. - -## Implementation order - -``` -Phase 1 (perms preset) → Phase 2 (dir warnings) → Phase 3 (foreign dir security) - ↓ -Phase 4 (deviating dirs) ← independent ────────────────┘ - ↓ -Phase 5 (target-to-source validation) → Phase 6 (feasibility checks) -``` - -## Findings - -- **Spec correction (Phase 1)**: `private` preset mapped `755 → 600`, which strips the directory execute bit and makes directories inaccessible. Corrected the spec table in `docs/algorithm/permissions-and-owner.md` to `755 → 700` for `private` directories, matching the user's instruction and the existing `shared`/`group`/`group-read` conventions (which already preserve execute bits). `PermissionPreset::Private::map_permissions` now returns `0o700` when `owner_perm == 0o700`. -- **Build targets for e2e**: Running `cargo build --release` alone is not enough — `e2e-tests/lib/env.ts` prefers the musl static binary at `target/x86_64-unknown-linux-musl/release/cfgsync` (and the `cfgsync-faketime` variant). Rebuild both musl targets before re-running e2e tests when source changes. -- **Deviating path canonicalization**: The `deviating` paths in `ResolvedDeviatingEntry` were previously stored as relative paths (no canonicalization), unlike `source_dir`/`target_dir` which are canonicalized. Now deviating paths are canonicalized during config loading, ensuring consistent absolute-path display in warnings. -- **Pre-existing test update**: `permissions-and-owner/deviating-dir-config.test.ts` used absolute system paths (`/etc/ssh`, `/etc/nginx`) which previously had no runtime effect (deviating entries were never read). After implementing `check_deviating_directories`, this test needed updating to use test-temp-directory paths with matching warning expectations. -- **Phase 6 e2e test deferred**: `test-feasibility-check.test.ts` requires creating read-only directories, which needs root context (Docker). The feasibility validation is correct by construction — all basic sanity checks (file existence, parent directory validity, state file writability) are implemented. A full e2e test would be `ignore: runningOutsideDocker` and is left as future work. \ No newline at end of file diff --git a/plans/004-explicit-owner-bypasses-security.md b/plans/004-explicit-owner-bypasses-security.md deleted file mode 100644 index 9b310e9..0000000 --- a/plans/004-explicit-owner-bypasses-security.md +++ /dev/null @@ -1,139 +0,0 @@ -# Explicit Owner Bypasses Security Checks - -## Summary - -As a cfgsync user running as root with a non-root-owned config, I want `owner = "root:root"` on a sync group to bypass file-operation and hook security checks, so that explicit owner configuration is treated as sufficient authorization and files are synced without warnings, skips, or interactive prompts. - -## Status - -open - -## Edge Cases - -- **No owner configured, config owner can't write**: Still `ErrorSkip` — no change. -- **No owner configured, config owner can write**: Still `None` (proceeds) — no change. -- **Owner configured, running non-root**: `security_bypass()` returns true (not root) — security checks never engage. No change. -- **Owner configured, root-owned config**: `security_bypass()` returns true — security never engages. No change. -- **Owner configured on group, per-glob owner differs**: `security_action()` uses group-level `owner`; `has_explicit_owner()` with fix checks both group-level and per-glob. Group-level owner takes precedence for security bypass. -- **Owner configured, target parent dir foreign to config owner (foreign-dir check)**: With `has_explicit_owner()` fix, group-level owner satisfies the check so the foreign-dir block is skipped. -- **Multiple groups, mixed owner presence**: Each group is checked independently. Groups with owner bypass; groups without still go through `ErrorSkip`/`None` logic. -- **Hook dry-run with owner configured**: Still prints `[dry-run] would run hook:*` — dry-run path unchanged. -- **Hook with owner configured, running non-root**: The existing non-root-owner skip at `run_hook_for_group` line 1404 still applies — hook skipped with "owner requires root" warning. No change. -- **Dead code removal (clippy `-D warnings`)**: Removed `WarnOrPrompt` variant, `security_prompt()`, `security_prompt_hook()`, `hook_security_needed()`, and `security_notice_printed` must be cleanly removed to avoid unused-code/fn/var warnings. - -## Tasks - -- [ ] **1. Change `security_action()` to return `None` for groups with owner** - - In `src/sync.rs`, `security_action()` line 1661: change `SecurityAction::WarnOrPrompt` to `SecurityAction::None`. - - Rationale: configuring an owner on the sync group constitutes explicit authorization; no further confirmation is needed. - -- [ ] **2. Remove `WarnOrPrompt` from `SecurityAction` enum and all match arms** - - In `src/sync.rs`: - - Remove `WarnOrPrompt` from the `SecurityAction` enum (line 1646). - - Remove the `SecurityAction::WarnOrPrompt => { ... }` arm from each of the 4 call sites: - - Non-interactive `CopyToTarget` (lines 95–110) - - Non-interactive `DeleteTarget` (lines 202–217) - - Interactive `CopyToTarget` (lines 356–374) - - Interactive `DeleteTarget` (lines 462–482) - - After this, `security_action()` has only two return values: `None` and `ErrorSkip`. - -- [ ] **3. Fix `has_explicit_owner()` to also check group-level owner** - - In `src/sync.rs`, `has_explicit_owner()` (line 863): add a check for `group.owner.is_some()` before the per-glob check: - - ```rust - fn has_explicit_owner(config: &ResolvedConfig, group_index: usize, rel_path: &str) -> bool { - let group = &config.sync_groups[group_index]; - if group.owner.is_some() { - return true; - } - find_matching_glob(group, rel_path) - .map(|g| g.owner.is_some()) - .unwrap_or(false) - } - ``` - - This ensures the foreign-parent-dir check (lines 115–128 and 378–390) also respects group-level owner. - -- [ ] **4. Remove hook security checks and dead code from `run_hook_for_group()`** - - In `src/sync.rs`: - - Remove the `security` variable and entire `if security { ... }` block (lines 1366–1397) from `run_hook_for_group()`. - - Remove the `security_notice_printed: &mut bool` parameter from `run_hook_for_group()`'s signature (line 1358). - -- [ ] **5. Remove dead functions: `security_prompt()`, `security_prompt_hook()`, `hook_security_needed()`** - - In `src/sync.rs`, remove the following now-unused functions: - - `hook_security_needed()` (lines 1628–1641) — always returns false after change. - - `security_prompt()` (lines 1677–1698) — only called from removed `WarnOrPrompt` arms. - - `security_prompt_hook()` (lines 1700–1721) — only called from removed hook security block. - - Verify `eprint_diff()` (line 1484) is still called (it is, from the interactive conflict path at line 284). - -- [ ] **6. Clean up `security_notice_printed` from `run()`** - - In `src/sync.rs`: - - Remove `let mut security_notice_printed = false;` (line 66). - - Update the two `run_hook_for_group` call sites (lines 548–556 and lines 565–572) to not pass the last argument. - -- [ ] **7. Update unit test at line 1819** - - In `src/sync.rs`, update the test call to `run_hook_for_group` to match the new signature (remove the last `&mut false` argument, replace with `false` if needed or rework the assertion). - -- [ ] **8. Update e2e tests in `security-root-target-confirm.test.ts`** - - File: `e2e-tests/to-check/validation/Security/security-root-target-confirm.test.ts` - - Tests to update (each requires assertion changes to reflect no-skip/no-prompt behavior): - - | Test name | Old behavior | New behavior | - |---|---|---| - | `security-warning-non-interactive` (L363) | File skipped, `permission skips: 1`, stderr has security warning | File copied, `source -> target: 1`, `permission skips: 0`, no security stderr | - | `security-prompt-owner-yes` (L413) | Interactive prompt, type `y`, file copied | No prompt, file copied directly. Remove `waitForStderr`/`type` calls. Assert no security stderr. | - | `security-prompt-owner-no` (L466) | Interactive prompt, type `n`, file skipped | No prompt, file copied directly. Same result as `owner-yes`. Remove prompt interaction. | - | `security-prompt-owner-quit` (L517) | Interactive prompt, type `q`, exit code 1 | No prompt, file copied, exit code 0. Remove prompt interaction. | - | `security-hook-owner-mismatch-yes` (L147) | Interactive prompts for file + hook, type `y` twice | No prompts. File copied, hook runs. Remove `waitForStderr`/`type` calls. | - | `security-hook-owner-mismatch-no` (L206) | Interactive prompts, type `y` for file, `n` for hook, hook skipped | No prompts. File copied, hook runs. Remove prompt interaction. | - | `security-hook-owner-mismatch-quit` (L265) | Interactive prompts, type `y` for file, `q` for hook, exit 1 | No prompts. File copied, hook runs, exit 0. Remove prompt interaction. | - - Tests that **do NOT need changes** (assertions unchanged): - - `security-bypass-root-owned-config` — bypass path unchanged - - `security-bypass-non-root` — bypass path unchanged - - `security-error-skip-cannot-write-dir` — no-owner ErrorSkip path unchanged - - `security-hook-no-owner-runs-as-config-owner` — no-owner hook path unchanged - - `security-foreign-dir-owner` (separate file) — unchanged - -- [ ] **9. Update AGENTS.md security documentation** - - In `AGENTS.md`, update the bullet under "Security confirmation" that reads: - > Groups with an `owner` configured always require `WarnOrPrompt` (chown is always privilege escalation). - - To: - > Groups with an `owner` configured bypass file-operation and hook security checks (the explicit owner is treated as authorization). Groups without an `owner` trigger `ErrorSkip` when the config owner lacks write permission to the target. - - Also check the "Hooks" bullet for similar language. - -- [ ] **10. Update `docs/algorithm/permissions-and-owner.md`** - - This file currently states (line 59–64): - > *A file or directory without explicit owner configuration is never copied into a directory owned by another user.* [...] *If it is necessary to create such a file, the owner can be set explicitely in the configuration.* - - Three changes needed: - 1. **Make the statement accurate**: The old text implies setting `owner` always works, but the old code required `-i` too. After this plan, it's truly correct — swap "can be set" for "must be set" and note that hooks are also authorized. Suggested rewrite: - > *A file or directory without explicit owner configuration is never copied into a directory owned by another user. If it is necessary to create such a file, the owner **must be set** explicitly in the configuration (on the sync group or the matching glob). Setting an explicit owner also authorizes hooks configured on that group to execute without additional security prompts.* - - 2. **Document the security bypass path**: Add a short paragraph explaining that when the config file is root-owned and not group/other-writable, all security checks are bypassed entirely (a trusted-config shortcut). This is a significant design point currently missing from the docs. - - 3. **Remove/update interactive-mode references**: The doc currently implies a binary "works / doesn't work." After the change, explicit owner = always proceeds (no interactive prompt needed). Update any language that suggests interactive confirmation is required for owner-configured groups. - -- [ ] **11. Run `mise run all-local` and fix any failures** - - After all changes, run full verification. Expected: all unit tests pass, all e2e tests pass, clippy has no warnings, fmt is clean. - -## Findings - - diff --git a/plans/005-fix-is-changed-owner-perms.md b/plans/005-fix-is-changed-owner-perms.md new file mode 100644 index 0000000..f2feef0 --- /dev/null +++ b/plans/005-fix-is-changed-owner-perms.md @@ -0,0 +1,137 @@ +# Fix: `is_changed()` and no-state equality — owner/permissions comparison + +## Status + +closed — implemented. `is_changed()` compares owner+perms against state. No-state equality checks owner. +8 tests added to `e2e-tests/checked/3-classifications/3-changed-files/`. + +## Summary + +The algorithm spec (sec 3.1) defines `is_equal(a, b)` as: `a.hash == b.hash AND a.perms == b.perms +AND a.owner == b.owner AND a.type == b.type`. `is_changed(file)` is `file.mtime != state.mtime AND +NOT is_equal(file, state)`. + +The implementation omits owner and permissions from both comparisons. This causes metadata-only +changes (owner or perms shift without content change) to be classified as `Clean` instead of +`CopyToTarget` or `CopyToSource`. + +## Status + +Not started. + +## Files affected + +| File | Change | +|------|--------| +| `src/changes.rs:451-468` | `is_changed()` — add owner + perms comparison against `state_entry` | +| `src/changes.rs:277-310` | No-state `classify_entry` — add owner comparison to the `UpdateState` vs `Conflict` decision | + +## Current behavior + +**`is_changed()`** checks only: +1. `file_type` matches state +2. `mtime` differs from state +3. `hash` differs from state + +Owner and permissions in the state file are ignored entirely. + +**No-state equality** checks: +1. Symlink type matches +2. Hash matches +3. File permissions match (mode & 0o777) + +Owner is ignored. + +## Required changes + +### 1. `is_changed()` (`changes.rs:451`) + +Add to the mtime-shortcut check: when `file.mtime == state_mtime`, additionally verify that +permissions and owner also match state. If they don't, the file IS changed. + +Add after the hash comparison: if hash matches but perms or owner differ from state, the file +IS changed. + +```rust +fn is_changed(file: &DiscoveredFile, abs_path: &Path, state_entry: &FileEntry) -> bool { + let file_type_str = if file.is_symlink { "symlink" } else { "file" }; + if state_entry.file_type != file_type_str { + return true; + } + + let state_mtime = parse_mtime_to_i64(&state_entry.mtime).unwrap_or(0); + if file.mtime == state_mtime { + // Same mtime → check perms and owner didn't change from state + return perms_differ_from_state(abs_path, state_entry) + || owner_differs_from_state(abs_path, state_entry); + } + + let file_hash = compute_file_hash(abs_path, file.is_symlink, file.symlink_target.as_deref()); + let Some(file_hash) = file_hash else { + return true; + }; + + if file_hash != state_entry.hash { + return true; + } + + // Hash matches, but check if perms/owner diverged from state + perms_differ_from_state(abs_path, state_entry) + || owner_differs_from_state(abs_path, state_entry) +} +``` + +Helper functions: +- `perms_differ_from_state(path, state_entry)` — returns `true` if file's current mode differs + from `state_entry.perms` (parsed as octal) +- `owner_differs_from_state(path, state_entry)` — returns `true` if file's current uid:gid + differs from `state_entry.owner` (parsed as `user:group`) + +### 2. No-state equality (`changes.rs:294`) + +After checking `src_hash == tgt_hash && perms_equal`, also check that source and target have +the same owner (uid:gid). If they differ → `Conflict` instead of `UpdateState`. + +```rust +let owner_equal = file_owner_matches(abs_src, abs_tgt); +if src_hash.is_some() && src_hash == tgt_hash && perms_equal && owner_equal { + Change::UpdateState { .. } +} else { + Change::Conflict { .. } +} +``` + +Helper: +- `file_owner_matches(a, b)` — returns `true` if both files have the same uid and gid. + +## E2e tests to add + +### B1. Owner change without content change detected as CopyToTarget + +``` +Setup: source+target both have file.txt with content "hello", tracked in state. +Change: chown target/file.txt to a different owner. +Expected: status shows source -> target: 1 (CopyToTarget because target owner diverges from state). +``` + +### B1b. Permissions change without content change detected as CopyToSource + +``` +Setup: source+target both have file.txt with content "hello", tracked in state. +Change: chmod target/file.txt to different perms. +Expected: status shows target -> source: 1 (CopyToSource because target perms diverge from state). +``` + +### B10. No-state: identical files but different owner → conflict + +``` +Setup: source/file.txt and target/file.txt with identical content and perms, but different owner. +No state file. +Expected: status shows conflict: 1 (not UpdateState). +``` + +## Verification + +```bash +mise run all-local +``` diff --git a/plans/006-validate-action-feasibility.md b/plans/006-validate-action-feasibility.md new file mode 100644 index 0000000..2efc717 --- /dev/null +++ b/plans/006-validate-action-feasibility.md @@ -0,0 +1,122 @@ +# Validate Action Feasibility — Step 4 + +## Summary + +The algorithm spec (sec 4) defines a validation step that runs between classification and execution. +It checks whether each action can actually be performed. The existing `validate_action()` function +only does rudimentary existence/path checks. Seven feasibility checks are missing. + +## Status + +Not started. + +## Files affected + +| File | Change | +|------|--------| +| `src/changes.rs:616-733` | `validate_actions()` / `validate_action()` — expand with full checks | +| `src/sync.rs` | No changes; checks are done in validation, not execution | + +## Required feasibility checks (from sec 4 of algorithm) + +| # | Action | Check needed | Currently | +|---|--------|-------------|-----------| +| 1 | `CopyToTarget` | Missing parent directories can be created | Not checked | +| 2 | `CopyToTarget` | Existing parent directories have correct permissions-and-owner | Not checked | +| 3 | `CopyToTarget` | Target file can be written or created | Not checked | +| 4 | `CopyToTarget` | Correct file owner can be set (running as root or as intended owner) | Not checked | +| 5 | `CopyToSource` | Source file can be written or created | Not checked | +| 6 | `CopyToSource` | Missing parent directories can be created | Not checked | +| 7 | `CopyToSource` | Correct file owner can be set (running as root or as config file owner) | Not checked | +| 8 | `DeleteTarget` | Target file can be deleted | Not checked | +| 9 | `DeleteSource` | Source file can be deleted | Not checked | + +## Design decisions + +- **Failed checks are warnings**: Actions with failed checks get populated in `failed_checks`. + `sync.rs` already skips actions with non-empty `failed_checks` and prints warnings. + No change needed to the execution path. +- **`status` shows failed count**: `ChangeCounts` already has a `failed` field. `print_status` + already prints it. +- **Don't break existing behavior**: The current shallow checks stay in place; we add additional + checks. Existing e2e tests should continue to pass. +- **Parent directory owner check**: For CopyToTarget, verify that existing parent dirs have owner + matching the config's owner (or config file owner as fallback). Permissions check uses + `dir_perms` from config. +- **File writable**: For target path `/etc/foo/bar.conf`, check that the parent `/etc/foo/` + allows writing (based on current user's uid/gid and directory mode). + +## Implementation approach + +Expand `validate_action()`: + +### For `CopyToTarget`: + +```rust +Change::CopyToTarget { abs_src, abs_tgt, group_index, .. } => { + check_state_writable(failed_checks, config); + let group = &config.sync_groups[*group_index]; + + // Existing checks (source exists, parent is not a non-dir file) + // ... keep existing ... + + // NEW: check parent directories can be created or are writable + check_parent_dirs_creatable(abs_tgt, group.target_dir, failed_checks); + + // NEW: check existing parent dirs have correct owner + dir_perms + check_parent_dir_owner_and_perms(abs_tgt, group, config, failed_checks); + + // NEW: check target writable + check_target_writable(abs_tgt, failed_checks); + + // NEW: check owner settable + check_owner_settable(group, rel_path, false /* is_copy_to_source */, config, failed_checks); +} +``` + +### For `CopyToSource`: + +Similar but for source side, with config file owner. + +### For `DeleteTarget` / `DeleteSource`: + +Check parent directory writability. + +## Reuse existing helpers + +Several checks can reuse or extract from existing `sync.rs` helpers: +- `parent_dir_owned_by_foreign_user()` (already checks if parent dir owned by another user) +- `check_owner_feasibility()` (non-root + explicit owner = infeasible) +- `can_write()` (checks uid/gid against mode bits) + +## E2e test to add + +### Target parent dir not writable + +``` +Setup: target/ owned by root:root with 700 perms. Config owned by user:user. +Source has file.txt. +Expected: status shows failed: 1 (cannot write parent dir). +``` + +### Source parent dir not writable (CopyToSource) + +``` +Setup: source/ owned by root:root with 700 perms. Config owned by user:user. +Target has file.txt. +Expected: status shows failed: 1 (cannot write source parent dir). +``` + +### Owner not settable (non-root, owner config set) + +``` +Setup: non-root, config has owner = "otheruser:othergroup". +Source has file.txt. +Expected: status shows failed: 1 (owner not settable without root). +``` + +## Verification + +```bash +mise run all-local +``` diff --git a/plans/007-fix-dir-perms-and-parent-dirs.md b/plans/007-fix-dir-perms-and-parent-dirs.md new file mode 100644 index 0000000..a09bffc --- /dev/null +++ b/plans/007-fix-dir-perms-and-parent-dirs.md @@ -0,0 +1,125 @@ +# Fix: `dir_perms` enforcement and parent directory permissions on CopyToTarget + +## Summary + +The algorithm spec says: +1. `dir_perms` should be applied to directories (permissions-and-owner.md) +2. "Create missing parent directories in the target folder and set the correct owner and + permissions" (index.md §5.3, CopyToTarget) + +Currently `dir_perms` is only used for warnings in `enforce_permissions_root()` (root-only). +Non-root gets no directory permission warnings. Newly created parent directories via +`create_dir_all()` get no owner or `dir_perms` applied. + +## Status + +Not started. + +## Files affected + +| File | Change | +|------|--------| +| `src/sync.rs` | `copy_file()` — apply owner+dir_perms to newly created parent dirs | +| `src/sync.rs` | Add non-root directory permission warnings (extract from root-only path) | + +## Current behavior + +1. **`copy_file()`** (`sync.rs:543`): Calls `create_dir_all()` on parent, then copies the file. + No chown/chmod on created directories. +2. **`warn_directory_permission_mismatch()`** (`sync.rs:1114`): Only called from + `enforce_permissions_root()` which runs as root. Non-root never gets directory warnings. + +## Required changes + +### 1. Apply owner + dir_perms to newly created parent directories + +After `create_dir_all(parent)`, walk up from `parent` to `group.target_dir`. For each directory +that `create_dir_all` just created (check by trying to read metadata — if it now exists and +is a directory), apply the configured owner and `dir_perms`. + +```rust +fn copy_file(src: &Path, dst: &Path, group: &ResolvedSyncGroup) -> Result<(), String> { + // ... existing symlink/source check ... + + if let Some(parent) = dst.parent() { + // Track directories that exist BEFORE create_dir_all + let existing: HashSet = walk_parents_to_target(parent, &group.target_dir) + .filter(|p| p.exists()) + .collect(); + + std::fs::create_dir_all(parent)?; + + // For directories that DIDN'T exist before, apply owner + dir_perms + for p in walk_parents_to_target(parent, &group.target_dir) { + if !existing.contains(&p) && p.is_dir() { + apply_directory_owner(&p, group); + apply_directory_perms(&p, group); + } + } + } + + // ... rest of copy ... +} +``` + +Note: this requires passing `&ResolvedSyncGroup` to `copy_file()`, which means updating the +caller in `sync.rs` run loop. + +### 2. Non-root directory permission warnings + +Extract directory warning logic from `enforce_permissions_root()` into a standalone function +and call it from the non-root path as well. + +The existing `enforce_permissions_root()` iterates globs in the target dir, checks files for +perms/owner, and calls `warn_directory_permission_mismatch()` for dirs. After sync in +non-root mode, we should similarly walk target directories and warn about mismatches. + +Simplest approach: call `enforce_permissions_root()` renamed to `check_permissions()` that +works for both root and non-root (root applies changes, non-root only warns). Or call the +existing function from both paths — `enforce_permissions_root` already only warns for dirs. + +Actually looking at the code, `enforce_permissions_root` has `if is_root()` guard. We need +to split this: + +- Root: `enforce_permissions_root()` — applies file perms + chown, warns for dirs +- Non-root: new `check_permissions_nonroot()` — warns for files + dirs + +Wait, there's no `check_permissions_nonroot` currently. Let me re-read the sync run flow... + +In `sync.rs::run()` (line 499): +```rust +if !dry_run { + if is_root() { + enforce_permissions_root(config, state)?; + } + check_deviating_directories(config); + // hooks... +} +``` + +So non-root currently gets NO permission checking at all (except for the per-file +`apply_target_permissions` during CopyToTarget which tries to set perms and warns on +failure). We need to add a `check_permissions_nonroot()` call in the `else` branch. + +## E2e tests to add + +### B2. `dir_perms` enforcement as root (sudo test) + +``` +Setup: source has subdir/ with a file. Config has dir_perms = "private" on glob. +Expected after sync as root: target/subdir/ has 700 perms. +``` + +### B3. Parent directory owner + perms during CopyToTarget + +``` +Setup: source has deep/path/file.txt. Config has owner = "someuser:somegroup", dir_perms = "private". +Expected after sync: target/deep/ and target/deep/path/ have owner = someuser:somegroup and +permissions = 700. +``` + +## Verification + +```bash +mise run all-local +``` diff --git a/plans/008-fix-copy-to-source-permissions.md b/plans/008-fix-copy-to-source-permissions.md new file mode 100644 index 0000000..620b062 --- /dev/null +++ b/plans/008-fix-copy-to-source-permissions.md @@ -0,0 +1,95 @@ +# Fix: CopyToSource — enforce source permissions + +## Summary + +The algorithm spec (sec 5.3, `CopyToSource`) says: "Ensure that permissions and owner match the +valid values." The permissions-and-owner.md spec (§Target to source) says: "The permissions are +determined by reversing the configured mapping to the original 644 or 755 permissions." + +Currently, after CopyToSource, only owner is applied (via `apply_source_owner`, root-only). +Permissions are never enforced on the source file. + +## Status + +Not started. + +## Files affected + +| File | Change | +|------|--------| +| `src/sync.rs` | CopyToSource execution path — add source permission enforcement | +| `src/config.rs` | `PermissionPreset::reverse_map_permissions` already exists | + +## Current behavior + +In the CopyToSource execution path (`sync.rs`), after `copy_file(abs_tgt, abs_src)` succeeds: + +```rust +apply_source_owner(config, *group_index, abs_src); +``` + +Nothing enforces permissions on the source file. The source file gets whatever permissions +`copy_file` sets (typically the target file's permissions preserved by `std::fs::copy`), +which may not be the canonical 644/755. + +## Required changes + +Add a `apply_source_permissions()` function and call it after `apply_source_owner()`: + +```rust +fn apply_source_permissions( + config: &ResolvedConfig, + group_index: usize, + src_path: &Path, + rel_path: &str, +) { + let group = &config.sync_groups[group_index]; + let glob_entry = match find_matching_glob(group, rel_path) { + Some(g) => g, + None => return, + }; + + let Some(ref preset) = glob_entry.file_perms else { + return; + }; + + let Ok(metadata) = std::fs::symlink_metadata(src_path) else { + return; + }; + if metadata.file_type().is_symlink() { + return; + } + + let current_mode = metadata.permissions().mode() & 0o777; + let canonical_mode = preset.reverse_map_permissions(current_mode); + let perms = std::fs::Permissions::from_mode(canonical_mode); + + if current_mode != canonical_mode { + if is_root() { + let _ = std::fs::set_permissions(src_path, perms); + } else { + eprintln!( + "Permission warning: source '{}' has {:o}, should have {:o} (run as root to fix)", + rel_path, current_mode, canonical_mode + ); + } + } +} +``` + +## E2e test to add + +### B4. Source permission enforcement after CopyToSource + +``` +Setup: config has file_perms = "private". Target has file.txt with content, perms = 600. +No state file. +Expected after sync: source/file.txt exists with content from target, perms = 644. +(600 reverse-maps to 644 for regular files.) +``` + +## Verification + +```bash +mise run all-local +``` diff --git a/plans/009-fix-last-sync-timestamp.md b/plans/009-fix-last-sync-timestamp.md new file mode 100644 index 0000000..b8885fd --- /dev/null +++ b/plans/009-fix-last-sync-timestamp.md @@ -0,0 +1,71 @@ +# Fix: `last_sync` timestamp in state file + +## Summary + +The algorithm spec (sec 5.3, `UpdateState`) says: "Update the mtime of both files to the +newest of both mtimes and set this time in the state as `last_sync`." + +Currently `update_state()` sets `state.last_sync = crate::time::now()` — the current +wall-clock time — instead of the maximum mtime across all synced files. + +## Status + +Not started. + +## Files affected + +| File | Change | +|------|--------| +| `src/sync.rs:629` | `update_state()` — compute `last_sync` from file mtimes instead of `now()` | + +## Current behavior + +```rust +state.last_sync = crate::time::now(); +state.file.clear(); +// ... rebuild file entries ... +``` + +Each file entry gets `mtime_val = src_mtime.max(tgt_mtime)`, but the global `last_sync` is +wall-clock time. + +## Required changes + +Track the maximum mtime seen across all rebuilt file entries, and use that for `last_sync`. + +```rust +let mut max_mtime: i64 = 0; + +// Inside the file rebuild loop: +if src_mtime > 0 || tgt_mtime > 0 || is_symlink { + let mtime_val = src_mtime.max(tgt_mtime); + max_mtime = max_mtime.max(mtime_val); + // ... create FileEntry with mtime_str ... +} + +// After rebuilding all entries: +state.last_sync = if max_mtime > 0 { + DateTime::from_timestamp_millis(max_mtime) + .unwrap_or(crate::time::now()) +} else { + crate::time::now() +}; +``` + +Edge case: if no files were synced and no files exist on disk (empty sync), fall back to `now()`. + +## E2e test to add + +### B6. State file `last_sync` reflects file mtimes + +``` +Setup: source has file.txt. Fake time at 2024-01-01T00:00:00Z. +Sync runs. Read the state file. +Expected: state.last_sync == "2024-01-01T00:00:00.000Z" (the file's mtime, not wall-clock). +``` + +## Verification + +```bash +mise run all-local +``` diff --git a/plans/010-fix-diff-output.md b/plans/010-fix-diff-output.md new file mode 100644 index 0000000..fd10610 --- /dev/null +++ b/plans/010-fix-diff-output.md @@ -0,0 +1,85 @@ +# Fix: Diff output — add owner and permissions information + +## Summary + +The algorithm spec (sec 5.2) says diff output should show owner and permission deltas: + +``` +=== file.conf (source -> target) === +Owner: -> +Perms: -> + +--- /abs/path/source/file.conf ... ++++ /abs/path/target/file.conf ... +``` + +Currently `print_diffs()` only shows the unified content diff. Owner and permissions are +missing from all diff variants (CopyToTarget, CopyToSource, Conflict, DeleteTarget, DeleteSource). + +## Status + +Not started. + +## Files affected + +| File | Change | +|------|--------| +| `src/diff.rs` | `print_diffs()` — add owner + perms lines before unified diff | + +## Required changes + +For `CopyToTarget`, `CopyToSource`, and `Conflict` change variants, after printing the header +and before the unified diff, print: + +``` +Owner: -> +Perms: -> +``` + +Where: +- **CopyToTarget**: `old` = target file's current owner/perms, `new` = configured owner/perms + from the glob's settings (or defaults) +- **CopyToSource**: `old` = source file's current owner/perms, `new` = config file owner + reverse-mapped perms +- **Conflict**: `old` = target (displayed as "Target-Owner"), `new` = configured values + +For `DeleteTarget` and `DeleteSource`, the algorithm already specifies the correct format +(no owner/perms shown), so no change needed. + +### Implementation approach + +Add helper functions: + +```rust +fn format_owner_uid_gid(path: &Path) -> String { ... } +fn format_perms_octal(path: &Path) -> String { ... } +fn configured_owner_for_copy_to_target(group: &ResolvedSyncGroup, rel_path: &str) -> String { ... } +fn configured_perms_for_copy_to_target(group: &ResolvedSyncGroup, rel_path: &str, src_path: &Path) -> String { ... } +``` + +In `print_diffs()`, for each variant: +- Read old owner/perms from the destination file (target for CopyToTarget, source for CopyToSource) +- Compute new owner/perms from config +- If old ≠ new, print the delta lines + +## E2e test to add + +### B7. Diff output includes owner and permissions + +``` +Setup: config has file_perms = "private", owner = "otheruser:othergroup". +Source has file.txt with content "hello", owner user:user, perms 644. +Target has file.txt with content "world", owner user:user, perms 644. +Run: cfgsync diff +Expected output includes: + === file.txt (source -> target) === + Owner: user:user -> otheruser:othergroup + Perms: 644 -> 600 + --- .../source/file.txt ... + +++ .../target/file.txt ... +``` + +## Verification + +```bash +mise run all-local +``` diff --git a/plans/011-fix-deviating-dirs.md b/plans/011-fix-deviating-dirs.md new file mode 100644 index 0000000..04b3d72 --- /dev/null +++ b/plans/011-fix-deviating-dirs.md @@ -0,0 +1,94 @@ +# Fix: Deviating directories — check optional expected permissions + +## Summary + +The algorithm spec (sec 1) says a deviating entry has: +- `path` (no glob) +- `optional expected permission` +- `optional expected owner` + +Currently `check_one_deviating_directory()` only checks `expected_owner`. The `permissions` field +on `DeviatingEntry` exists but is never compared. + +The existing e2e test (`deviating-dir-config.test.ts`) is in `wrong/` because it expects no +warnings — it should warn about mismatching owner. + +## Status + +Not started. + +## Files affected + +| File | Change | +|------|--------| +| `src/config.rs` | `DeviatingEntry` / `ResolvedDeviatingEntry` — check if `permissions` field exists | +| `src/sync.rs:1221-1259` | `check_deviating_directories()` / `check_one_deviating_directory()` — add permission check | + +## Current behavior + +`ResolvedDeviatingEntry` has: +```rust +pub path: PathBuf, +pub owner: Option, +``` + +No `permissions` field. The TOML config schema (schema_doc.toml) shows `permissions` as an +optional field on `deviating` entries, but it's not deserialized. + +## Required changes + +### 1. Add `permissions` field to deviating entry type + +In `config.rs`, add `permissions: Option` to the config struct and +`permissions: Option` (parsed octal) to the resolved struct. + +### 2. Deserialize and validate the field + +Accept octal strings like `"755"` and `"644"` as valid permission values. + +### 3. Check permissions in `check_one_deviating_directory()` + +```rust +fn check_one_deviating_directory( + dir_path: &Path, + expected_owner: &Option, + expected_perms: &Option, +) { + // ... existing metadata/dir checks ... + + if let Some(owner_spec) = expected_owner + && !owner_spec_matches(&metadata, owner_spec) + { + eprintln!("Warning: deviating directory '{}' is owned by {}, expected '{}' ...", + dir_path.display(), format_actual_owner(&metadata), owner_spec); + } + + // NEW: permission check + if let Some(expected_mode) = expected_perms { + use std::os::unix::fs::PermissionsExt; + let actual_mode = metadata.permissions().mode() & 0o777; + if actual_mode != *expected_mode { + eprintln!("Warning: deviating directory '{}' has perms {:o}, expected {:o} ...", + dir_path.display(), actual_mode, expected_mode); + } + } +} +``` + +### 4. Move `deviating-dir-config.test.ts` from `wrong/` to `to-check/` + +After the fix, the test should pass with the correct warnings. Add a second test case that +sets both `owner` and `permissions` on the deviating entry and verifies both warnings appear. + +## E2e test + +Update and rewrite the existing `wrong/deviating-dir-config.test.ts`: +- Test 1: Deviating directory with wrong owner → warning +- Test 2: Deviating directory with wrong permissions → warning +- Test 3: Deviating directory with both wrong → both warnings + +## Verification + +```bash +mise run all-local +``` diff --git a/plans/012-additional-e2e-edge-cases.md b/plans/012-additional-e2e-edge-cases.md new file mode 100644 index 0000000..355f0d0 --- /dev/null +++ b/plans/012-additional-e2e-edge-cases.md @@ -0,0 +1,76 @@ +# Additional E2E Edge Case Tests + +## Summary + +Add e2e tests for algorithm scenarios not covered by existing tests or the feature-specific +plans above. These are standalone tests that don't require implementation changes. + +## Status + +Not started. + +## Tests to add + +### B8. Stale group in state file + +``` +Setup: State file has a file entry with group = "./nonexistent" (a target dir path that +no longer matches any sync group). +Source and target dirs exist with matching files. + +Expected: cfgsync status runs successfully (exit 0), ignores the stale state entry. +The stale entry should not cause an error or be included in change classification. +``` + +### B9. File type change with state (symlink → regular file on source) + +``` +Setup: source+target both have link.txt as a symlink to "hello", tracked in state as symlink. +Change: Delete source/link.txt, write source/link.txt as a regular file with content "hello". +Expected: status shows source -> target: 1 (CopyToTarget, type change detected). +``` + +### B10. File type change with state (regular file → symlink on target) + +``` +Setup: source+target both have link.txt as a regular file, tracked in state as file. +Change: Replace target/link.txt with a symlink. +Expected: status shows target -> source: 1 (CopyToSource, type change detected). +``` + +### B11. Interactive conflict resolution preserves configured permissions + +``` +Setup: source+target both have file.txt with different content, tracked in state. +Config has file_perms = "private". +Run: cfgsync sync -i, choose [t]arget (CopyToTarget). + +Expected: After sync, target/file.txt has content from source and perms = 600 (private preset). +State file tracks the file with perms = "600". +``` + +### B12. File matching multiple globs within same group → error + +``` +Setup: Single sync group with two globs that overlap (e.g., "*.conf" and "*.{conf,txt}"). +Source has file.conf. + +Expected: cfgsync exits with error about multiple globs matching the same file. +``` + +Note: B12 might already be covered by a unit test in `changes.rs` (`test_glob_respects_glob` +only tests filtering, not overlap detection). Verify if `seen` HashSet in `scan_dir` handles +this — it does (line 576-581 of `changes.rs`), so this test validates the e2e error message. + +## Priority order + +1. B8 (stale group) — simplest, validates robustness +2. B9 + B10 (file type changes) — extends existing symlink tests +3. B11 (interactive perms) — covers interactive mode edge case +4. B12 (intra-group glob overlap) — valid but low priority (unit test might already cover it) + +## Verification + +```bash +mise run all-local +``` diff --git a/src/changes.rs b/src/changes.rs index 3859b7b..879e70e 100644 --- a/src/changes.rs +++ b/src/changes.rs @@ -291,7 +291,8 @@ fn classify_entry( let tgt_hash = compute_file_hash(abs_tgt, _t.is_symlink, _t.symlink_target.as_deref()); let perms_equal = file_perms_match(abs_src, abs_tgt); - if src_hash.is_some() && src_hash == tgt_hash && perms_equal { + let owner_equal = file_owner_matches(abs_src, abs_tgt); + if src_hash.is_some() && src_hash == tgt_hash && perms_equal && owner_equal { Change::UpdateState { group_index: gi, rel_path: rel, @@ -456,7 +457,8 @@ fn is_changed(file: &DiscoveredFile, abs_path: &Path, state_entry: &FileEntry) - let state_mtime = parse_mtime_to_i64(&state_entry.mtime).unwrap_or(0); if file.mtime == state_mtime { - return false; + return perms_differ_from_state(abs_path, state_entry) + || owner_differs_from_state(abs_path, state_entry); } let file_hash = compute_file_hash(abs_path, file.is_symlink, file.symlink_target.as_deref()); @@ -464,7 +466,74 @@ fn is_changed(file: &DiscoveredFile, abs_path: &Path, state_entry: &FileEntry) - return true; }; - file_hash != state_entry.hash + if file_hash != state_entry.hash { + return true; + } + + perms_differ_from_state(abs_path, state_entry) + || owner_differs_from_state(abs_path, state_entry) +} + +fn perms_differ_from_state(abs_path: &Path, state_entry: &FileEntry) -> bool { + let Ok(metadata) = std::fs::symlink_metadata(abs_path) else { + return true; + }; + if metadata.file_type().is_symlink() { + return false; + } + let actual_mode = metadata.permissions().mode() & 0o777; + let state_perms = u32::from_str_radix(&state_entry.perms, 8).unwrap_or(0); + actual_mode != state_perms +} + +fn owner_differs_from_state(abs_path: &Path, state_entry: &FileEntry) -> bool { + use std::os::unix::fs::MetadataExt; + let Ok(metadata) = std::fs::symlink_metadata(abs_path) else { + return true; + }; + if metadata.file_type().is_symlink() { + return false; + } + let actual_uid = metadata.uid(); + let actual_gid = metadata.gid(); + + let parts: Vec<&str> = state_entry.owner.splitn(2, ':').collect(); + if parts.len() != 2 { + return false; + } + + if let (Ok(exp_uid), Ok(exp_gid)) = (parts[0].parse::(), parts[1].parse::()) { + return actual_uid != exp_uid || actual_gid != exp_gid; + } + + let exp_uid = nix::unistd::User::from_name(parts[0]) + .ok() + .flatten() + .map(|u| u.uid.as_raw()); + let exp_gid = nix::unistd::Group::from_name(parts[1]) + .ok() + .flatten() + .map(|g| g.gid.as_raw()); + + match (exp_uid, exp_gid) { + (Some(eu), Some(eg)) => actual_uid != eu || actual_gid != eg, + _ => false, + } +} + +fn file_owner_matches(a: &Path, b: &Path) -> bool { + use std::os::unix::fs::MetadataExt; + let meta_a = std::fs::symlink_metadata(a); + let meta_b = std::fs::symlink_metadata(b); + match (meta_a, meta_b) { + (Ok(ma), Ok(mb)) => { + if ma.file_type().is_symlink() || mb.file_type().is_symlink() { + return true; + } + ma.uid() == mb.uid() && ma.gid() == mb.gid() + } + _ => false, + } } fn file_perms_match(a: &Path, b: &Path) -> bool { @@ -656,6 +725,7 @@ fn validate_action(change: &mut Change, config: &ResolvedConfig) { abs_tgt, failed_checks, rel_path, + group_index, .. } => { check_state_writable(failed_checks, config); @@ -674,6 +744,21 @@ fn validate_action(change: &mut Change, config: &ResolvedConfig) { parent.display() )); } + validate_target_perms_for_copy_to_source( + abs_tgt, + rel_path, + *group_index, + config, + failed_checks, + ); + validate_target_owner_for_copy_to_source( + abs_tgt, + rel_path, + *group_index, + config, + failed_checks, + ); + validate_copy_to_source_user(&config.config_path, failed_checks); } Change::DeleteTarget { abs_tgt, @@ -742,9 +827,120 @@ fn check_state_writable(failed_checks: &mut Vec, config: &ResolvedConfig } } +fn validate_target_perms_for_copy_to_source( + abs_tgt: &Path, + rel_path: &str, + group_index: usize, + config: &ResolvedConfig, + failed_checks: &mut Vec, +) { + use std::os::unix::fs::PermissionsExt; + let Ok(metadata) = std::fs::symlink_metadata(abs_tgt) else { + return; + }; + if metadata.file_type().is_symlink() { + return; + } + let actual_mode = metadata.permissions().mode() & 0o777; + + let group = &config.sync_groups[group_index]; + let glob = crate::sync::find_matching_glob(group, rel_path); + + if let Some(glob_entry) = glob + && let Some(ref preset) = glob_entry.file_perms + { + let reversed = preset.reverse_map_permissions(actual_mode); + let expected = preset.map_permissions(reversed); + if actual_mode != expected { + failed_checks.push(format!( + "target file '{}' has permissions {:o}, expected {:o}", + rel_path, actual_mode, expected + )); + return; + } + } + + if actual_mode != 0o644 && actual_mode != 0o755 { + failed_checks.push(format!( + "target file '{}' has permissions {:o}, must be 644 or 755 when no file_perms is configured", + rel_path, actual_mode + )); + } +} + +fn validate_target_owner_for_copy_to_source( + abs_tgt: &Path, + rel_path: &str, + group_index: usize, + config: &ResolvedConfig, + failed_checks: &mut Vec, +) { + let group = &config.sync_groups[group_index]; + let glob = crate::sync::find_matching_glob(group, rel_path); + + let expected_owner = if let Some(glob_entry) = glob + && let Some(ref owner_spec) = glob_entry.owner + { + owner_spec.clone() + } else if let Some(ref owner_spec) = group.owner { + owner_spec.clone() + } else { + format_owner_from_file_metadata(&config.config_path) + }; + + let actual_owner = format_owner_from_file_metadata(abs_tgt); + if actual_owner != expected_owner { + failed_checks.push(format!( + "target file '{}' is owned by {}, expected '{}'", + rel_path, actual_owner, expected_owner + )); + } +} + +fn format_owner_from_file_metadata(path: &Path) -> String { + use std::os::unix::fs::MetadataExt; + let Ok(metadata) = std::fs::symlink_metadata(path) else { + return String::new(); + }; + let uid = metadata.uid(); + let gid = metadata.gid(); + let user = nix::unistd::User::from_uid(nix::unistd::Uid::from_raw(uid)) + .ok() + .flatten() + .map(|u| u.name) + .unwrap_or_else(|| uid.to_string()); + let group = nix::unistd::Group::from_gid(nix::unistd::Gid::from_raw(gid)) + .ok() + .flatten() + .map(|g| g.name) + .unwrap_or_else(|| gid.to_string()); + format!("{}:{}", user, group) +} + +fn validate_copy_to_source_user(config_path: &Path, failed_checks: &mut Vec) { + use std::os::unix::fs::MetadataExt; + if !crate::sync::is_root() { + let Ok(config_meta) = std::fs::metadata(config_path) else { + return; + }; + let current_uid = nix::unistd::Uid::current().as_raw(); + let config_uid = config_meta.uid(); + if current_uid != config_uid { + failed_checks.push(format!( + "cannot copy to source: must run as root or as the config file owner (uid {})", + config_uid + )); + } + } +} + pub fn count_changes(changes: &[Change]) -> ChangeCounts { let mut counts = ChangeCounts::default(); for change in changes { + if !change.failed_checks().is_empty() { + counts.failed += 1; + continue; + } match change { Change::CopyToTarget { .. } => counts.copy_to_target += 1, Change::CopyToSource { .. } => counts.copy_to_source += 1, diff --git a/src/sync.rs b/src/sync.rs index 73b5bb1..392fcc4 100644 --- a/src/sync.rs +++ b/src/sync.rs @@ -802,7 +802,7 @@ fn validate_target_for_copy_to_source( Ok(()) } -fn find_matching_glob<'a>( +pub fn find_matching_glob<'a>( group: &'a crate::config::ResolvedSyncGroup, rel_path: &str, ) -> Option<&'a crate::config::ResolvedGlob> { @@ -1000,7 +1000,7 @@ fn chown_state_file(state_path: &Path, config_path: &Path) { let _ = nix::unistd::chown(state_path, Some(uid), Some(gid)); } -fn is_root() -> bool { +pub fn is_root() -> bool { unsafe { nix::libc::geteuid() == 0 } }