Create the service unit directory owner-only instead of refusing it

README.md

@@ -919,7 +919,7 @@ That message names the wrong cause — the same text appears for a missing proje
 |---|---|---|
 | `GOOGLE_CLOUD_PROJECT`, `GOOGLE_CLOUD_PROJECT_ID`, `GOOGLE_CLOUD_LOCATION`, `GOOGLE_GENAI_USE_VERTEXAI`, `GOOGLE_GENAI_USE_GCA` | `GOOGLE_APPLICATION_CREDENTIALS`, `GOOGLE_GEMINI_BASE_URL`, `GOOGLE_VERTEX_BASE_URL` | `GOOGLE_API_KEY`, `GOOGLE_CREDENTIALS` |
 
-The middle column is secret-*capable* — a credential path says where your credential lives, and a base URL can carry a token in userinfo or a query string. On macOS and Linux that is bounded by a guarantee kcap enforces: unit files are written `0600`, the mode is re-checked on the open handle, and `install` refuses a group- or world-writable directory. On Windows the wrapper inherits your user profile's ACL, which kcap neither sets nor verifies, so those three are excluded there — the same reason `GH_TOKEN` is never carried. If you need Vertex-with-ADC on a Windows daemon, set it in the service's own environment yourself.
+The middle column is secret-*capable* — a credential path says where your credential lives, and a base URL can carry a token in userinfo or a query string. On macOS and Linux that is bounded by a guarantee kcap enforces: unit files are written `0600`, the mode is re-checked on the open handle, and the unit directory is created `0700`. A unit directory that already grants group or world write has those bits removed; `install` refuses only when they cannot be removed, which means the directory belongs to another account. On Windows the wrapper inherits your user profile's ACL, which kcap neither sets nor verifies, so those three are excluded there — the same reason `GH_TOKEN` is never carried. If you need Vertex-with-ADC on a Windows daemon, set it in the service's own environment yourself.
 
 ⚠️ **Capture happens at install time.** Exporting the project *after* `kcap daemon service install` leaves a unit without it. Set it first, or re-run `install` afterwards — and restart the daemon.
 

src/Capacitor.Cli/Services/LaunchdServiceManager.cs

@@ -27,10 +27,8 @@ sealed partial class LaunchdServiceManager(
 
     /// <summary>The unit-writing half of <see cref="Install"/>, split out so it is testable without
     /// invoking launchctl.</summary>
-    internal void WriteUnitFiles(ServiceSpec spec) {
-        Directory.CreateDirectory(LaunchdUnit.AgentsDir(home));
+    internal void WriteUnitFiles(ServiceSpec spec) =>
         _writeUnit(LaunchdUnit.PlistPath(home, spec.ServiceId), LaunchdUnit.Plist(spec), null);
-    }
 
     [LibraryImport("libc", EntryPoint = "getuid")]
     private static partial uint getuid();

src/Capacitor.Cli/Services/ServiceFiles.cs

@@ -4,7 +4,9 @@
 namespace Capacitor.Cli.Services;
 
 static class ServiceFiles {
-    const UnixFileMode OwnerOnly = UnixFileMode.UserRead | UnixFileMode.UserWrite;
+    const UnixFileMode OwnerOnly    = UnixFileMode.UserRead | UnixFileMode.UserWrite;
+    const UnixFileMode OwnerOnlyDir = OwnerOnly | UnixFileMode.UserExecute;
+    const UnixFileMode SharedWrite  = UnixFileMode.GroupWrite | UnixFileMode.OtherWrite;
 
     /// <summary>Writes a service unit readable only by its owner, or fails without leaving one behind.
     ///
@@ -23,13 +25,17 @@ static class ServiceFiles {
     /// <param name="verifyFinal">Test seam. Production passes null and gets the real post-rename check;
     /// a test supplies a failing one to prove the rollback, which is otherwise only reachable on a
     /// filesystem that does not preserve mode across a rename.</param>
+    /// <param name="tightenDirectory">Test seam. Production passes null and gets the real chmod; a test
+    /// supplies a no-op one to prove the refusal, which is otherwise only reachable through a directory
+    /// the current user does not own.</param>
     public static void WriteOwnerOnly(
-            string path, string content, Encoding? encoding = null, Action<string>? verifyFinal = null) {
+            string path, string content, Encoding? encoding = null, Action<string>? verifyFinal = null,
+            Action<string>? tightenDirectory = null) {
         var directory = Path.GetDirectoryName(path);
 
         if (!string.IsNullOrEmpty(directory)) {
-            Directory.CreateDirectory(directory);
-            RequireNotWorldWritable(directory);
+            CreateDirectory(directory);
+            RequireNoSharedWrite(directory, tightenDirectory);
         }
 
         // Full GUID: the staging name must not be guessable by a local process racing to pre-create it,
@@ -73,18 +79,63 @@ static void WriteStaging(string staging, string content, Encoding? encoding) {
         writer.Write(content);
     }
 
-    /// <summary>Refuses to write a unit into a directory other local accounts can write — owner-only mode
-    /// on the unit is no protection when someone else can replace the unit and choose what the daemon
-    /// runs.</summary>
-    static void RequireNotWorldWritable(string directory) {
+    /// <summary>Creates the unit directory, and every ancestor it has to create, owner-only — so the umask
+    /// cannot decide who may replace a unit.
+    ///
+    /// <para>The plain overload applies <c>0777 &amp; ~umask</c>, and umask 002 is the default wherever a
+    /// user's primary group is their own name — the <c>pam_umask</c> usergroups behaviour Debian and Ubuntu
+    /// enable. That yields a group-writable directory, which <see cref="RequireNoSharedWrite"/> then has to
+    /// repair; asking for the mode at creation leaves no window in which it is wrong.</para>
+    ///
+    /// <para>One level at a time because the mode-taking overload applies it to the LEAF only: every
+    /// ancestor it creates on the way still lands <c>0777 &amp; ~umask</c>, and a writable parent is a
+    /// rename away from replacing the unit directory whole. Ancestors that already exist are left as the
+    /// operator has them — <c>~/.config</c> is not this code's to tighten.</para>
+    ///
+    /// <para>Then chmod'd, because the requested mode is filtered through the umask exactly like the
+    /// default one: it is a request, not a result. A restrictive umask strips the OWNER bits — under
+    /// umask 077 the directory lands <c>0700</c>, but under umask 400 it lands <c>0300</c>, which the unit
+    /// can still be written into and which <c>ListInstalled</c> then cannot enumerate. An explicit chmod
+    /// is not filtered.</para></summary>
+    static void CreateDirectory(string directory) {
+        if (OperatingSystem.IsWindows()) { Directory.CreateDirectory(directory); return; }
+
+        var missing = new Stack<string>();
+
+        for (var d = directory; !string.IsNullOrEmpty(d) && !Directory.Exists(d); d = Path.GetDirectoryName(d)!)
+            missing.Push(d);
+
+        while (missing.Count > 0) {
+            var d = missing.Pop();
+
+            Directory.CreateDirectory(d, OwnerOnlyDir);
+            File.SetUnixFileMode(d, OwnerOnlyDir);
+        }
+    }
+
+    /// <summary>Strips group and world write from the unit directory, and refuses the install if they
+    /// survive — owner-only mode on the unit is no protection when someone else can replace the unit and
+    /// choose what the daemon runs.
+    ///
+    /// <para>Repaired rather than refused outright, because the bits alone do not say another account is
+    /// involved: a user-private group has exactly one member, and a directory we can chmod is one no other
+    /// account controls. A chmod that fails is the case worth refusing, and the re-read is what decides —
+    /// not the attempt.</para></summary>
+    static void RequireNoSharedWrite(string directory, Action<string>? tighten) {
         if (OperatingSystem.IsWindows()) return;   // ACL-governed, inherited from the user profile
 
-        var mode = File.GetUnixFileMode(directory);
+        if ((File.GetUnixFileMode(directory) & SharedWrite) == 0) return;
+
+        try {
+            if (tighten is not null) tighten(directory);
+            else File.SetUnixFileMode(directory, File.GetUnixFileMode(directory) & ~SharedWrite);
+        } catch (Exception) { /* the re-read decides, not the attempt */ }
 
-        if (mode.HasFlag(UnixFileMode.OtherWrite) || mode.HasFlag(UnixFileMode.GroupWrite))
+        if ((File.GetUnixFileMode(directory) & SharedWrite) != 0)
             throw new InvalidOperationException(
                 $"Refusing to write a service unit into a group- or world-writable directory: {directory}. "
-              + "Another local account could replace the unit and choose what the daemon runs.");
+              + "Another local account could replace the unit and choose what the daemon runs. Remove those "
+              + "write bits with `chmod g-w,o-w` and re-run the install.");
     }
 
     /// <summary>Requires EXACTLY owner read+write on the open handle, repairing once.

src/Capacitor.Cli/Services/SystemdServiceManager.cs

@@ -40,10 +40,8 @@ public ServiceQuery Query(string serviceId) {
 
     /// <summary>The unit-writing half of <see cref="Install"/>, split out so it is testable without
     /// invoking systemctl.</summary>
-    internal void WriteUnitFiles(ServiceSpec spec) {
-        Directory.CreateDirectory(SystemdUnit.UserUnitDir(home));
+    internal void WriteUnitFiles(ServiceSpec spec) =>
         _writeUnit(SystemdUnit.UnitPath(home, spec.ServiceId), SystemdUnit.Unit(spec), null);
-    }
 
     public void Install(ServiceSpec spec, bool startNow) {
         WriteUnitFiles(spec);

src/Capacitor.Cli/Services/WindowsScheduledTaskServiceManager.cs

@@ -50,7 +50,6 @@ public ServiceQuery Query(string serviceId) {
     internal IReadOnlyList<GeneratedFile> WriteUnitFiles(ServiceSpec spec) {
         var files = GenerateFiles(spec);
         foreach (var f in files) {
-            Directory.CreateDirectory(Path.GetDirectoryName(f.Path)!);
             // schtasks /XML wants UTF-16; the .cmd wrapper is fine as UTF-8.
             var encoding = f.Path.EndsWith(".task.xml", StringComparison.Ordinal) ? Encoding.Unicode : Encoding.UTF8;
             _writeUnit(f.Path, f.Content, encoding);

test/Capacitor.Cli.Tests.Unit/Services/ServiceFilesTests.cs

@@ -121,11 +121,75 @@ await Assert.That(Directory.GetFiles(tmp.Path)).IsEmpty()
             .Because("the staging file must not survive either");
     }
 
-    /// <summary>Installing into a directory other local accounts can write is refused: owner-only mode on
-    /// the unit is no protection if someone else can replace the unit and choose what the daemon runs.</summary>
+    /// <summary>A directory the writer creates itself is usable, whatever the umask.
+    ///
+    /// <para>umask 002 is the default for a user whose primary group matches their own name — the
+    /// <c>pam_umask</c> usergroups behaviour Debian and Ubuntu enable — so a directory created under it
+    /// lands 0775. The writer creates the unit directory, so a check that rejects group-write outright
+    /// rejects the writer's own work and no install can succeed on those distributions.</para></summary>
+    [Test]
+    [NotInParallel]
+    [Arguments(0x2u)]      // umask 002 — group-writable
+    [Arguments(0u)]        // umask 000 — group- and world-writable
+    [Arguments(0x100u)]    // umask 400 — strips owner READ, leaving a directory nothing can enumerate
+    [Arguments(0x1FFu)]    // umask 777 — strips every bit, leaving one nothing can be written into
+    [UnsupportedOSPlatform("windows")]
+    public async Task WriteOwnerOnly_creates_a_usable_unit_directory_under_any_umask(uint mask) {
+        Skip.When(OperatingSystem.IsWindows(), "POSIX file modes");
+
+        using var tmp = new TempDir();
+        var path     = tmp.PathTo("systemd", "user", "kcap-daemon-test.service");
+        var dir      = tmp.PathTo("systemd", "user");
+        var previous = umask(mask);
+        try {
+            ServiceFiles.WriteOwnerOnly(path, "SECRET-COMMAND");
+        } finally {
+            _ = umask(previous);
+        }
+
+        await Assert.That(await File.ReadAllTextAsync(path)).IsEqualTo("SECRET-COMMAND");
+
+        // EXACTLY owner rwx, both directions. Too permissive lets another account replace the unit; too
+        // restrictive is the opposite failure — the requested mode is filtered through the umask like any
+        // other, so the owner's own read or write bit can be stripped, and a directory the owner cannot
+        // enumerate is one `service list` fails on after an install that reported success.
+        await Assert.That(File.GetUnixFileMode(dir)).IsEqualTo(OwnerOnlyDir);
+        await Assert.That(File.GetUnixFileMode(tmp.PathTo("systemd"))).IsEqualTo(OwnerOnlyDir)
+            .Because("a writable parent is a rename away from replacing the whole unit directory");
+    }
+
+    /// <summary>A directory that arrives group- or world-writable is tightened rather than refused — the
+    /// write bits for other accounts are what the check is about, and on a directory we own they can simply
+    /// be removed. The unit still lands, and the directory no longer grants anyone else write.</summary>
+    [Test]
+    [UnsupportedOSPlatform("windows")]
+    public async Task WriteOwnerOnly_tightens_a_shared_writable_directory_it_can_repair() {
+        Skip.When(OperatingSystem.IsWindows(), "POSIX file modes");
+
+        using var tmp = new TempDir();
+        File.SetUnixFileMode(tmp.Path,
+            UnixFileMode.UserRead   | UnixFileMode.UserWrite   | UnixFileMode.UserExecute |
+            UnixFileMode.GroupRead  | UnixFileMode.GroupWrite  | UnixFileMode.GroupExecute |
+            UnixFileMode.OtherRead  | UnixFileMode.OtherWrite  | UnixFileMode.OtherExecute);
+
+        ServiceFiles.WriteOwnerOnly(tmp.PathTo("unit.plist"), "SECRET-COMMAND");
+
+        await Assert.That(await File.ReadAllTextAsync(tmp.PathTo("unit.plist"))).IsEqualTo("SECRET-COMMAND");
+        await Assert.That(File.GetUnixFileMode(tmp.Path) & SharedWrite).IsEqualTo(default(UnixFileMode));
+        await Assert.That(File.GetUnixFileMode(tmp.Path).HasFlag(UnixFileMode.OtherRead)).IsTrue()
+            .Because("only the write bits are the hazard; read and traverse are left as the operator set them");
+    }
+
+    /// <summary>When the write bits cannot be removed, the install is refused and no unit is left behind:
+    /// owner-only mode on the unit is no protection if someone else can replace the unit and choose what
+    /// the daemon runs.
+    ///
+    /// <para>The repair is suppressed rather than provoked. Reaching this for real needs a directory the
+    /// current user does not own, and the only ones a test could rely on finding are shared system
+    /// directories that a run as root would then really chmod.</para></summary>
     [Test]
     [UnsupportedOSPlatform("windows")]
-    public async Task WriteOwnerOnly_refuses_a_world_writable_directory() {
+    public async Task WriteOwnerOnly_refuses_a_shared_writable_directory_it_cannot_repair() {
         Skip.When(OperatingSystem.IsWindows(), "POSIX file modes");
 
         using var tmp = new TempDir();
@@ -135,7 +199,8 @@ public async Task WriteOwnerOnly_refuses_a_world_writable_directory() {
                 UnixFileMode.OtherRead | UnixFileMode.OtherWrite | UnixFileMode.OtherExecute);
 
             var ex = Assert.Throws<InvalidOperationException>(
-                () => ServiceFiles.WriteOwnerOnly(tmp.PathTo("unit.plist"), "x"));
+                () => ServiceFiles.WriteOwnerOnly(tmp.PathTo("unit.plist"), "x", null,
+                    tightenDirectory: _ => { }));
 
             await Assert.That(ex!.Message).Contains("writable");
             await Assert.That(File.Exists(tmp.PathTo("unit.plist"))).IsFalse();
@@ -148,6 +213,9 @@ public async Task WriteOwnerOnly_refuses_a_world_writable_directory() {
         }
     }
 
+    const UnixFileMode SharedWrite  = UnixFileMode.GroupWrite | UnixFileMode.OtherWrite;
+    const UnixFileMode OwnerOnlyDir = UnixFileMode.UserRead | UnixFileMode.UserWrite | UnixFileMode.UserExecute;
+
     /// <summary>A pre-existing entry at the staging path is not followed or truncated — the staging inode
     /// is created exclusively. The name carries a full GUID, so this asserts the mechanism rather than a
     /// realistic collision.</summary>