bunpen: wait for xdg-dbus-proxy to be ready before continuing execution

pkgs/by-name/bunpen/integration_test

@@ -286,6 +286,8 @@ test_11_dbus_02_doesnt_bridge_bus_by_default() {
   mkdir -p $PWD/dbus
   dbus-daemon --address="$DBUS_SESSION_BUS_ADDRESS" --config-file $PWD/test/dbus-session.conf --fork
   ! bunpen --bunpen-path /nix/store busctl --user list
+  # but still reachable from the outside
+  busctl --user list
 }
 
 test_11_dbus_03_doesnt_spawn_bus() {
@@ -293,13 +295,15 @@ test_11_dbus_03_doesnt_spawn_bus() {
   ! bunpen --bunpen-path /nix/store --bunpen-path $PWD busctl --user list
 }
 
-DISABLED_TODO_FIX_test_11_dbus_04_proxy_all() {
+test_11_dbus_04_proxy_nonexistent() {
   export DBUS_SESSION_BUS_ADDRESS=unix:path=$PWD/dbus/bus
   mkdir -p $PWD/dbus
   dbus-daemon --address="$DBUS_SESSION_BUS_ADDRESS" --config-file $PWD/test/dbus-session.conf --fork
-  bunpen --bunpen-path /nix/store --bunpen-dbus-talk '*' busctl --user list
+  # parameters *have* to be "valid" dbus names, i.e. a-z with dot separators, at least multiple components.
-  bunpen --bunpen-path /nix/store --bunpen-dbus-own '*' busctl --user list
+  # they don't have to be actual, existing names
-  bunpen --bunpen-path /nix/store --bunpen-dbus-talk '*' --bunpen-dbus-own '*' busctl --user list
+  bunpen --bunpen-path /nix/store --bunpen-dbus-talk 'does.not.exist' busctl --user list
+  bunpen --bunpen-path /nix/store --bunpen-dbus-own 'does.not.exist' busctl --user list
+  bunpen --bunpen-path /nix/store --bunpen-dbus-talk 'does.not.exist' --bunpen-dbus-own 'also.not.exist' busctl --user list
 }
 
 runTests() {

pkgs/by-name/bunpen/restrict/dbus_proxy.ha

@@ -61,6 +61,7 @@
 use config;
 use errors;
 use errors::ext;
+use fmt;
 use fs;
 use io;
 use log;
@@ -72,6 +73,7 @@ use resources;
 use rt;
 use rt::ext;
 use strings;
+use unix;
 
 // given an existing, unfiltered bus at the fs path `upstream`,
 // proxy the bus into the fs path indicated by `downstream`.
@@ -84,26 +86,38 @@ fn dbus_restrict(dbus: resources::dbus_subset, upstream_parent_fd: io::file, dow
     dbus_ensure_bus_dir(downstream_path),
     path::string(downstream_path),
   );
+
   // TODO: restrict resources to the xdg-dbus-proxy instance
+
+  // pipe by which child xdg-dbus-proxy can notify the parent of readiness
+  let (pipe_parent_rd, pipe_child_wr) = unix::pipe()!;
   match (ps::fork_and_die_with_parent()) {
     case void =>
-      // dup the upstream to give it a predictable fd
+      // close the pipe ends which aren't ours
+      io::close(pipe_parent_rd)!;
+
+      // move the fd's to be one of the special fd's which *don't*
+      // get closed on exec (e.g. /proc/self/fd/0, stdin).
+      // assumed necessary based on pasta; *might not* actually be required here.
       rt::dup2(upstream_parent_fd, 0)!;
       io::close(upstream_parent_fd)!;
-      os::setenv("DBUS_SESSION_BUS_ADDRESS", "path:unix=/proc/self/fd/0/bus")!;  // TODO: pass this to execvpe's env instead of setenv'ing it here. TODO: don't hardcode "bus", but take it from `basename(downstream_path)`
+      let upstream_fd_path = fmt::asprintf("unix:path=/proc/self/fd/0/{}", path::basename(path::string(downstream_path)));
+
+      rt::dup2(pipe_child_wr, 1)!;
+      io::close(pipe_child_wr)!;
 
       let proxy_args: []str = [];
       append(proxy_args, "xdg-dbus-proxy");
+      append(proxy_args, upstream_fd_path);
       append(proxy_args, path::string(downstream_path));
       append(proxy_args, "--filter");
+      append(proxy_args, "--fd=1");
       // append(proxy_args, "--log");
       for (let spec .. dbus.talk) {
-        append(proxy_args, "--talk");
+        append(proxy_args, fmt::asprintf("--talk={}", spec));
-        append(proxy_args, spec);
       };
       for (let spec .. dbus.own) {
-        append(proxy_args, "--own");
+        append(proxy_args, fmt::asprintf("--own={}", spec));
-        append(proxy_args, spec);
       };
       errors::ext::check(
         "dbus_restrict: invoke xdg-dbus-proxy",
@@ -115,15 +129,22 @@ fn dbus_restrict(dbus: resources::dbus_subset, upstream_parent_fd: io::file, dow
         ),
       );
     case let child_pid: os::exec::process =>
-      // xdg-dbus-proxy provides no explicit synchronization mechanism;
+      // close the pipe ends which aren't ours
-      // we have to assume that once we fork, it starts listening "reasonably quickly".
+      io::close(pipe_child_wr)!;
-      // TODO: i could probably hack some synchronization in here:
+
+      // wait for the proxy to signal readiness
+      io::readall(pipe_parent_rd, &[0u8])!;
+
+      // XXX: DON'T close the pipe: xdg-dbus-proxy docs say it will exit when
+      // the fd is closed.
+      // TODO: make sure this implementation is compatible with non-pid
+      // namespaced invocations! if it isn't, consider alternative synchronizations:
       // - `write` a HELLO message and block until it's read.
       // - invoke with `--log` and parse the output.
-      // - or maybe pre-create the unix socket _before_ forking here, and
+      // - pre-create the unix socket _before_ forking here, and somehow pass
-      //   somehow pass that into xdg-dbus-proxy instead of letting it create
+      //   that into xdg-dbus-proxy instead of letting it create its own socket.
-      //   its own socket.
+      //
-      void;
+      // io::close(pipe_parent_rd)!;
   };
 };