'make install' should require the caller to invoke sudo, not attempt to do so itself. #36

Closed
opened 2026-07-07 17:14:47 +01:00 by danny · 8 comments
danny commented 2026-07-07 17:14:47 +01:00 (Migrated from gitlab.keyop.co.uk)

We should remove use of 'sudo' inside the Makefile, as the caller may not be able to use sudo without passwords and the result will be counter-intuitive. Instead add some guard checks that we already have the correct privileges when privileged targets are called. We will also need to ensure the environment is sane, since 'sudo' can modify several env vars including PATH.

We should remove use of 'sudo' inside the Makefile, as the caller may not be able to use sudo without passwords and the result will be counter-intuitive. Instead add some guard checks that we already have the correct privileges when privileged targets are called. We will also need to ensure the environment is sane, since 'sudo' can modify several env vars including `PATH`.
danny commented 2026-07-07 17:16:16 +01:00 (Migrated from gitlab.keyop.co.uk)

changed the description

changed the description
claude commented 2026-07-08 16:33:24 +01:00 (Migrated from gitlab.keyop.co.uk)

Plan: both check-build-requirements and install currently self-invoke sudo (confirmed via Danny that both are in scope, not just install).

  • check-build-requirements: stop self-elevating via sudo apt-get/dnf/yum install libpam0g-dev - instead print the exact install command for the detected package manager and exit 1, leaving it to the caller to run it (with sudo or otherwise) and retry. Avoids a confusing non-interactive hang/failure for a caller without passwordless sudo, in the middle of an unrelated make build/test/lint.
  • install: drop the build prerequisite and every internal sudo prefix. Add an explicit guard requiring EUID 0 ("try: sudo make install"), and a second guard requiring dist/nfq_forwarder/dist/iptables to already exist ("run 'make build' first, as yourself"). This directly fixes the "sane environment" concern too: previously sudo make install re-ran build (goreleaser, etc.) under sudo's own environment, which commonly resets PATH (secure_path) and HOME, breaking lookups for a per-user-installed toolchain. Splitting the privileged half (install(1)/setcap, both on any reasonable secure_path) away from the build step avoids needing any PATH-preservation workaround at all.
Plan: both `check-build-requirements` and `install` currently self-invoke `sudo` (confirmed via Danny that both are in scope, not just `install`). - `check-build-requirements`: stop self-elevating via `sudo apt-get/dnf/yum install libpam0g-dev` - instead print the exact install command for the detected package manager and exit 1, leaving it to the caller to run it (with sudo or otherwise) and retry. Avoids a confusing non-interactive hang/failure for a caller without passwordless sudo, in the middle of an unrelated `make build`/`test`/`lint`. - `install`: drop the `build` prerequisite and every internal `sudo` prefix. Add an explicit guard requiring EUID 0 ("try: sudo make install"), and a second guard requiring `dist/nfq_forwarder`/`dist/iptables` to already exist ("run 'make build' first, as yourself"). This directly fixes the "sane environment" concern too: previously `sudo make install` re-ran `build` (goreleaser, etc.) under sudo's own environment, which commonly resets PATH (secure_path) and HOME, breaking lookups for a per-user-installed toolchain. Splitting the privileged half (`install`(1)/`setcap`, both on any reasonable secure_path) away from the build step avoids needing any PATH-preservation workaround at all.
claude commented 2026-07-08 16:39:00 +01:00 (Migrated from gitlab.keyop.co.uk)

mentioned in commit 16e08fb12a5fa24d0f3dae02a86e44c6e0ae68b0

mentioned in commit 16e08fb12a5fa24d0f3dae02a86e44c6e0ae68b0
claude commented 2026-07-08 16:39:21 +01:00 (Migrated from gitlab.keyop.co.uk)

mentioned in merge request !79

mentioned in merge request !79
claude commented 2026-07-08 19:08:42 +01:00 (Migrated from gitlab.keyop.co.uk)

mentioned in commit 14f847bf81

mentioned in commit 14f847bf8181cb290b317b8fe4f457175f36b8a2
claude commented 2026-07-08 19:08:42 +01:00 (Migrated from gitlab.keyop.co.uk)

mentioned in commit 6c88be75e0

mentioned in commit 6c88be75e0f996b0ffe54670834541f629fc841d
danny commented 2026-07-08 19:16:38 +01:00 (Migrated from gitlab.keyop.co.uk)

mentioned in commit ad5d4b4646

mentioned in commit ad5d4b4646760bf8e045a91f69373ee2c1ca1206
danny (Migrated from gitlab.keyop.co.uk) closed this issue 2026-07-08 19:16:38 +01:00
claude commented 2026-07-08 19:17:51 +01:00 (Migrated from gitlab.keyop.co.uk)

Merged via !79 (merge commit ad5d4b4). No release for this one either, as agreed - Makefile/dev-workflow only, nothing in the shipped binary changes.

Merged via !79 (merge commit `ad5d4b4`). No release for this one either, as agreed - Makefile/dev-workflow only, nothing in the shipped binary changes.
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
keyop-go/nfq_forwarder#36
No description provided.