Allow other users #1357
Allow other users #1357
Conversation
WalkthroughThis pull request parameterizes container runtime user and read-only operation ownership by introducing configurable UID/GID through Docker build-time arguments and environment variables, replacing hardcoded numeric IDs across Dockerfiles, docker-compose configuration, entrypoint scripts, and documentation. Additional changes refactor entrypoint logic and add container environment testing for non-root UID/GID scenarios. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~28 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/DOCKER_COMPOSE.md (1)
6-6: Minor: Duplicate sentence in paragraph.Line 6 contains duplicate text: "Great care is taken to ensure NetAlertX meets the needs of everyone..." appears twice in the same sentence (once as "Great care" and once as "Good care").
🔎 Proposed fix
-Great care is taken to ensure NetAlertX meets the needs of everyone while being flexible enough for anyone. This document outlines how you can configure your docker-compose. There are many settings, so we recommend using the Baseline Docker Compose as-is, or modifying it for your system.Good care is taken to ensure NetAlertX meets the needs of everyone while being flexible enough for anyone. This document outlines how you can configure your docker-compose. There are many settings, so we recommend using the Baseline Docker Compose as-is, or modifying it for your system. +Great care is taken to ensure NetAlertX meets the needs of everyone while being flexible enough for anyone. This document outlines how you can configure your docker-compose. There are many settings, so we recommend using the Baseline Docker Compose as-is, or modifying it for your system.
🧹 Nitpick comments (3)
docs/DOCKER_INSTALLATION.md (2)
27-34: Clarify shell variable usage in docker run example.The
${NETALERTX_UID:-20211}syntax in the docker run command may confuse users. If these environment variables aren't exported on the host, the shell will substitute the defaults, but users may not realize they need to exportNETALERTX_UID/NETALERTX_GIDbefore running the command if they want custom values.Consider either:
- Using literal defaults (
uid=20211,gid=20211) with a note about customization, or- Adding a brief note that users should
export NETALERTX_UID=<value>before running if customizing
87-90: Same clarification needed for chown example.The
${NETALERTX_UID:-20211}:${NETALERTX_GID:-20211}syntax requires the user to either have these variables exported or understand shell parameter expansion. For a troubleshooting section, explicit values might be clearer:🔎 Suggested alternative
# Use the runtime UID/GID you intend to run with (default 20211:20211) -sudo chown -R ${NETALERTX_UID:-20211}:${NETALERTX_GID:-20211} /local_data_dir +sudo chown -R 20211:20211 /local_data_dir # or your custom UID:GIDinstall/production-filesystem/entrypoint.d/0-storage-permission.sh (1)
40-46: Hardcoded user references in warning message may cause confusion.The warning message still references
'netalertx' userand the defaultUSER in the image (20211:20211)while the script now supports arbitrary users viaTARGET_USER. Consider updating the message to reference${TARGET_USER}or making it more generic when custom users are configured.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
.devcontainer/Dockerfile(8 hunks).devcontainer/resources/devcontainer-Dockerfile(1 hunks)Dockerfile(5 hunks)docker-compose.yml(2 hunks)docs/DOCKER_COMPOSE.md(2 hunks)docs/DOCKER_INSTALLATION.md(2 hunks)docs/docker-troubleshooting/incorrect-user.md(1 hunks)install/production-filesystem/entrypoint.d/0-storage-permission.sh(2 hunks)install/production-filesystem/entrypoint.d/35-writable-config.sh(1 hunks)install/production-filesystem/entrypoint.d/60-user-netalertx.sh(1 hunks)install/production-filesystem/entrypoint.d/99-ports-available.sh(2 hunks)test/docker_tests/test_container_environment.py(6 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
.devcontainer/**/*
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Never
chmodorchownduring operations in devcontainer. Everything is already writable. If you need permissions, the devcontainer setup is broken - fix.devcontainer/scripts/setup.shor.devcontainer/resources/devcontainer-Dockerfileinstead.
Files:
.devcontainer/Dockerfile.devcontainer/resources/devcontainer-Dockerfile
**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.py: Usemylog(level, [message])for logging; levels are: none/minimal/verbose/debug/trace. Usenonefor most important messages that should always appear, such as exceptions.
Always set explicit timeouts on subprocess calls. Default to 60s minimum unless plugin config specifies otherwise. Nested subprocess calls need their own timeout.
Never hardcode ports or secrets; always useget_setting_value()to retrieve configuration values.
Use environment variables (NETALERTX_DB,NETALERTX_LOG, etc.) everywhere instead of hardcoding paths like/data/dbor relative paths.
Usehelper.pyfunctions (timeNowDB,normalize_mac, sanitizers) for time/MAC/string operations. Validate MACs before DB writes.
Add/modify settings viaccd()inserver/initialise.pyor per-plugin manifest. Define config once and read it via helpers everywhere.
Always leave logging enabled. If there is a possibility it will be difficult to debug with current logging, add more logging.
Files:
test/docker_tests/test_container_environment.py
🧠 Learnings (17)
📚 Learning: 2025-09-20T14:09:29.159Z
Learnt from: adamoutler
Repo: jokob-sk/NetAlertX PR: 1184
File: .devcontainer/scripts/setup.sh:103-116
Timestamp: 2025-09-20T14:09:29.159Z
Learning: In NetAlertX devcontainer setup, the netalertx user has write permissions to /var/log/nginx/ directory as it's explicitly chowned to netalertx:www-data in the Dockerfile, so setup.sh can write to nginx log files without sudo.
Applied to files:
docs/DOCKER_COMPOSE.mddocs/docker-troubleshooting/incorrect-user.mddocs/DOCKER_INSTALLATION.mddocker-compose.yml.devcontainer/Dockerfileinstall/production-filesystem/entrypoint.d/0-storage-permission.shinstall/production-filesystem/entrypoint.d/60-user-netalertx.shDockerfile
📚 Learning: 2025-10-26T17:09:18.621Z
Learnt from: adamoutler
Repo: jokob-sk/NetAlertX PR: 1235
File: .devcontainer/scripts/setup.sh:146-148
Timestamp: 2025-10-26T17:09:18.621Z
Learning: In `.devcontainer/scripts/setup.sh` and other devcontainer setup scripts for NetAlertX, chmod 666 on /var/run/docker.sock is acceptable because devcontainer environments are single-user development contexts where convenience can take priority over strict permission hardening.
Applied to files:
docs/DOCKER_COMPOSE.mddocs/docker-troubleshooting/incorrect-user.mddocs/DOCKER_INSTALLATION.mddocker-compose.yml.devcontainer/Dockerfileinstall/production-filesystem/entrypoint.d/0-storage-permission.shDockerfile
📚 Learning: 2025-10-19T01:40:57.095Z
Learnt from: adamoutler
Repo: jokob-sk/NetAlertX PR: 1230
File: .devcontainer/resources/devcontainer-overlay/services/config/nginx/netalertx.conf.template:1-2
Timestamp: 2025-10-19T01:40:57.095Z
Learning: In the NetAlertX repository, .devcontainer/resources/devcontainer-overlay/services/config/nginx/netalertx.conf.template is an auto-generated file that is intentionally committed to source control. It cannot be regenerated automatically outside the devcontainer environment and is required for the devcontainer to start, creating a bootstrap dependency.
Applied to files:
docs/DOCKER_COMPOSE.md.devcontainer/Dockerfile
📚 Learning: 2025-12-08T01:59:00.903Z
Learnt from: CR
Repo: jokob-sk/NetAlertX PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-08T01:59:00.903Z
Learning: Applies to **/*.py : Use environment variables (`NETALERTX_DB`, `NETALERTX_LOG`, etc.) everywhere instead of hardcoding paths like `/data/db` or relative paths.
Applied to files:
docs/DOCKER_COMPOSE.mddocs/DOCKER_INSTALLATION.mddocker-compose.yml.devcontainer/Dockerfileinstall/production-filesystem/entrypoint.d/0-storage-permission.sh
📚 Learning: 2025-09-20T14:08:48.256Z
Learnt from: adamoutler
Repo: jokob-sk/NetAlertX PR: 1184
File: .devcontainer/scripts/setup.sh:90-96
Timestamp: 2025-09-20T14:08:48.256Z
Learning: In the NetAlertX devcontainer setup, the setup.sh script intentionally removes user_notifications.json from the API directory during development environment initialization to prevent notification clutter that accumulates during container launches and development work.
Applied to files:
docs/docker-troubleshooting/incorrect-user.mdinstall/production-filesystem/entrypoint.d/60-user-netalertx.sh
📚 Learning: 2025-09-20T03:01:19.912Z
Learnt from: adamoutler
Repo: jokob-sk/NetAlertX PR: 1184
File: .devcontainer/Dockerfile:18-19
Timestamp: 2025-09-20T03:01:19.912Z
Learning: In the NetAlertX repository, .devcontainer/Dockerfile is auto-generated and should not be reviewed directly. Review comments about dependencies and build steps should be directed at the root Dockerfile where the actual source commands are located.
Applied to files:
docs/DOCKER_INSTALLATION.mddocker-compose.yml.devcontainer/DockerfileDockerfile.devcontainer/resources/devcontainer-Dockerfile
📚 Learning: 2025-09-20T02:56:24.501Z
Learnt from: adamoutler
Repo: jokob-sk/NetAlertX PR: 1184
File: .devcontainer/devcontainer.json:5-8
Timestamp: 2025-09-20T02:56:24.501Z
Learning: In the NetAlertX devcontainer setup, the final .devcontainer/Dockerfile is generated by combining the root Dockerfile with .devcontainer/resources/devcontainer-Dockerfile using the generate-dockerfile.sh script. The devcontainer.json should reference the generated file, not the root Dockerfile.
Applied to files:
docker-compose.yml.devcontainer/Dockerfile
📚 Learning: 2025-12-08T01:59:00.903Z
Learnt from: CR
Repo: jokob-sk/NetAlertX PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-08T01:59:00.903Z
Learning: Applies to .devcontainer/**/* : Never `chmod` or `chown` during operations in devcontainer. Everything is already writable. If you need permissions, the devcontainer setup is broken - fix `.devcontainer/scripts/setup.sh` or `.devcontainer/resources/devcontainer-Dockerfile` instead.
Applied to files:
.devcontainer/Dockerfileinstall/production-filesystem/entrypoint.d/0-storage-permission.shDockerfile
📚 Learning: 2025-12-08T01:59:00.903Z
Learnt from: CR
Repo: jokob-sk/NetAlertX PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-08T01:59:00.903Z
Learning: Devcontainer philosophy: brutal simplicity. One user, everything writable, completely idempotent. No permission checks, no conditional logic, no sudo needed.
Applied to files:
.devcontainer/DockerfileDockerfile
📚 Learning: 2025-09-20T14:13:33.635Z
Learnt from: adamoutler
Repo: jokob-sk/NetAlertX PR: 1184
File: .devcontainer/Dockerfile:11-16
Timestamp: 2025-09-20T14:13:33.635Z
Learning: In the NetAlertX repository with Alpine 3.22 base image, the `python -m venv` command works correctly in the devcontainer setup, likely due to symlink creation in the root Dockerfile that makes `python` available as an alias to `python3`.
Applied to files:
.devcontainer/Dockerfile.devcontainer/resources/devcontainer-Dockerfile
📚 Learning: 2025-09-20T14:08:44.152Z
Learnt from: adamoutler
Repo: jokob-sk/NetAlertX PR: 1184
File: .devcontainer/Dockerfile:82-92
Timestamp: 2025-09-20T14:08:44.152Z
Learning: In devcontainer builds, source repository files are not available during the Docker build phase. The source code gets mounted into the container after it's built and started, so COPY commands referencing source files will fail. Configuration files need to be handled at runtime (e.g., in setup scripts) rather than during the build stage.
Applied to files:
.devcontainer/Dockerfile
📚 Learning: 2025-09-20T14:13:33.635Z
Learnt from: adamoutler
Repo: jokob-sk/NetAlertX PR: 1184
File: .devcontainer/Dockerfile:11-16
Timestamp: 2025-09-20T14:13:33.635Z
Learning: In the NetAlertX devcontainer setup, the `python -m venv /opt/venv` command works successfully on Alpine 3.22 despite the typical Alpine behavior of not providing a /usr/bin/python symlink by default. The build completes successfully and pytest runs without issues.
Applied to files:
.devcontainer/DockerfileDockerfile
📚 Learning: 2025-10-10T22:16:02.770Z
Learnt from: adamoutler
Repo: jokob-sk/NetAlertX PR: 1214
File: install/ubuntu24/uninstall.sh:129-141
Timestamp: 2025-10-10T22:16:02.770Z
Learning: NetAlertX uninstall procedures should only remove files from the repository (specifically /app files) and should not touch system packages like PHP, nginx, avahi, or other shared system components to avoid damaging user systems.
Applied to files:
.devcontainer/DockerfileDockerfile
📚 Learning: 2025-11-24T00:56:58.733Z
Learnt from: adamoutler
Repo: jokob-sk/NetAlertX PR: 1304
File: .hadolint.yaml:1-2
Timestamp: 2025-11-24T00:56:58.733Z
Learning: In the NetAlertX repository, the .hadolint.yaml config file is intentionally configured to only globally ignore DL3018 (Alpine package pinning). Rules DL3006 (explicit image tagging) and SC2114 (deleting system directories) are handled via inline `# hadolint ignore` directives in specific Dockerfiles where needed, rather than being globally ignored, to ensure new Dockerfiles are still checked for these rules.
Applied to files:
.devcontainer/Dockerfile.devcontainer/resources/devcontainer-Dockerfile
📚 Learning: 2025-10-26T16:45:41.247Z
Learnt from: adamoutler
Repo: jokob-sk/NetAlertX PR: 1235
File: install/production-filesystem/services/scripts/check-ramdisk.sh:0-0
Timestamp: 2025-10-26T16:45:41.247Z
Learning: In NetAlertX check scripts (install/production-filesystem/services/scripts/check-*.sh), not all checks should exit with non-zero status. Some checks, like check-ramdisk.sh, are warning-only and exit 0 even when issues are detected, allowing the application to start despite suboptimal configuration.
Applied to files:
install/production-filesystem/entrypoint.d/60-user-netalertx.sh
📚 Learning: 2025-11-01T19:02:10.635Z
Learnt from: adamoutler
Repo: jokob-sk/NetAlertX PR: 1263
File: install/production-filesystem/entrypoint.sh:60-86
Timestamp: 2025-11-01T19:02:10.635Z
Learning: In the NetAlertX project (install/production-filesystem/entrypoint.sh), when fail-fast behavior is adopted for entrypoint checks in the future, be lenient during review because tests will be removed or changed to accommodate that behavior. The current continue-on-failure approach exists to support the existing test suite, but this is expected to change along with corresponding test adjustments.
Applied to files:
install/production-filesystem/entrypoint.d/60-user-netalertx.shinstall/production-filesystem/entrypoint.d/35-writable-config.sh
📚 Learning: 2025-10-26T15:39:36.707Z
Learnt from: adamoutler
Repo: jokob-sk/NetAlertX PR: 1235
File: install/production-filesystem/services/scripts/check-first-run-config.sh:0-0
Timestamp: 2025-10-26T15:39:36.707Z
Learning: In NetAlertX startup scripts, critical initialization failures (e.g., unable to create config directory or copy default config files) should exit with non-zero status to fail fast and provide clear error messages, rather than continuing in a broken state.
Applied to files:
install/production-filesystem/entrypoint.d/60-user-netalertx.sh
🧬 Code graph analysis (1)
test/docker_tests/test_container_environment.py (1)
test/docker_tests/test_ports_available.py (3)
_setup_mount_tree(60-98)_run_container(106-167)_assert_contains(170-178)
🪛 GitHub Actions: Code checks
test/docker_tests/test_container_environment.py
[error] 48-52: IndentationError: expected an indented block after function definition on line 48.
🪛 LanguageTool
docs/docker-troubleshooting/incorrect-user.md
[style] ~17-~17: Consider using a different verb for a more formal wording.
Context: ...re owned by a different UID. ## How to Correct the Issue Option A: Use defaults (reco...
(FIX_RESOLVE)
🪛 Ruff (0.14.8)
test/docker_tests/test_container_environment.py
52-52: Expected an indented block after function definition
(invalid-syntax)
55-56: Expected ), found newline
(invalid-syntax)
⏰ 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). (1)
- GitHub Check: docker_dev
🔇 Additional comments (20)
.devcontainer/resources/devcontainer-Dockerfile (1)
25-27: LGTM!The addition of
py3-psutilis appropriate for the devcontainer environment, supporting enhanced Python tooling and testing capabilities.docker-compose.yml (1)
11-12: LGTM!The runtime UID/GID parameterization is well-implemented with consistent defaults across both the
user:directive and tmpfs mount options. This enables flexible user configuration while maintaining backward compatibility.Also applies to: 54-54
docs/DOCKER_COMPOSE.md (1)
54-58: LGTM!The documentation updates clearly explain the runtime UID/GID configurability and provide helpful guidance in the TIP block about aligning mounts with the runtime user. The parameterized tmpfs examples are consistent with the actual docker-compose.yml.
Also applies to: 63-65, 97-99
Dockerfile (4)
53-58: LGTM!The separation of service account (NETALERTX_UID/GID) from read-only lock owner (READONLY_UID/GID) is a sound security design. The ARG declarations with sensible defaults enable build-time customization while maintaining backward compatibility.
138-140: LGTM!Dynamic user/group creation using ARG values enables flexible UID/GID configuration at build time.
159-160: LGTM!Correct use of READONLY ownership for the virtualenv and version files, implementing the "ownership-as-a-lock" security pattern where the readonly user cannot modify its own files.
Also applies to: 167-175
196-213: LGTM!Proper re-declaration of ARGs in the hardened stage (required since ARGs don't persist across FROM) and consistent application of the readonly user/group creation with configurable IDs.
install/production-filesystem/entrypoint.d/99-ports-available.sh (2)
9-14: Parsing approach is reasonable with caveats.The crude JSON parsing on line 12 handles common cases but may miss edge cases (e.g., whitespace variations, different quote styles). The fallback chain (
PORT_GQL → GRAPHQL_PORT → 20212) provides a safe default when parsing fails.The
|| trueensures the script continues gracefully if the pattern isn't found.
61-73: Verify intended behavior: warning always emitted if GraphQL port is listening.The comment says "always emit if listening, even if same port" but this could result in duplicate warnings when
PORT_APP == PORT_GQL(one from lines 17-28 about config conflict, and another here about the port being in use).This appears intentional for maximum visibility, but confirm this is the desired UX.
docs/docker-troubleshooting/incorrect-user.md (1)
1-31: LGTM!The restructured documentation clearly explains the UID/GID mismatch issue, provides concrete causes, and offers two remediation paths (defaults vs. custom UID/GID). The guidance aligns well with the configurable UID/GID feature introduced in this PR.
.devcontainer/Dockerfile (1)
1-2: Skipping detailed review - auto-generated file.Based on learnings, this file is auto-generated by
.devcontainer/scripts/generate-configs.shfrom the rootDockerfileand.devcontainer/resources/devcontainer-Dockerfile. Review comments for the source content have been provided on those respective files.install/production-filesystem/entrypoint.d/0-storage-permission.sh (2)
26-27: LGTM! Good parameterization of the target user.The use of
NETALERTX_USERenvironment variable with a sensible default aligns well with the PR's goal of enabling arbitrary user specification.
59-63: LGTM! Consistent use of TARGET_USER.The chown command and restart message correctly use the parameterized
TARGET_USERvariable.install/production-filesystem/entrypoint.d/60-user-netalertx.sh (2)
11-16: Fallback logic silently allows any user.When
getent passwdfails (user doesn't exist in container),EXPECTED_UID/GIDdefault toCURRENT_UID/GID, causing the check on line 18 to always pass. This is likely intentional to support arbitrary users, but verify this doesn't mask legitimate misconfiguration scenarios where thenetalertxuser should exist but doesn't.
21-23: Non-blocking note on UID/GID mismatch.The script now exits successfully (0) even when UID/GID don't match expectations, emitting only a note. This aligns with the PR's goal of allowing other users, but ensure downstream scripts and documentation reflect that this is now informational rather than a warning that blocks startup.
install/production-filesystem/entrypoint.d/35-writable-config.sh (1)
39-40: LGTM on error handling restructure.The existing path-not-found case correctly closes and resets color codes before the new non-regular-file branch.
test/docker_tests/test_container_environment.py (4)
214-214: LGTM onpre_entrypointparameter addition.The new optional parameter enables injecting setup scripts before entrypoint execution, which is useful for test scaffolding.
266-282: LGTM on pre-entrypoint script injection logic.The implementation correctly handles newline termination and integrates the pre-entrypoint script into both wait_for_exit and background execution paths.
48-51: [Your rewritten review comment text here]
[Exactly ONE classification tag]
459-466: The function signature for_setup_mount_treedoes include additional optional parameters beyond the two positional arguments shown in lines 52-54. Line 510 demonstrates this with theseed_db=Falsekeyword argument. However, all call sites across the file are consistent and correctly use the function—they invoke it with either two positional arguments or two positional arguments plus optional keyword arguments. Line 459 is correct in calling with just two arguments, relying on defaults for the optional parameters. No changes needed.
| elif [ ! -f "$path" ]; then | ||
| failures=1 | ||
| >&2 printf "%s" "${YELLOW}" | ||
| >&2 cat <<EOF | ||
| ══════════════════════════════════════════════════════════════════════════════ | ||
| ⚠️ ATTENTION: Read permission denied. | ||
|
|
||
| The path "${path}" is not a regular file (current type: $(stat -c %F "$path" 2>/dev/null || echo unknown)). | ||
| This prevents NetAlertX from reading the configuration and indicates a | ||
| permissions or mount issue — often seen when running with custom UID/GID. | ||
|
|
||
| https://github.com/jokob-sk/NetAlertX/blob/main/docs/docker-troubleshooting/file-permissions.md | ||
| ══════════════════════════════════════════════════════════════════════════════ | ||
| EOF |
There was a problem hiding this comment.
Misleading warning message for non-regular file check.
The warning heading says "⚠️ ATTENTION: Read permission denied." but the actual condition is that the path is not a regular file (e.g., it's a directory, symlink, or device). This mismatch will confuse users. Consider updating the heading to reflect the actual issue:
🔎 Proposed fix
-⚠️ ATTENTION: Read permission denied.
+⚠️ ATTENTION: Path is not a regular file.🤖 Prompt for AI Agents
In install/production-filesystem/entrypoint.d/35-writable-config.sh around lines
41 to 54, the warning header incorrectly states "Read permission denied" while
the code branch is triggered because the path is not a regular file; update the
message header and body to accurately reflect that the path is not a regular
file (e.g., directory, symlink, device) and that this prevents NetAlertX from
reading the expected configuration, keeping the existing guidance and link but
changing the heading and wording to avoid implying a permissions-only issue.
| def test_nonroot_custom_uid_logs_note( | ||
| tmp_path: pathlib.Path, | ||
| uid_gid: tuple[int, int], | ||
| ) -> None: | ||
| def _setup_mount_tree( | ||
| tmp_path: pathlib.Path, | ||
| prefix: str, | ||
| seed_config: bool = True, | ||
| seed_db: bool = True, | ||
| ) -> dict[str, pathlib.Path]: | ||
| label = _unique_label(prefix) | ||
| base = tmp_path / f"{label}_MOUNT_ROOT" | ||
| base.mkdir() | ||
| paths: dict[str, pathlib.Path] = {} | ||
|
|
||
| # Create unified /data mount root | ||
| data_root = base / f"{label}_DATA_INTENTIONAL_NETALERTX_TEST" | ||
| data_root.mkdir(parents=True, exist_ok=True) | ||
| data_root.chmod(0o777) | ||
| paths["data"] = data_root | ||
|
|
||
| # Create required data subdirectories and aliases | ||
| db_dir = data_root / "db" | ||
| db_dir.mkdir(exist_ok=True) | ||
| db_dir.chmod(0o777) | ||
| paths["app_db"] = db_dir | ||
| paths["data_db"] = db_dir | ||
|
|
||
| config_dir = data_root / "config" | ||
| config_dir.mkdir(exist_ok=True) | ||
| config_dir.chmod(0o777) | ||
| paths = _setup_mount_tree(tmp_path, f"note_uid_{uid}") | ||
| for key in ["data", "app_db", "app_config"]: | ||
| paths[key].chmod(0o777) | ||
| volumes = _build_volume_args_for_keys(paths, {"data"}) | ||
| result = _run_container( | ||
| f"note-uid-{uid}", | ||
| volumes, | ||
| user=f"{uid}:{gid}", | ||
| sleep_seconds=5, | ||
| ) | ||
| _assert_contains(result, f"NetAlertX note: current UID {uid} GID {gid}", result.args) | ||
| assert "expected UID" in result.output | ||
| assert result.returncode == 0 | ||
| paths["app_config"] = config_dir |
There was a problem hiding this comment.
Critical syntax error: Test function body is misplaced inside another function definition.
The test_nonroot_custom_uid_logs_note function on line 48 has no body - it immediately transitions to the _setup_mount_tree function definition on line 52. The actual test logic (lines 55-67) appears to be orphaned inside _setup_mount_tree. This causes the pipeline failure:
IndentationError: expected an indented block after function definition on line 48
The test also references uid and gid variables that are never unpacked from the uid_gid tuple parameter.
🔎 Proposed fix structure
def test_nonroot_custom_uid_logs_note(
tmp_path: pathlib.Path,
uid_gid: tuple[int, int],
) -> None:
+ """Test that non-root custom UID/GID logs an informational note."""
+ uid, gid = uid_gid
+ paths = _setup_mount_tree(tmp_path, f"note_uid_{uid}")
+ for key in ["data", "app_db", "app_config"]:
+ paths[key].chmod(0o777)
+ volumes = _build_volume_args_for_keys(paths, {"data"})
+ result = _run_container(
+ f"note-uid-{uid}",
+ volumes,
+ user=f"{uid}:{gid}",
+ sleep_seconds=5,
+ )
+ _assert_contains(result, f"NetAlertX note: current UID {uid} GID {gid}", result.args)
+ assert "expected UID" in result.output
+ assert result.returncode == 0
+
+
def _setup_mount_tree(
tmp_path: pathlib.Path,
prefix: str,
- paths = _setup_mount_tree(tmp_path, f"note_uid_{uid}")
- for key in ["data", "app_db", "app_config"]:
- paths[key].chmod(0o777)
- ...Note: The _setup_mount_tree function signature also appears incomplete - it likely needs additional parameters like seed_config and seed_db based on usage elsewhere in the file.
Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 GitHub Actions: Code checks
[error] 48-52: IndentationError: expected an indented block after function definition on line 48.
🪛 Ruff (0.14.8)
52-52: Expected an indented block after function definition
(invalid-syntax)
55-56: Expected ), found newline
(invalid-syntax)
🤖 Prompt for AI Agents
In test/docker_tests/test_container_environment.py around lines 48 to 68, the
test function test_nonroot_custom_uid_logs_note has its body accidentally
replaced by a new function definition (_setup_mount_tree), causing an
IndentationError and leaving uid/gid unused; move the test logic out of the
inner function so test_nonroot_custom_uid_logs_note contains the test steps
(setup mounts, chmod, build volumes, run container, assertions), restore or
correct the _setup_mount_tree helper signature to accept the needed parameters
(e.g., tmp_path, prefix, seed_config, seed_db) and return the paths dict, and
unpack uid_gid into uid, gid at the top of the test; also ensure config_dir is
defined/obtained before assigning paths["app_config"] = config_dir.
|
I must have fat fingered something before committing. I will open a new one later. |
1 participant
This PR modifies NetAlertX to remove the requirement for a dedicated NetAlertX user, enabling broader access for other users. The changes include rewriting the startup and login flow to support this new model.
Changes:
Summary by CodeRabbit
New Features
Documentation
Tests
✏️ Tip: You can customize this high-level summary in your review settings.