Beta

[rdwr-rahulk]

No description provided.

[Copilot]

🟡 Changes recommended

Unresolved installer, GID-validation, and bind-mount permission issues remain.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

This PR hardens the watchdog container and deployment flow for non-root execution, Docker socket access, and resource limits.

Changes:

• Runs the container as UID 1000 with filesystem and capability hardening.
• Adds Docker socket GID configuration and resource limits.
• Updates installation scripts, environment templates, and deployment guidance.

File summaries

File	Review summary
README.md	Documents hardening and limits; guidance should cover duplicated limits in the build Compose file.
install.sh	Updates ownership, permissions, and GID migration; privilege handling and permission checks require fixes.
Dockerfile	Adds non-root execution; host-mounted files must be readable by UID 1000.
docker-compose.yaml	Adds production hardening and limits; the GID sentinel bypasses intended validation.
docker-compose.build.yaml	Mirrors hardening and resource limits for build deployments.
DEPLOYMENT.md	Updates manual deployment guidance; the GID example must align with validation behavior.
.env.example	Adds Docker socket GID configuration; its non-empty sentinel causes opaque startup failures.

Review details

Suppressed comments (6)

.env.example:47

• The literal value in the template is non-empty, so ${DOCKER_GID:?…} does not reject it as unset/empty. Copying .env.example without editing this line therefore passes REPLACE_WITH_DOCKER_SOCKET_GID through to group_add and produces an opaque invalid-group/container-start error instead of the documented Compose validation message. Use an empty template value (and update the matching deployment example) or add explicit numeric validation before startup.

DOCKER_GID=REPLACE_WITH_DOCKER_SOCKET_GID

Dockerfile:20

• This switches the process to UID 1000, so every host bind mount must be readable by that numeric UID. The production compose file mounts watchdog-config.yaml and watchdog.py from the host, but the installer only normalizes the log directory and .env; with a restrictive umask or a manually protected 0600 config, the container now fails to start with PermissionError. Make these non-secret bind-mounted files readable by UID 1000 (or grant an equivalent ACL) in both installation paths before enforcing the non-root user.

USER watchdog

README.md:96

• This PR applies the same memory limits to docker-compose.build.yaml, but these instructions only tell operators to raise mem_limit/mem_reservation in docker-compose.yaml. Someone running the build compose file can therefore follow the guidance and still remain capped at 256 MiB. Refer to whichever compose file is used, or explicitly instruct updating both files for all duplicated resource limits.

> **Note:** The values above are approximate and may vary depending on the host operating system, Docker version, and installed dependencies. If usage approaches the 256 MB limit (check with `docker stats docker-container-watchdog`), raise `mem_limit`/`mem_reservation` in `docker-compose.yaml` rather than removing the limit — and raise `memswap_limit` to at least the new `mem_limit` in **both** `docker-compose.yaml` and `docker-compose.build.yaml`, since Docker rejects a config where `memswap_limit` is lower than `mem_limit`.

docker-compose.yaml:40

• ${DOCKER_GID:?…} only rejects an unset or empty value. Because the supplied .env.example and deployment snippet use the non-empty REPLACE_WITH_DOCKER_SOCKET_GID sentinel, Compose accepts it during interpolation and defers the failure until Docker tries to create the container with that literal group name, contrary to the documented fail-fast behavior. Use an empty template value or validate/reject the sentinel explicitly.

      - "${DOCKER_GID:?Set DOCKER_GID in .env to the docker.sock GID - run install.sh or see DEPLOYMENT.md}"

install.sh:130

• This makes sudo a hard prerequisite for every non-root install, even when the installer user owns the newly created watchdog directory (for example, the common UID 1000 user). That contradicts the documented non-root Docker-group prerequisite and causes installs to exit before attempting a direct chown; try the ownership change first and only fall back to sudo when it is actually needed, or make the runtime UID configurable.

    if [ "$EUID" -ne 0 ]; then
        if command -v sudo &>/dev/null; then
            PRIV="sudo"
        else
            print_error "Not running as root and sudo is not available — cannot set ${LOG_DIR} ownership to UID 1000:GID 1000"

install.sh:144

• chmod 750 makes the log directory accessible only to UID 1000 and its GID. When the installer is run by root or by a deployment account whose UID is not 1000, the host operator cannot follow the documented tail -f .../watchdog.log command without sudo, even though the installer explicitly supports non-root invocation. Preserve operator read/traverse access with an ACL or an agreed host group, or document that log access requires elevated privileges.

    $PRIV chmod 750 "$LOG_DIR"

• Files reviewed: 7/7 changed files
• Comments generated: 2
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[egori4]

Rahul, I reviewed PR #3 (beta → main). The branch comparison itself looks correct: the PR contains the remaining beta-to-main delta.

Before we merge, please make the following fixes in the same beta branch. You do not need to open a new PR — just commit/push the fixes to beta and PR #3 will update automatically.

1. Fix image versioning and upgrade behavior

Now that watchdog.py is packaged inside the Docker image and is no longer bind-mounted, we should not rely only on watchdog:latest.

Please version the Docker image using the application release version.

For this release, if the release is 1.5.4, the following should be consistent:

README version history: 1.5.4
Repository VERSION file: 1.5.4
Installer version: read from VERSION
Docker image: watchdog:1.5.4
watchdog.tar: must contain/tag watchdog:1.5.4
Compose deployment should run the explicitly selected version

Please add a root-level VERSION file and have install.sh read the version from it instead of hardcoding VERSION="1.0.0".

The installer should load the supplied watchdog.tar during an upgrade and configure Compose to use the package version. It must not silently keep an older Docker image simply because another watchdog image already exists.

Using watchdog:latest as an additional convenience tag is OK, but the deployed container should use the explicit release version.

Expected upgrade example:

1.5.3 → install 1.5.4 package → load watchdog:1.5.4 → container runs watchdog:1.5.4

Please also verify after installation that the running container is using the expected image/version.

2. Fix existing .env migration permissions

The upgrade code adds:

WATCHDOG_UID=1000
WATCHDOG_GID=1000
DOCKER_GID=<detected GID>

to an existing .env.

If .env was originally created by root and is 600 root:root, a normal Docker-group user cannot modify it.

Please make the migration handle the required permissions/sudo correctly and fail clearly if the file cannot be updated.

After writing, verify that all three values were actually saved before continuing.

3. Make watchdog-config.yaml readable by the non-root container

The new container normally runs as UID/GID 1000 when installed through install.sh.

Please make sure watchdog-config.yaml generated or reused by the installer is readable by that runtime user/group.

This should also work when upgrading an older installation where the config may have been created by root with restrictive permissions.

4. Version documentation

The latest beta commit says version 1.5.4, while the README changelog currently only adds 1.5.3.

Please make the version/changelog consistent with the actual release being merged.

5. Add basic CI validation for pull requests

We currently do not have any automated validation in this repository before merging changes into main.

Please add a simple GitHub Actions CI workflow so GitHub automatically checks the repository whenever a pull request is opened or updated against main.

The goal is not to deploy anything. This is only an automated validation step that runs on a temporary GitHub-hosted Linux machine and reports PASS/FAIL directly on the PR.

What needs to be added

Create this directory/file in the repository:

.github/workflows/ci.yml

If .github or workflows do not exist yet, create them.

The workflow should run when there is a pull request targeting main:

on:
  pull_request:
    branches:
      - main

This means that our normal process:

beta → PR → main

will automatically trigger the CI.

If PR #3 is already open and you push another commit to beta, the PR updates automatically and CI should run again automatically.

What the CI should validate

At minimum please run these checks:

Shell script syntax

bash -n install.sh
bash -n uninstall.sh

This checks that the installer/uninstaller do not contain Bash syntax errors.

Python syntax

python3 -m py_compile watchdog.py

This checks that watchdog.py can be parsed successfully by Python.

Docker Compose configuration

Because Compose expects an .env file, first prepare a temporary one for CI:

cp .env.example .env

Then validate both Compose files:

docker compose -f docker-compose.yaml config -q
docker compose -f docker-compose.build.yaml config -q

This checks that the Docker Compose YAML, variables and configuration can be parsed successfully.

Suggested initial workflow

You can use the following as the starting point:

name: CI

on:
  pull_request:
    branches:
      - main

jobs:
  validate:
    runs-on: ubuntu-latest

    steps:
      - name: Checkout repository
        uses: actions/checkout@v4

      - name: Validate shell scripts
        run: |
          bash -n install.sh
          bash -n uninstall.sh

      - name: Validate Python syntax
        run: |
          python3 -m py_compile watchdog.py

      - name: Prepare CI environment
        run: |
          cp .env.example .env

      - name: Validate Docker Compose
        run: |
          docker compose -f docker-compose.yaml config -q
          docker compose -f docker-compose.build.yaml config -q

How to verify it works

After you commit and push .github/workflows/ci.yml to beta:

1. PR #3 should update automatically.
2. GitHub should start a new workflow run.
3. On the PR page we should see a Checks section.
4. The CI / validate check should finish green.

If the check fails, please open the failed job, review the error and fix it in beta. Push the fix to the same branch; the same PR will update and CI will run again.

Please do not create a new PR.

Repository configuration

For the first step, adding .github/workflows/ci.yml should be enough for GitHub Actions to start running, assuming GitHub Actions is enabled for this repository.

Please verify under:

Repository → Actions

that the workflow appears and runs.

If Actions are disabled or restricted at the Radware organization/repository level, let me know rather than trying to work around it.

Later: make CI mandatory before merge

For this PR, the first goal is simply to get the CI workflow running and green.

After the workflow exists and we confirm the check name, we can separately configure branch protection/rules for main so GitHub does not allow a PR to be merged unless this CI check passes.

That repository setting is separate from the workflow itself and may require repository/admin permissions.

For now:

• create the workflow,
• push it to beta,
• confirm it runs on PR #3,
• confirm CI / validate is green,
• then request review again.