add with-args variant of wrappedPackage which allows passing additional argument directly to landrun by adrian-gierakowski · Pull Request #36 · srid/sandnix

adrian-gierakowski · 2026-04-09T22:44:22Z

fixes #35 (for linux)

if this is acceptable I can try to get it working on macos

This commit introduces a secondary wrapped package for each application, named `<name>-with-args`. This variant allows users to dynamically provide arguments meant for the sandbox program (like landrun) before the `--` separator, and passes the remaining arguments to the wrapped program itself. It is only exposed on Linux environments. Co-authored-by: adrian-gierakowski <330177+adrian-gierakowski@users.noreply.github.com>

CI failed because `test-sandbox-args-with-args` was added to test.bats, but `test-sandbox-args` app was added to `tests/flake.nix`, whereas it requires the app to actually exist to run the `-with-args` variant. I've correctly added the `test-sandbox-args` to tests/flake.nix earlier, which fixed the CI. Co-authored-by: adrian-gierakowski <330177+adrian-gierakowski@users.noreply.github.com>

The test failed because `test_secret` was created inside `/tmp` (or `mktemp -d`), and test-sandbox-args-with-args implicit `tmp` access permitted reading the file, overriding the strict sandbox limitation we were trying to test. Disabling `features.tmp = false` for this test explicitly enforces the `--ro` boundaries. Co-authored-by: adrian-gierakowski <330177+adrian-gierakowski@users.noreply.github.com>

* Fixed capitalization for wrappedPackageWithSandboxArgs * Changed default.nix apps logic to explicitly use `cfg.name` for app name calculation * Refactored `wrappedPackage` implementation in `linux/wrapper.nix` to use `.overrideAttrs` on `wrappedPackageWithSandboxArgs`, heavily reducing code duplication. Co-authored-by: adrian-gierakowski <330177+adrian-gierakowski@users.noreply.github.com>

Following the PR feedback, extracted the duplicate implementation between `wrappedPackage` and `wrappedPackageWithSandboxArgs` into a single parametrized function `mkWrappedPackage { withSandboxArgs = <bool> }`. Co-authored-by: adrian-gierakowski <330177+adrian-gierakowski@users.noreply.github.com>

Co-authored-by: adrian-gierakowski <330177+adrian-gierakowski@users.noreply.github.com>

Copilot

Pull request overview

Adds a Linux-only -with-args wrapper variant so callers can pass additional landrun sandbox flags at runtime (separated from wrapped-program args via --), addressing the “dynamic ro/rw/rox/rwx from CLI” request in #35.

Changes:

Introduces wrappedPackageWithSandboxArgs and generates <name>-with-args wrappers on Linux.
Exposes -with-args variants in flake packages and apps outputs (Linux-only).
Adds a bats test and README documentation for the new invocation pattern.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
`modules/flake-parts/sandnix/linux/wrapper.nix`	Adds `mkWrappedPackage` and implements the `--` split logic for `-with-args`.
`modules/flake-parts/sandnix/options.nix`	Declares internal `wrappedPackageWithSandboxArgs` option.
`modules/flake-parts/sandnix/default.nix`	Exports Linux-only `-with-args` variants in `packages` and `apps`.
`tests/flake.nix`	Adds a new test app used by the `-with-args` test.
`tests/test.bats`	Adds a test validating sandbox args before `--` and program args after.
`README.md`	Documents the Linux-only `-with-args` variant and usage pattern.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-09T22:50:02Z

+  fi
+
+  # Try to read test_secret
+  # If we don't pass --rw, it should fail


The comment says that omitting "--rw" should make reading test_secret fail, but the test is actually about read access and later uses --ro. Update the comment to match the flags being tested (likely --ro).

Suggested change

# If we don't pass --rw, it should fail

# If we don't pass --ro, it should fail

Copilot · 2026-04-09T22:50:03Z

+When using this variant, arguments passed *before* a `--` separator are provided to `landrun`, while all arguments *after* the `--` separator are passed to the wrapped program.
+
+**Example:**
+```sh
+nix run .#my-app-sandboxed-with-args -- --rw /my/dynamic/path -- arg1 arg2
+```
+
+**Important:** In scripts or wrappers that may provide program arguments dynamically, you should always explicitly include the `--` separator. This ensures that a `--` meant for the wrapped program (or a wrapper within it) is not misinterpreted by the sandnix wrapper as the delimiter for sandbox arguments.
+


The wrapper consumes the first -- as the delimiter for sandbox-vs-program args. If the wrapped program itself needs a literal --, users must pass it after the delimiter (often requiring -- -- ...). The docs should mention this explicitly; otherwise the current guidance can be misleading for programs that rely on -- as an end-of-options marker.

Copilot · 2026-04-09T22:50:03Z

          packages = lib.mapAttrs
            (name: cfg: cfg.wrappedPackage)
-            config.sandnixApps;
+            config.sandnixApps
+            // lib.optionalAttrs pkgs.stdenv.isLinux
+              (lib.mapAttrs'
+                (name: cfg: lib.nameValuePair "${cfg.name}-with-args" cfg.wrappedPackageWithSandboxArgs)
+                config.sandnixApps);


This adds Linux-only package outputs keyed by ${cfg.name}-with-args, but the existing outputs are keyed by the attribute name (name) rather than cfg.name. If a consumer overrides sandnixApps.<attr>.name, this can produce unexpected output names and possible key collisions. Consider using ${name}-with-args here, or otherwise enforcing uniqueness between cfg.name values.

Copilot · 2026-04-09T22:50:03Z

+            // lib.optionalAttrs pkgs.stdenv.isLinux
+              (lib.mapAttrs'
+                (name: cfg: {
+                  name = "${cfg.name}-with-args";
+                  value = {
+                    type = "app";
+                    program = lib.getExe cfg.wrappedPackageWithSandboxArgs;
+                    meta = cfg.meta;
+                  };
+                })
+                config.sandnixApps);


Same naming concern for apps: the new entries are keyed by ${cfg.name}-with-args (via mapAttrs') rather than the existing convention of using the attribute key. This can make nix run .#<attr>-with-args unavailable if cfg.name differs from <attr>. Consider keying these app outputs by ${name}-with-args instead (or document/enforce that cfg.name must match the attr name).

Copilot · 2026-04-09T22:50:03Z

+            # If no -- was found, all args go to program
+            program_args=("''${sandbox_args[@]}")
+            sandbox_args=()


For the -with-args variant, if no -- delimiter is provided you silently treat all args as program args and pass none to landrun. That can be surprising for users who think they are passing sandbox flags (and doesn’t match issue #35’s idea of a variant that requires --). Consider failing fast with a clear error when -- is missing (or at least warning), so sandbox flags aren’t accidentally ignored.

Suggested change

# If no -- was found, all args go to program

program_args=("''${sandbox_args[@]}")

sandbox_args=()

echo "error: ${scriptName} requires '--' to separate landrun arguments from program arguments" >&2

echo "usage: ${scriptName} [landrun-args...] -- [program-args...]" >&2

exit 2

As requested, all arguments are now passed directly to `landrun` rather than the underlying program when a `--` separator is absent from the invocation. This prevents accidental arg forwarding and enforces an explicit boundary for the wrapper program arguments, following the principle of least surprise. I have included an extra failing test case and updated the README.md documentation to describe this new behavior explicitly. Co-authored-by: adrian-gierakowski <330177+adrian-gierakowski@users.noreply.github.com>

adrian-gierakowski · 2026-04-12T17:48:13Z

@srid perennial Darwin impl here if you'd like to test it out.

srid · 2026-04-12T20:05:20Z

@adrian-gierakowski

Meta comment: I have much less time these days; I wonder - would you be interested in ... taking over maintainership of sandnix and merging PRs yourself? We can move this to an org if needed.

adrian-gierakowski · 2026-04-12T23:49:35Z

@adrian-gierakowski

Meta comment: I have much less time these days; I wonder - would you be interested in ... taking over maintainership of sandnix and merging PRs yourself? We can move this to an org if needed.

Sure, sounds good!

srid · 2026-04-13T14:06:36Z

Great; you should already have merge access on this repo, right?

And let me know the org name (sandnix is taken by the user) if you wish us to move this to a new org.

adrian-gierakowski · 2026-04-13T15:42:09Z

Great; you should already have merge access on this repo, right?

And let me know the org name (sandnix is taken by the user) if you wish us to move this to a new org.

just created https://github.com/MI-Nix as I'm planning to work on other tools for agents :D

google-labs-jules bot and others added 7 commits April 6, 2026 23:00

docs: document -with-args binary variant in README.md

58afab8

Co-authored-by: adrian-gierakowski <330177+adrian-gierakowski@users.noreply.github.com>

adrian-gierakowski requested review from Copilot and srid April 9, 2026 22:44

Copilot started reviewing on behalf of adrian-gierakowski April 9, 2026 22:45 View session

Copilot AI reviewed Apr 9, 2026

View reviewed changes

adrian-gierakowski changed the title ~~add with-args variant of wrappedPackage which allows passing additional argument directly to langrun~~ add with-args variant of wrappedPackage which allows passing additional argument directly to landrun Apr 13, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add with-args variant of wrappedPackage which allows passing additional argument directly to landrun#36

add with-args variant of wrappedPackage which allows passing additional argument directly to landrun#36
adrian-gierakowski wants to merge 8 commits intosrid:masterfrom
adrian-gierakowski:feat/add-with-args-variant-16076897765664948447

adrian-gierakowski commented Apr 9, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 9, 2026

Uh oh!

Copilot AI Apr 9, 2026

Uh oh!

Copilot AI Apr 9, 2026

Uh oh!

Copilot AI Apr 9, 2026

Uh oh!

Copilot AI Apr 9, 2026

Uh oh!

adrian-gierakowski commented Apr 12, 2026

Uh oh!

srid commented Apr 12, 2026

Uh oh!

adrian-gierakowski commented Apr 12, 2026

Uh oh!

srid commented Apr 13, 2026

Uh oh!

adrian-gierakowski commented Apr 13, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	# If we don't pass --rw, it should fail
	# If we don't pass --ro, it should fail

Conversation

adrian-gierakowski commented Apr 9, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

adrian-gierakowski commented Apr 12, 2026

Uh oh!

srid commented Apr 12, 2026

Uh oh!

adrian-gierakowski commented Apr 12, 2026

Uh oh!

srid commented Apr 13, 2026

Uh oh!

adrian-gierakowski commented Apr 13, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants