pwnshop: add support for runtime nix packages

[zardus]

Adds --nix-packages support to pwnshop run.

[Copilot]

Pull request overview

This PR adds a --nix-packages option to the pwnshop CLI tool, allowing users to inject Nix packages into challenge containers for debugging and testing purposes. The implementation resolves package names using nix build, mounts the Nix store read-only, and adds package binaries to the container's PATH.

Changes:

• Added resolve_nix_packages() function to resolve Nix package names to store paths
• Modified run_challenge() to accept nix_paths parameter and configure PATH and volume mounts accordingly
• Added --nix-packages CLI option to the run command
• Updated AGENTS.md documentation with usage examples

Reviewed changes

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

File	Description
tools/pwnshop/src/pwnshop/commands/run.py	Adds resolve_nix_packages function and --nix-packages CLI option to resolve and pass nix package paths
tools/pwnshop/src/pwnshop/lib/init.py	Extends run_challenge to accept nix_paths, build PATH with nix binaries, and mount /nix/store
AGENTS.md	Adds usage examples for --nix-packages option and removes unrelated "Porting Legacy Challenges" section

---

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

[Copilot]

When nix_paths are provided, the code mounts /nix/store from the host into the container. However, if the host doesn't have /nix/store (e.g., if Nix isn't properly installed), Docker will either fail to mount or create an empty directory. The resolved nix package paths will then be missing in the container, causing commands to fail. Consider checking if /nix/store exists on the host before attempting to mount it, and providing a clear error message if it doesn't exist.

Suggested change

	volume_mounts.append("--volume=/nix/store:/nix/store:ro")
	nix_store_path = "/nix/store"
	if not os.path.isdir(nix_store_path):
	msg = (
	f"nix_paths were provided but '{nix_store_path}' does not exist on the host. "
	"Ensure Nix is installed and that /nix/store is available before requesting nix packages."
	)
	logger.error(msg)
	raise RuntimeError(msg)
	volume_mounts.append(f"--volume={nix_store_path}:{nix_store_path}:ro")

[Copilot]

This PR removes the entire "Porting Legacy Challenges" section (approximately 20 lines) from AGENTS.md. This removal appears unrelated to the PR's stated purpose of adding a --nix-packages option. If this deletion is intentional, it should be mentioned in the PR description. If it's accidental, it should be reverted.

[Copilot]

The bin_path.exists() check at line 151 may fail silently if the nix package doesn't have a bin directory. While this is handled gracefully (the path simply won't be added to PATH), it would be helpful to log a debug or warning message when a nix package is resolved but doesn't have a bin directory, as this might indicate an issue with the package or unexpected package structure.

Suggested change

	path_components.append(str(bin_path))
	path_components.append(str(bin_path))
	else:
	logger.debug("nix path %s has no 'bin' directory; skipping", nix_path)

[Copilot]

The nix package resolution uses subprocess.run with shell=False (implicit default) and constructs the command as a list, which is good for preventing command injection. However, the package names are taken directly from user input without validation. While the nix command itself will validate the package names, consider adding basic validation to reject package names containing suspicious characters (like shell metacharacters or path traversal sequences) before passing them to nix build, to provide earlier and clearer error messages to users.

[Copilot]

When multiple nix packages are specified and one fails to resolve, the function will raise an exception and stop processing remaining packages. Consider whether this is the desired behavior, or if you want to collect all errors and report them together, or continue processing and warn about failed packages. The current fail-fast behavior is reasonable but may frustrate users who specify multiple packages where only one has a typo.

[Copilot]

The subprocess.run call for nix build has no timeout parameter. Since nix build may need to download packages from the network, this could potentially hang indefinitely if there are network issues. Consider adding a reasonable timeout (e.g., 300 seconds) to prevent the command from hanging indefinitely. You can catch subprocess.TimeoutExpired and provide a helpful error message.

[ConnorNelson]

@zardus Can you further explain what the goal here is?

At first glance, this UX feels very awkward. My instinct is that rather than adhoc requesting tools like curl for some particular instantiation of a challenge, we probably just have a large collection of tools, that is made available to all challenges (similar to the dojo). But even this, I'm not sure how much we want that right now. I think I'm still convinced we'll eventually move to a model where the entire challenge set lives in nix, and a test script can reach for some tool in a nix-first way (rather than Dockerfiles being required to track all the dependencies for both the challenges and tests). But maybe you have pressing concerns right now for what you want to do?

Regarding agentic workflows, I think it makes much more sense to just docker cp assets out of the running container. If suddenly an agent wants to reach for objdump, it should not be required to fully restart the challenge (it might currently have the challenge in some currently complex state).

[zardus]

So far, agentically, this seems to be working pretty well. It doesn't seem to me that the agents have problems restarting things and getting programs back into relevant states.

Longer-term, even if we have a default set of tools, it feels like it could be good to have a way to extend that easily and temporarily. And even if we have a default set of tools, the whole mounting /nix would still be applicable. IMO, we should merge this for now and then deprecate it if we have a more general solution.

[ConnorNelson]

I'm having codex rip out the unrelated changes into separate PRs (it's easy now to keep PRs clean, and I think it might be helpful for future agents crawling through PR discussions).

---

Sure, we can always change our minds.

Once use-case that isn't clear is tests though. I understand we can use this to quickly prototype solutions / analyze / etc. But how does CI deal with this?

My testcase wants python, but the base image doesn't have python. Am I just expected to put python in the image to make it work? Or is there room for nix to play a role here? Any current thoughts on this?

[zardus]

Interesting... Two thoughts:

1. tests_private has its own Dockerfile (or Dockerfile.j2 that can extend the challenge's)
2. challenge.yml specifies test requirements

I actually like the idea of test requirements being injected through nix (option 2), as it better reflects the dojo workspace.

[ConnorNelson]

I think I agree with option 2 (or something similar), but then do you also agree this means that nix is a hard dependency for this monorepo? Just as docker is? I am of course in favor of that, but just want to make sure we're on the same page about it. Alternatively it could technically be a soft dependency, but some tests will need to be skipped (I think I'd prefer the hard dependency route, at least to start).

[zardus]

We don't necessarily need to make it hard dependency in terms of it being installed on the system. I think medium-term, I'd prefer to have pwnshop initialize a pwnshop-nix docker volume or something, and use that, so I don't have to have this on my system :-)