(socks-proxy) add healthcheck to dockerfile

[zeylos]

Purpose

Description...

Proposal

Description...

• [] item 1...
• [] item 2...

Summary by CodeRabbit

• Chores
  • Added automated runtime health checks for the SOCKS proxy.
  • Hardened build and startup processes for more reliable container operation (noninteractive package installs and runtime tooling ensured).
  • Relaxed strict file-permission handling in favor of a safer startup umask to better support runtime updates and secret handling.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Walkthrough

Updates to the SOCKS proxy Docker image and entrypoint: apt installs made noninteractive in the base stage; runtime stage now installs netcat-traditional then copies sockd; a HEALTHCHECK using nc probing localhost on the proxy port was added; entrypoint sets umask 0077 and removes chmod 0600 /etc/sockd.conf.

Changes

Cohort / File(s)	Summary
Dockerfile: apt installs, runtime prep & healthcheck src/socks-proxy/Dockerfile	Prefixes apt installs in the base stage with DEBIAN_FRONTEND="noninteractive"; adds a RUN in the runtime stage to install netcat-traditional (and clean apt lists) before copying sockd; moves the COPY of sockd after that RUN; inserts a HEALTHCHECK that uses nc to probe localhost:<port>; CMD to run sockd remains but is repositioned.
Entrypoint: startup umask & permissions src/socks-proxy/entrypoint.sh	Adds umask 0077 at startup and removes the explicit chmod 0600 /etc/sockd.conf; no other functional changes reported.

Sequence Diagram(s)

sequenceDiagram
    participant Container
    participant Entrypoint
    participant Sockd
    participant Healthcheck as HC

    Container->>Entrypoint: container start
    rect `#F6F8FF`
        Entrypoint->>Entrypoint: set umask 0077
        Entrypoint->>Entrypoint: remaining init logic (unchanged)
        Entrypoint->>Sockd: exec sockd (CMD)
    end

    loop periodic
        HC->>Container: nc -zv 127.0.0.1 <PROXY_PORT>
        alt success
            HC-->>Container: probe OK (exit 0)
        else failure
            HC-->>Container: probe failed (non-zero)
        end
    end

Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

• Verify the HEALTHCHECK uses the correct port and that netcat-traditional is installed in the runtime image layer where the check runs.
• Confirm umask 0077 and removal of the explicit chmod do not break expected file permissions for config or credential files.
• Check Dockerfile layer ordering for caching and correctness.

Poem

🐰 I set a quiet umask at the start of the day,
Installed a little netcat to safely say "hey",
A tiny health ping ticks at the door,
Sockd wakes up and listens once more,
I hop, I hum, the proxy hums away.

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The PR title accurately describes the main change: adding a HEALTHCHECK instruction to the socks-proxy Dockerfile, which is the primary addition evident in the changeset.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

src/socks-proxy/entrypoint.sh (1)

44-44: Optional refactor: replace xargs whitespace trim with bash parameter expansion.

Line 44 uses echo "$ip_range" | xargs to trim whitespace. This spawns a subprocess unnecessarily. Consider a pure bash approach for consistency and minor efficiency gain:

-    ip_range=$(echo "$ip_range" | xargs)
+    ip_range="${ip_range#"${ip_range%%[![:space:]]*}"}"  # trim leading
+    ip_range="${ip_range%"${ip_range##*[![:space:]]}"}"  # trim trailing

Or more concisely using parameter expansion (requires bash 4.4+):

+    ip_range=${ip_range//[[:space:]]}  # removes all whitespace

However, for maximum clarity and compatibility, the current xargs approach is also acceptable.

Also applies to: 41-63

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4d68cb4 and bc0ac1a.

📒 Files selected for processing (2)

• src/socks-proxy/Dockerfile (2 hunks)
• src/socks-proxy/entrypoint.sh (1 hunks)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)

• GitHub Check: docker-publish-mta-in / docker-build-push
• GitHub Check: docker-publish-socks-proxy / docker-build-push
• GitHub Check: test-back
• GitHub Check: lint-back
• GitHub Check: build-front
• GitHub Check: check-api-state

🔇 Additional comments (3)

src/socks-proxy/Dockerfile (1)

9-9: Good practice: noninteractive apt installation and efficient layer structure.

The use of DEBIAN_FRONTEND="noninteractive" prevents interactive prompts, and the runtime stage correctly isolates curl installation with proper cleanup, reducing layer bloat.

Also applies to: 25-29

src/socks-proxy/entrypoint.sh (2)

3-3: Good security practice: restrictive umask and credential storage.

The umask 0077 ensures /healthcreds is created with 0600 permissions (readable only by owner), protecting the healthcheck credentials from unauthorized access. The random password generation using /dev/urandom and character stripping is appropriate.

Also applies to: 10-12

---

41-63: Dynamic DANTE config generation handles IP whitelist correctly.

The loop properly splits PROXY_SOURCE_IP_WHITELIST by comma and generates client/socks sections for each range. Default to 0.0.0.0/0 when unset is sensible. This approach scales better than hardcoded configuration.

[coderabbitai]

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/socks-proxy/entrypoint.sh (1)

18-23: Misleading message: code always resets password despite "skipping" log.

The message on line 19 says the user is being skipped, but line 23 unconditionally calls chpasswd, which resets the password every invocation. Either skip the password reset or clarify the message to match the actual behavior.

Apply this diff to clarify intent:

  if id "$user" &>/dev/null; then
-   echo "User $user already exists, skipping"
+   echo "User $user already exists, updating password"
  else
    useradd -M -s /usr/sbin/nologin "$user"
  fi

Alternatively, if you want true idempotency and only set password on creation, restructure as:

  if id "$user" &>/dev/null; then
    echo "User $user already exists, skipping"
  else
    useradd -M -s /usr/sbin/nologin "$user"
    echo "$user:$pass" | chpasswd
  fi

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between bc0ac1a and e352af3.

📒 Files selected for processing (2)

• src/socks-proxy/Dockerfile (2 hunks)
• src/socks-proxy/entrypoint.sh (1 hunks)

🚧 Files skipped from review as they are similar to previous changes (1)

• src/socks-proxy/Dockerfile

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)

• GitHub Check: docker-publish-socks-proxy / docker-build-push
• GitHub Check: check-api-state
• GitHub Check: build-front
• GitHub Check: lint-back
• GitHub Check: test-back

🔇 Additional comments (1)

src/socks-proxy/entrypoint.sh (1)

10-12: Verify Dockerfile HEALTHCHECK correctly reads generated credentials.

The entrypoint generates and stores healthcheck credentials in /healthcreds (plaintext), but idempotency and timing must align with the Dockerfile's HEALTHCHECK probe. Ensure:

1. The HEALTHCHECK curl command reads from /healthcreds or receives the password via environment.
2. The /healthcreds file is readable by the process running the health probe.
3. No race condition exists between credential generation (line 12) and initial healthcheck probe.

Since the Dockerfile was not provided for review, please verify that the healthcheck mechanism correctly uses these credentials.

[sylvinus]

@zeylos not sure I feel good about adding a dependency to an external service ? (ip.me)

[zeylos]

you're right actually, I hit ip.me so that I can have the current outgoing IP in the docker inspect command but it's not really useful in our setup.
I chose ip.me because it's managed by Protonmail and it's pretty stable but we can just hit https://messages.suite.anct.gouv.fr ? or just check the port is open, but then we're only checking the daemon's up, not the auth part

[zeylos]

Actually I don't want other instances to rely on us and putting a var just for the healthcheck feels overkill. Since netcat will handle what we actually want -checking the daemon's health- I'll just go with netcat

[zeylos]

Netcat was generating way too much logs, and I didn't find any easy way to delete netcat logs and keep the actual traffic's logs. the current healthcheck is not actually that useful - it only checks if the process is not in Disk Sleep or Zombie state. This should do, we can work on it if we find any weird edge case later