Hardening fixes #1235
Hardening fixes #1235
Conversation
WalkthroughFixes PYTHONPATH and Python binary resolution; expands devcontainer tooling, permissions, and docker-socket group handling; writes build timestamps; parameterizes compose networking/volumes; adds many startup/storage/network/permission validation scripts; adjusts service startup/entrypoints and adds container-focused pytest tests. (33 words) Changes
Sequence Diagram(s)sequenceDiagram
rect rgb(250,250,252)
participant Host as Developer Host
participant DevCt as DevContainer
participant Setup as setup.sh
participant Docker as Docker Socket
Host->>DevCt: start devcontainer (mount `/var/run/docker.sock`)
DevCt->>Setup: run postCreateCommand (pip installs)
DevCt->>Setup: run postStartCommand -> setup.sh
Setup->>Docker: stat `/var/run/docker.sock` (get gid)
Setup->>Setup: ensure_docker_socket_access() → resolve/create group, add `netalertx` user
Setup->>DevCt: set permissions, write `NETALERTX_FRONT/buildtimestamp.txt`
DevCt-->>Host: switch to `netalertx` user, `sleep infinity`
end
sequenceDiagram
participant Entrypoint as entrypoint.sh
participant Checker as check-*.sh
participant Services as backend/nginx/php-fpm
participant Vendor as update_vendors.sh
Entrypoint->>Checker: execute each check script
Checker-->>Entrypoint: return per-check status
Entrypoint->>Entrypoint: record non-zero into FAILED_STATUS (string)
alt FAILED_STATUS non-empty
Entrypoint->>Entrypoint: print failures, exit with code
else NETALERTX_CHECK_ONLY == 1
Entrypoint->>Entrypoint: exit after checks with status
else
Entrypoint->>Services: start services (background)
Entrypoint->>Vendor: run vendor update via `bash` (background)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (1)📚 Learning: 2025-10-26T16:45:41.228ZApplied to files:
⏰ 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)
🔇 Additional comments (3)
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: 14
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
test/test_safe_builder_unit.py (1)
15-153: Do not reimplement SafeConditionBuilder in tests; import the production class to avoid security drift.Redefining SafeConditionBuilder here risks divergence from server/db/sql_safe_builder.py and weakens guarantees of your SQLi tests. Import the real class and keep the lightweight logger shim.
Apply:
@@ -# Standalone version of SafeConditionBuilder for testing -class SafeConditionBuilder: - """ - Test version of SafeConditionBuilder with mock logger. - """ - ... - def get_safe_condition_legacy(self, condition_setting): - """Convert legacy condition settings to safe parameterized queries.""" - if not condition_setting or not condition_setting.strip(): - return "", {} - - try: - return self.build_safe_condition(condition_setting) - except ValueError: - # Log the error and return empty condition for safety - return "", {} +# Use the production implementation to ensure tests cover real behavior +from server.db.sql_safe_builder import SafeConditionBuilderKeep
sys.modules['logger'] = Mock()sofrom logger import mylogin the prod module resolves to a harmless mock. Based on learnings.
🧹 Nitpick comments (12)
server/api_server/nettools_endpoint.py (1)
5-5: Remove unused import.The
speedtestmodule is imported with an alias but never used in the code. The function usessubprocessto call thespeedtest-clicommand instead.Apply this diff to remove the unused import:
-import speedtest as speedtest_cliinstall/production-filesystem/services/scripts/check-nginx-config.sh (1)
48-48: Consider trap-based cleanup for the temp file.If the script is interrupted between lines 30 and 48, the temp file may remain. While unlikely to cause issues, a trap handler would ensure cleanup in all scenarios.
Add cleanup on exit:
+trap 'rm -f "${TMP_FILE}"' EXIT + TMP_FILE="${CONF_ACTIVE_DIR}/.netalertx-write-test" if ! ( : >"${TMP_FILE}" ) 2>/dev/null; thenThen remove the explicit
rm -f "${TMP_FILE}"on line 48 since the trap handles it..devcontainer/resources/devcontainer-Dockerfile (1)
10-10: Consider using a variable for the Python version path.The hardcoded
python3.12in PYTHONPATH will break when Alpine updates to Python 3.13+. Consider using a shell variable derived frompython3 --versionor a glob pattern in the entrypoint script to maintain forward compatibility.test/test_compound_conditions.py (1)
236-236: Address unused variable warnings from static analysis.Several test functions unpack both
sqlandparamsbut only use one of them. Use underscore-prefixed variables for unused values to signal intent and satisfy linters.Apply these changes:
Line 236 (in try block - params is used in except):
try: - sql, params = builder.build_safe_condition(condition) + sql, _params = builder.build_safe_condition(condition)Line 249:
- sql, params = builder.build_safe_condition(condition) + _sql, params = builder.build_safe_condition(condition)Line 303:
- sql, params = builder.build_safe_condition(condition) + _sql, params = builder.build_safe_condition(condition)Line 314:
- sql, params = builder.build_safe_condition(condition) + _sql, params = builder.build_safe_condition(condition)Also applies to: 249-249, 303-303, 314-314
install/production-filesystem/services/scripts/check-persistent_storage.sh (1)
38-40: Minor: Inconsistent indentation (tabs vs spaces).Line 39 uses a tab character while the rest of the script uses spaces. Consider standardizing on spaces for consistency.
test/test_safe_builder_unit.py (3)
271-279: Silence Ruff RUF059: prefix unusedsqlwith underscore.
sqlis not used; prefer_to signal intentional discard.- sql, params = builder.build_safe_condition("AND devName = 'Device1'") + _, params = builder.build_safe_condition("AND devName = 'Device1'")
311-314: Silence Ruff RUF059: prefix unusedsqlwith underscore.Same nit here.
- sql, params = builder.build_safe_condition(f"AND devName = '{unicode_str}'") + _, params = builder.build_safe_condition(f"AND devName = '{unicode_str}'")
326-333: Avoid broadexcept Exception; let pytest surface unexpected errors.Catching all exceptions hides real failures. Either remove the try/except or catch a specific error.
- for case in edge_cases: - try: - sql, params = builder.get_safe_condition_legacy(case) - # Should either return valid result or empty safe result - assert isinstance(sql, str) - assert isinstance(params, dict) - except Exception: - pytest.fail(f"Unexpected exception for edge case: {case}") + for case in edge_cases: + sql, params = builder.get_safe_condition_legacy(case) + # Should either return valid result or empty safe result + assert isinstance(sql, str) + assert isinstance(params, dict)test/docker_tests/test_container_environment.py (4)
286-307: Use pytest tmp_path instead of a hardcoded /tmp directory.Hardcoding /tmp is brittle and flagged by static analysis. Let pytest manage a temporary directory via tmp_path.
-def test_second_run_starts_clean() -> None: +def test_second_run_starts_clean(tmp_path: pathlib.Path) -> None: @@ - base = pathlib.Path("/tmp/NETALERTX_SECOND_RUN_CLEAN_TEST_MOUNT_INTENTIONAL") - paths = _setup_fixed_mount_tree(base) + base = tmp_path / "NETALERTX_SECOND_RUN_CLEAN_TEST_MOUNT_INTENTIONAL" + paths = _setup_fixed_mount_tree(base) @@ - finally: - shutil.rmtree(base, ignore_errors=True) + finally: + shutil.rmtree(base, ignore_errors=True)
106-120: Avoid devcontainer-specific hardcoded paths; resolve repo root dynamically.Tests currently assume /workspaces/NetAlertX; compute the repository root relative to this test file or via an env var override.
+REPO_ROOT = pathlib.Path(os.environ.get("NETALERTX_REPO_ROOT", "")).resolve() \ + if os.environ.get("NETALERTX_REPO_ROOT") \ + else pathlib.Path(__file__).resolve().parents[2] @@ - shutil.copyfile( - "/workspaces/NetAlertX/back/app.conf", - config_file, - ) + shutil.copyfile(str(REPO_ROOT / "back/app.conf"), config_file) @@ - shutil.copyfile( - "/workspaces/NetAlertX/db/app.db", - db_file, - ) + shutil.copyfile(str(REPO_ROOT / "db/app.db"), db_file) @@ - shutil.copyfile("/workspaces/NetAlertX/back/app.conf", paths["app_config"] / "app.conf") - shutil.copyfile("/workspaces/NetAlertX/db/app.db", paths["app_db"] / "app.db") + shutil.copyfile(str(REPO_ROOT / "back/app.conf"), paths["app_config"] / "app.conf") + shutil.copyfile(str(REPO_ROOT / "db/app.db"), paths["app_db"] / "app.db")Also applies to: 296-301
32-50: Remove or use unused Docker volume helpers._helpers
_create_docker_volumeand_remove_docker_volumeare currently unused; consider removing to reduce noise or add tests that use named volumes as per TODO.
25-25: Optional: guard on docker availability to skip gracefully.Consider skipping when
dockerisn’t present to reduce CI flakiness on non-docker runners.-pytestmark = [pytest.mark.docker, pytest.mark.feature_complete] +pytestmark = [pytest.mark.docker, pytest.mark.feature_complete] + +if shutil.which("docker") is None: + pytest.skip("docker not available on PATH; skipping docker integration tests", allow_module_level=True)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (31)
.devcontainer/Dockerfile(3 hunks).devcontainer/devcontainer.json(2 hunks).devcontainer/resources/devcontainer-Dockerfile(1 hunks).devcontainer/scripts/setup.sh(2 hunks).gitignore(1 hunks)README.md(2 hunks)docker-compose.yml(3 hunks)install/production-filesystem/entrypoint.sh(1 hunks)install/production-filesystem/services/scripts/check-app-permissions.sh(1 hunks)install/production-filesystem/services/scripts/check-capabilities.sh(1 hunks)install/production-filesystem/services/scripts/check-first-run-config.sh(1 hunks)install/production-filesystem/services/scripts/check-first-run-db.sh(1 hunks)install/production-filesystem/services/scripts/check-mandatory-folders.sh(1 hunks)install/production-filesystem/services/scripts/check-network-mode.sh(1 hunks)install/production-filesystem/services/scripts/check-nginx-config.sh(1 hunks)install/production-filesystem/services/scripts/check-permissions.sh(0 hunks)install/production-filesystem/services/scripts/check-persistent_storage.sh(1 hunks)install/production-filesystem/services/scripts/check-ramdisk.sh(1 hunks)install/production-filesystem/services/scripts/check-root.sh(1 hunks)install/production-filesystem/services/scripts/check-storage-extra.sh(1 hunks)install/production-filesystem/services/scripts/check-storage.sh(0 hunks)install/production-filesystem/services/scripts/check-user-netalertx.sh(1 hunks)install/production-filesystem/services/start-backend.sh(1 hunks)install/production-filesystem/services/start-nginx.sh(1 hunks)pyproject.toml(1 hunks)scripts/db_cleanup/regenerate-database.sh(0 hunks)server/api_server/nettools_endpoint.py(2 hunks)test/docker_tests/dockerfiles/no-error-compose.yml(1 hunks)test/docker_tests/test_container_environment.py(1 hunks)test/test_compound_conditions.py(1 hunks)test/test_safe_builder_unit.py(2 hunks)
💤 Files with no reviewable changes (3)
- scripts/db_cleanup/regenerate-database.sh
- install/production-filesystem/services/scripts/check-storage.sh
- install/production-filesystem/services/scripts/check-permissions.sh
🧰 Additional context used
📓 Path-based instructions (3)
**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use 'logger.mylog' for logging at levels: none, minimal, verbose, debug, or trace.
Files:
test/test_compound_conditions.pyserver/api_server/nettools_endpoint.pytest/test_safe_builder_unit.pytest/docker_tests/test_container_environment.py
test/**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Tests must reside under 'test/' and use pytest.
Files:
test/test_compound_conditions.pytest/test_safe_builder_unit.pytest/docker_tests/test_container_environment.py
server/api_server/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
When introducing new server functionality, add endpoints only within 'server/api_server/*' and keep authorization checks consistent.
Files:
server/api_server/nettools_endpoint.py
🧠 Learnings (6)
📚 Learning: 2025-10-19T01:40:57.071Z
Learnt from: adamoutler
PR: jokob-sk/NetAlertX#1230
File: .devcontainer/resources/devcontainer-overlay/services/config/nginx/netalertx.conf.template:1-2
Timestamp: 2025-10-19T01:40:57.071Z
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:
test/docker_tests/dockerfiles/no-error-compose.yml.devcontainer/Dockerfile
📚 Learning: 2025-09-20T03:01:19.912Z
Learnt from: adamoutler
PR: jokob-sk/NetAlertX#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:
.devcontainer/resources/devcontainer-Dockerfile.devcontainer/Dockerfile
📚 Learning: 2025-09-20T02:56:24.501Z
Learnt from: adamoutler
PR: jokob-sk/NetAlertX#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:
.devcontainer/resources/devcontainer-Dockerfile.devcontainer/Dockerfile
📚 Learning: 2025-09-20T14:13:33.635Z
Learnt from: adamoutler
PR: jokob-sk/NetAlertX#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/resources/devcontainer-Dockerfile.devcontainer/Dockerfile
📚 Learning: 2025-09-20T14:13:33.635Z
Learnt from: adamoutler
PR: jokob-sk/NetAlertX#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/resources/devcontainer-Dockerfile.devcontainer/Dockerfile
📚 Learning: 2025-09-20T14:09:29.159Z
Learnt from: adamoutler
PR: jokob-sk/NetAlertX#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:
.devcontainer/resources/devcontainer-Dockerfile.devcontainer/Dockerfile
🧬 Code graph analysis (2)
test/test_compound_conditions.py (2)
test/test_safe_builder_unit.py (3)
SafeConditionBuilder(15-152)builder(156-158)build_safe_condition(81-96)server/db/sql_safe_builder.py (2)
SafeConditionBuilder(23-623)build_safe_condition(122-151)
test/test_safe_builder_unit.py (2)
server/db/sql_safe_builder.py (7)
SafeConditionBuilder(23-623)_sanitize_string(64-84)_validate_column_name(86-96)_validate_operator(98-108)_build_simple_condition(414-438)build_safe_condition(122-151)get_safe_condition_legacy(604-623)test/test_compound_conditions.py (1)
builder(21-23)
🪛 Ruff (0.14.1)
test/test_compound_conditions.py
236-236: Unpacked variable params is never used
Prefix it with an underscore or any other dummy variable pattern
(RUF059)
249-249: Unpacked variable sql is never used
Prefix it with an underscore or any other dummy variable pattern
(RUF059)
303-303: Unpacked variable sql is never used
Prefix it with an underscore or any other dummy variable pattern
(RUF059)
314-314: Unpacked variable sql is never used
Prefix it with an underscore or any other dummy variable pattern
(RUF059)
server/api_server/nettools_endpoint.py
84-84: Starting a process with a partial executable path
(S607)
test/test_safe_builder_unit.py
274-274: Unpacked variable sql is never used
Prefix it with an underscore or any other dummy variable pattern
(RUF059)
312-312: Unpacked variable sql is never used
Prefix it with an underscore or any other dummy variable pattern
(RUF059)
332-332: Do not catch blind exception: Exception
(BLE001)
test/docker_tests/test_container_environment.py
34-34: subprocess call: check for execution of untrusted input
(S603)
35-35: Starting a process with a partial executable path
(S607)
44-44: subprocess call: check for execution of untrusted input
(S603)
45-45: Starting a process with a partial executable path
(S607)
55-55: Avoid specifying long messages outside the exception class
(TRY003)
76-76: subprocess call: check for execution of untrusted input
(S603)
83-83: Avoid specifying long messages outside the exception class
(TRY003)
92-92: Loop control variable target not used within loop body
Rename unused target to _target
(B007)
214-214: f-string without any placeholders
Remove extraneous f prefix
(F541)
221-221: subprocess call: check for execution of untrusted input
(S603)
293-293: Probable insecure usage of temporary file or directory: "/tmp/NETALERTX_SECOND_RUN_CLEAN_TEST_MOUNT_INTENTIONAL"
(S108)
🪛 Shellcheck (0.11.0)
install/production-filesystem/services/start-backend.sh
[warning] 15-15: Quote this to prevent word splitting.
(SC2046)
⏰ 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 (34)
.gitignore (1)
1-1: Add coverage ignore entry to align with pytest workflow.The addition of
.coverageis appropriate given the PR's test modernization to pytest, which generates coverage reports via coverage.py. This prevents build artifacts from being committed to version control.install/production-filesystem/services/scripts/check-root.sh (1)
32-32: Sleep duration seems reasonable for user readability.The 5-second pause gives users adequate time to read the warning before the container stops.
install/production-filesystem/services/scripts/check-ramdisk.sh (1)
48-48: Directory check is appropriate for conf.active.The change from file (
-f) to directory (-d) check correctly reflects thatconf.activeis a directory containing configuration files rather than a single file.install/production-filesystem/services/scripts/check-user-netalertx.sh (1)
1-43: User validation logic is sound but exits non-fatally.The implementation correctly validates the running user against expected UID/GID with a reasonable fallback to hardcoded defaults. However, exiting with code 0 after the validation failure warning (line 43) means the container continues starting even when running as the wrong user.
Given the warning message states this "loses crucial safeguards," verify this is the intended behavior. If enforcement is desired, change line 43 to
exit 1. Otherwise, ensure the entrypoint or orchestration layer can detect and respond to this condition appropriately.install/production-filesystem/services/scripts/check-nginx-config.sh (1)
1-50: Nginx config validation is well-implemented.The script correctly validates both the existence and writability of the config directory with clear error messages and remediation steps. The temp file write test (line 30) is a reliable approach.
Note: This script exits with code 1 on failures (lines 26, 46), while most other check scripts in this PR exit with code 0. Verify this is intentional - presumably nginx config is considered critical while other checks are advisory. Consider documenting this distinction in the script header.
README.md (2)
49-56: Deployment instructions are clear and actionable.The added git clone workflow provides a straightforward path for users to deploy from source. The use of
--force-recreate --buildflags ensures a clean build.Consider adding a brief note about customization options (environment variables, ports, volumes) either inline or with a link to the docker-compose.yml documentation.
13-31: Improved table of contents structure.The restructured TOC with dedicated subsections under Features and a new "Everything else" section improves navigation and discoverability.
install/production-filesystem/services/scripts/check-first-run-db.sh (1)
5-15: LGTM!The conditional logic correctly implements idempotent first-run behavior: ALWAYS_FRESH_INSTALL forces a fresh database rebuild, and existing installations exit early to preserve data.
.devcontainer/scripts/setup.sh (1)
28-65: Solid Docker socket discovery and group handling.The ensure_docker_socket_access() function correctly detects the Docker socket, resolves its group with appropriate fallbacks, and idempotently adds the netalertx user to the group. Error handling is thorough.
pyproject.toml (1)
5-9: Good test infrastructure updates.The expanded testpaths and new markers enable selective execution of Docker integration tests and extended coverage suites without cluttering the default test run. Aligns well with the PR's testing strategy.
install/production-filesystem/services/start-nginx.sh (1)
51-55: LGTM!Adding error_log to /dev/stderr in the nginx -g directive improves container observability by routing errors to stdout for container log aggregation while maintaining the file log. Correct approach for containerized services.
install/production-filesystem/entrypoint.sh (2)
57-81: Excellent pre-startup health check framework.The guarded check loop correctly runs validation scripts only in production (NETALERTX_DEBUG != 1), captures and propagates exit codes, and provides per-script logging for troubleshooting. The check-only mode is useful for validation. Proper error handling throughout.
Verify that all check-*.sh scripts in ${SYSTEM_SERVICES_SCRIPTS} are intended to be discovered and executed by this loop, and that they all return appropriate exit codes (0 for success, non-zero for failure).
85-85: LGTM!Switching vendor update from sh to bash allows more robust scripting and running in the background avoids blocking startup. Good change for container readiness.
.devcontainer/devcontainer.json (1)
26-46: LGTM!Docker socket mount enables integration testing with Docker socket access, and the postCreateCommand installs essential test and debug tools (pytest, docker, debugpy) into the isolated venv. Both changes support the PR's testing modernization.
install/production-filesystem/services/scripts/check-app-permissions.sh (1)
38-47: LGTM for root permission fixes.The chown and find commands correctly ensure netalertx ownership and appropriate permissions (700 for dirs, 600 for files) when running as root. This is the right approach for initializing the container filesystem.
install/production-filesystem/services/scripts/check-network-mode.sh (3)
1-14: LGTM! Clean early-exit and interface detection.The script correctly:
- Uses portable
/bin/shfor Alpine compatibility- Respects the debug flag for development workflows
- Safely handles missing default routes by exiting early
17-42: Bridge detection heuristics are reasonable for a non-blocking warning.The MAC and IP pattern checks cover common Docker bridge scenarios (02:42:, 172.16-31., 192.168.65.*), and the "@if" check catches veth pairs. While these heuristics may have edge cases, they're appropriate for a warning-level check that doesn't block startup.
47-64: Clear, actionable warning with appropriate non-blocking behavior.The warning message correctly:
- Uses ANSI colors for visibility
- Explains the impact of bridge networking on ARP/discovery
- Provides concrete remediation (--network=host with required capabilities)
- Exits 0 to avoid blocking startup (warning-only behavior)
.devcontainer/resources/devcontainer-Dockerfile (2)
20-22: LGTM! Package additions support the Docker integration test suite.The addition of
py3-docker-pyanddocker-clialigns with the new Docker integration tests introduced in test/docker_tests/.
31-31: Verify that world-writable permissions on /opt/venv are intentional.The
chmod o+rwxon all/opt/venvdirectories grants write access to any user, which could allow malicious code or tooling to modify the virtual environment. While this is a devcontainer (lower risk), consider using group ownership withchown -R netalertx:netalertx /opt/venvandchmod -R g+winstead to limit write access to the netalertx user and group.install/production-filesystem/services/scripts/check-mandatory-folders.sh (2)
5-50: LGTM! Robust error handling with clear feedback.The function correctly:
- Guards each directory/file creation with existence checks
- Provides explicit error messages with the full path
- Returns non-zero on any failure
- Uses consistent messaging and structure
52-53: LGTM! Implicit exit status propagation is correct.The script correctly relies on shell behavior where the last command's exit status becomes the script's exit status, so failures in
check_mandatory_folders()will propagate as non-zero exits.test/docker_tests/dockerfiles/no-error-compose.yml (2)
1-16: LGTM! Security-hardened configuration follows best practices.The configuration correctly implements:
- Host networking for ARP/scanning requirements
- Read-only filesystem to prevent tampering
- Least-privilege capabilities (drop ALL, add only NET_ADMIN/NET_RAW/NET_BIND_SERVICE)
This aligns with CIS Docker benchmarks mentioned in the PR objectives.
17-41: LGTM! Volume strategy correctly separates persistent data from runtime configuration.The mix of named volumes (for config/db persistence) and bind mounts (for timezone) is appropriate, and the commented examples provide useful guidance for customization scenarios.
test/test_compound_conditions.py (2)
26-152: LGTM! Comprehensive test coverage for compound condition parsing.The tests correctly:
- Cover the user-reported Issue #1210 scenario
- Test single and compound conditions for backward compatibility
- Verify parameter generation and SQL structure
- Use clear, descriptive test names
The pytest migration maintains all original test logic while improving readability.
229-243: LGTM! SQL injection test correctly validates security posture.The test appropriately accepts two valid security outcomes:
- Raising ValueError to reject malicious input
- Sanitizing the input to remove dangerous constructs
The assertions verify that neither
DROPnor semicolons appear in the generated SQL.docker-compose.yml (3)
55-70: LGTM! Resource limits and environment variables are well-configured.The updates include:
- New GRAPHQL_PORT for the GraphQL API
- Concrete default for ALWAYS_FRESH_INSTALL (false) instead of a template
- Appropriate resource limits (2GB max, 1GB reservation) to prevent resource exhaustion
- Clear logging configuration for troubleshooting
72-78: LGTM! Restart policy and volume definitions are appropriate.The
restart: unless-stoppedpolicy ensures the container survives host reboots while allowing manual stops, and the minimal volume definitions are sufficient for local Docker installations.
18-27: Update documentation to clarify migration path and align examples with named volumes approach.The change from
${APP_DATA_LOCATION}bind mounts to named volumes (netalertx_config,netalertx_db) is confirmed as breaking for existing deployments. However, comprehensive migration guidance exists in docs/MIGRATION.md with automatic database/config migration support, and storage validation is enforced bycheck-persistent_storage.sh.The concerns are documentation-related, not missing functionality:
- docs/DOCKER_COMPOSE.md still shows bind mount examples with environment variables as the primary approach, which contradicts the production docker-compose.yml now using named volumes
- MIGRATION.md exists but users upgrading from bind mounts may not discover it—add a banner or link in docker-compose.yml comments pointing to it
- Consider adding a brief upgrade note in docker-compose.yml explaining why named volumes are used and when users need to consult MIGRATION.md
The migration path is viable but documentation should be better integrated to guide users proactively.
install/production-filesystem/services/scripts/check-persistent_storage.sh (2)
48-52: Warning-only behavior is correctly implemented (once variable scope bug is fixed).The script appropriately:
- Provides a 5-second delay to ensure users see the warning
- Uses non-fatal exit (exit 0) to avoid blocking container startup
- Documents the warning-only intent in comments
This aligns with the script's purpose as a user-facing notification rather than a hard requirement.
4-35: Critical bug: failures variable scope prevents error detection.Line 12 assigns
failures=1inside the function, but in POSIX shell, this creates a local variable that doesn't affect the globalfailuresinitialized at line 42. The check at line 48 will never detect failures fromwarn_if_not_persistent_mount().Fix by having the function return a status code and accumulate failures in the caller:
warn_if_not_persistent_mount() { path="$1" # Check if the path is a mount point by looking for it in /proc/self/mountinfo # We are looking for an exact match in the mount point column (field 5) if awk -v target="${path}" '$5 == target {found=1} END {exit found ? 0 : 1}' /proc/self/mountinfo; then return 0 fi - failures=1 YELLOW=$(printf '\033[1;33m') RESET=$(printf '\033[0m') >&2 printf "%s" "${YELLOW}" >&2 cat <<EOF ══════════════════════════════════════════════════════════════════════════════ ⚠️ ATTENTION: ${path} is not a persistent mount. Your data in this directory may not persist across container restarts or upgrades. To ensure your settings and history are saved, you must mount this directory as a persistent volume. Fix: mount ${path} explicitly as a bind mount or a named volume: # Bind mount --mount type=bind,src=/path/on/host,dst=${path} # Named volume --mount type=volume,src=netalertx-data,dst=${path} Apply one of these mount options and restart the container. ══════════════════════════════════════════════════════════════════════════════ EOF >&2 printf "%s" "${RESET}" + return 1 } # If NETALERTX_DEBUG=1 then we will exit if [ "${NETALERTX_DEBUG}" = "1" ]; then exit 0 fi failures=0 # NETALERTX_DB is a file, so we check its directory -warn_if_not_persistent_mount "$(dirname "${NETALERTX_DB_FILE}")" -warn_if_not_persistent_mount "${NETALERTX_CONFIG}" +warn_if_not_persistent_mount "$(dirname "${NETALERTX_DB_FILE}")" || failures=$((failures + 1)) +warn_if_not_persistent_mount "${NETALERTX_CONFIG}" || failures=$((failures + 1))Likely an incorrect or invalid review comment.
install/production-filesystem/services/scripts/check-storage-extra.sh (1)
35-40: Exit behavior is correct (once variable scope bug is fixed).Unlike check-persistent_storage.sh, this script correctly exits with status 1 when failures are detected, making it a blocking check rather than a warning-only check. The 5-second delay provides time for users to read the error message.
.devcontainer/Dockerfile (2)
1-1: Auto-generated; defer review to generator and root Dockerfile.Per project learnings, this file is generated by .devcontainer/scripts/generate-configs.sh; review should target the generator and root Dockerfile, not this artifact. Skipping detailed review. Based on learnings.
148-156: Confirm intent: granting NET_RAW/NET_ADMIN caps to the venv Python binary.
setcap ... $(readlink -f ${VIRTUAL_ENV_BIN}/python)enables raw sockets for Python (e.g., scapy). Confirm this is required in Runner and remains safe post-hardened perms. If only needed in devcontainer, consider limiting to that stage.Would you like me to trace call-sites needing raw sockets and suggest stage-scoped caps?
install/production-filesystem/services/scripts/check-capabilities.sh
Outdated
Show resolved
Hide resolved
install/production-filesystem/services/scripts/check-first-run-config.sh
Outdated
Show resolved
Hide resolved
install/production-filesystem/services/scripts/check-first-run-config.sh
Outdated
Show resolved
Hide resolved
|
Coderabbit is a bit overly verbose suddenly. It's good to have more info but it takes effort to scroll to the bottom of each comment. |
There was a problem hiding this comment.
Actionable comments posted: 9
♻️ Duplicate comments (2)
install/production-filesystem/services/start-backend.sh (1)
14-15: SC2046: confirm intent for extra launch parameters; quote if single token.If the parameters file is a single argument, quote the substitution; if it’s multiple args, keep as-is. Also mirror the choice in the echo for accuracy.
Option if single token:
-echo "Starting python3 $(cat /services/config/python/backend-extra-launch-parameters 2>/dev/null) -m server > /app/log/stdout.log 2> >(tee /app/log/stderr.log >&2)" -exec python3 $(cat /services/config/python/backend-extra-launch-parameters 2>/dev/null) -m server > /app/log/stdout.log 2> >(tee /app/log/stderr.log >&2) +echo "Starting python3 \"$(cat /services/config/python/backend-extra-launch-parameters 2>/dev/null)\" -m server > /app/log/stdout.log 2> >(tee /app/log/stderr.log >&2)" +exec python3 "$(cat /services/config/python/backend-extra-launch-parameters 2>/dev/null)" -m server > /app/log/stdout.log 2> >(tee /app/log/stderr.log >&2).devcontainer/Dockerfile (1)
188-190: Remove redundant directory creation loop.This file contains the same redundant for-loop issue as the main Dockerfile (lines 185-187). Since this file is auto-generated from the root Dockerfile and
.devcontainer/resources/devcontainer-Dockerfile, fixing the issue in the source Dockerfile will resolve it here as well.Based on learnings
🧹 Nitpick comments (3)
install/production-filesystem/services/scripts/check-app-permissions.sh (1)
54-54: Minor: remove unused variable.
ALL_PATHSisn’t used.-ALL_PATHS="${READ_ONLY_PATHS} ${READ_WRITE_PATHS}"test/test_compound_conditions.py (1)
237-237: Consider prefixing unused unpacked variables with underscore.When unpacking tuples but only using one value, prefix unused variables with
_to signal intent and silence linter warnings. For example,_sql, params = builder.build_safe_condition(condition)orsql, _params = ....Apply these diffs to address the linter hints:
- sql, params = builder.build_safe_condition(condition) + _sql, params = builder.build_safe_condition(condition)- sql, params = builder.build_safe_condition(condition) + _sql, params = builder.build_safe_condition(condition)- sql, params = builder.build_safe_condition(condition) + _sql, params = builder.build_safe_condition(condition)- sql, params = builder.build_safe_condition(condition) + _sql, params = builder.build_safe_condition(condition)Also applies to: 250-250, 304-304, 315-315
Dockerfile (1)
179-190: Remove redundant directory creation loop.The
install -dcommand at line 182 already creates all directories inREAD_WRITE_FOLDERSwith the correct ownership and permissions. The subsequent for-loop (lines 185-187) repeats the same operation, which is redundant and adds unnecessary build time.Apply this diff to remove the redundant loop:
RUN chown -R ${READ_ONLY_USER}:${READ_ONLY_GROUP} ${READ_ONLY_FOLDERS} && \ chmod -R 004 ${READ_ONLY_FOLDERS} && \ find ${READ_ONLY_FOLDERS} -type d -exec chmod 005 {} + && \ install -d -o ${NETALERTX_USER} -g ${NETALERTX_GROUP} -m 700 ${READ_WRITE_FOLDERS} && \ chown -R ${NETALERTX_USER}:${NETALERTX_GROUP} ${READ_WRITE_FOLDERS} && \ chmod -R 600 ${READ_WRITE_FOLDERS} && \ find ${READ_WRITE_FOLDERS} -type d -exec chmod 700 {} + && \ chown ${READ_ONLY_USER}:${READ_ONLY_GROUP} /entrypoint.sh /opt /opt/venv && \ chmod 005 /entrypoint.sh ${SYSTEM_SERVICES}/*.sh /app /opt /opt/venv && \ - for dir in ${READ_WRITE_FOLDERS}; do \ - install -d -o ${NETALERTX_USER} -g ${NETALERTX_GROUP} -m 700 "$dir"; \ - done && \ apk del apk-tools && \ rm -Rf /var /etc/sudoers.d/* /etc/shadow /etc/gshadow /etc/sudoers \ /lib/apk /lib/firmware /lib/modules-load.d /lib/sysctl.d /mnt /home/ /root \ /srv /media && \ sed -i "/^\(${READ_ONLY_USER}\|${NETALERTX_USER}\):/!d" /etc/passwd && \ sed -i "/^\(${READ_ONLY_GROUP}\|${NETALERTX_GROUP}\):/!d" /etc/group && \ echo -ne '#!/bin/sh\n"$@"\n' > /usr/bin/sudo && chmod +x /usr/bin/sudo
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (22)
.devcontainer/Dockerfile(4 hunks).devcontainer/devcontainer.json(2 hunks).devcontainer/resources/devcontainer-Dockerfile(1 hunks).devcontainer/scripts/setup.sh(2 hunks).vscode/tasks.json(1 hunks)Dockerfile(1 hunks)install/production-filesystem/entrypoint.sh(1 hunks)install/production-filesystem/services/scripts/check-app-permissions.sh(1 hunks)install/production-filesystem/services/scripts/check-capabilities.sh(1 hunks)install/production-filesystem/services/scripts/check-first-run-config.sh(2 hunks)install/production-filesystem/services/scripts/check-nonpersistent-storage.sh(1 hunks)install/production-filesystem/services/scripts/check-persistent-storage.sh(1 hunks)install/production-filesystem/services/scripts/check-ramdisk.sh(1 hunks)install/production-filesystem/services/scripts/check-root.sh(1 hunks)install/production-filesystem/services/scripts/check-user-netalertx.sh(1 hunks)install/production-filesystem/services/scripts/update_vendors.sh(1 hunks)install/production-filesystem/services/start-backend.sh(1 hunks)install/production-filesystem/services/start-crond.sh(1 hunks)install/production-filesystem/services/start-nginx.sh(1 hunks)install/production-filesystem/services/start-php-fpm.sh(1 hunks)test/docker_tests/test_container_environment.py(1 hunks)test/test_compound_conditions.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
- .devcontainer/resources/devcontainer-Dockerfile
- install/production-filesystem/services/scripts/check-user-netalertx.sh
- install/production-filesystem/services/scripts/check-first-run-config.sh
- install/production-filesystem/services/scripts/check-root.sh
- install/production-filesystem/services/scripts/check-capabilities.sh
- install/production-filesystem/services/start-nginx.sh
🧰 Additional context used
📓 Path-based instructions (2)
**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use 'logger.mylog' for logging at levels: none, minimal, verbose, debug, or trace.
Files:
test/test_compound_conditions.pytest/docker_tests/test_container_environment.py
test/**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Tests must reside under 'test/' and use pytest.
Files:
test/test_compound_conditions.pytest/docker_tests/test_container_environment.py
🧠 Learnings (7)
📚 Learning: 2025-10-26T15:39:36.689Z
Learnt from: adamoutler
PR: jokob-sk/NetAlertX#1235
File: install/production-filesystem/services/scripts/check-first-run-config.sh:0-0
Timestamp: 2025-10-26T15:39:36.689Z
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.sh
📚 Learning: 2025-09-20T03:01:19.912Z
Learnt from: adamoutler
PR: jokob-sk/NetAlertX#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:
.devcontainer/Dockerfile
📚 Learning: 2025-09-20T02:56:24.501Z
Learnt from: adamoutler
PR: jokob-sk/NetAlertX#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:
.devcontainer/Dockerfile
📚 Learning: 2025-10-19T01:40:57.095Z
Learnt from: adamoutler
PR: jokob-sk/NetAlertX#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:
.devcontainer/Dockerfile
📚 Learning: 2025-09-20T14:13:33.635Z
Learnt from: adamoutler
PR: jokob-sk/NetAlertX#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
📚 Learning: 2025-09-20T14:13:33.635Z
Learnt from: adamoutler
PR: jokob-sk/NetAlertX#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/Dockerfile
📚 Learning: 2025-09-20T14:09:29.159Z
Learnt from: adamoutler
PR: jokob-sk/NetAlertX#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:
.devcontainer/Dockerfile
🧬 Code graph analysis (3)
install/production-filesystem/services/scripts/check-persistent-storage.sh (1)
install/production-filesystem/services/scripts/check-nonpersistent-storage.sh (1)
warn_if_not_persistent_mount(9-34)
install/production-filesystem/services/scripts/check-nonpersistent-storage.sh (1)
install/production-filesystem/services/scripts/check-persistent-storage.sh (1)
warn_if_not_persistent_mount(38-67)
test/test_compound_conditions.py (1)
server/db/sql_safe_builder.py (2)
SafeConditionBuilder(23-623)build_safe_condition(122-151)
🪛 Ruff (0.14.1)
test/test_compound_conditions.py
237-237: Unpacked variable params is never used
Prefix it with an underscore or any other dummy variable pattern
(RUF059)
250-250: Unpacked variable sql is never used
Prefix it with an underscore or any other dummy variable pattern
(RUF059)
304-304: Unpacked variable sql is never used
Prefix it with an underscore or any other dummy variable pattern
(RUF059)
315-315: Unpacked variable sql is never used
Prefix it with an underscore or any other dummy variable pattern
(RUF059)
test/docker_tests/test_container_environment.py
34-34: subprocess call: check for execution of untrusted input
(S603)
35-35: Starting a process with a partial executable path
(S607)
44-44: subprocess call: check for execution of untrusted input
(S603)
45-45: Starting a process with a partial executable path
(S607)
55-55: Avoid specifying long messages outside the exception class
(TRY003)
76-76: subprocess call: check for execution of untrusted input
(S603)
83-83: Avoid specifying long messages outside the exception class
(TRY003)
92-92: Loop control variable target not used within loop body
Rename unused target to _target
(B007)
180-180: Probable insecure usage of temporary file or directory: "/tmp:mode=777"
(S108)
225-225: subprocess call: check for execution of untrusted input
(S603)
242-243: try-except-pass detected, consider logging the exception
(S110)
242-242: Do not catch blind exception: Exception
(BLE001)
249-249: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
252-256: Avoid specifying long messages outside the exception class
(TRY003)
698-698: Avoid specifying long messages outside the exception class
(TRY003)
🪛 Shellcheck (0.11.0)
install/production-filesystem/services/scripts/check-nonpersistent-storage.sh
[warning] 5-5: In POSIX sh, == in place of = is undefined.
(SC3014)
install/production-filesystem/services/start-backend.sh
[warning] 15-15: Quote this to prevent word splitting.
(SC2046)
install/production-filesystem/services/scripts/check-app-permissions.sh
[warning] 54-54: ALL_PATHS appears unused. Verify use (or export if used externally).
(SC2034)
⏰ 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 (6)
.vscode/tasks.json (1)
165-167: Confirmdocker buildxchoice.The task uses
docker buildx buildrather than the simplerdocker build. Verify this is intentional (e.g., for multi-platform builds) or consider usingdocker buildfor standard single-platform scenarios.install/production-filesystem/services/scripts/check-ramdisk.sh (1)
40-48: Scoping explanation is incorrect; refactored diffs are flawed. Core issue exists but proposed solution is wrong.The review comment contains a critical error in its analysis:
Scoping claim is incorrect: The statement that
warn_if_not_dedicated_mountsetsfailures=1"locally (scoped to the function)" is wrong. Without thelocalkeyword,failures=1inside the function modifies the globalfailuresvariable. The variable IS properly propagated.Core issue is real: The script does exit unconditionally with
exit 0(line 48) regardless offailuresvalue, and the conf.active check (lines 43–45) doesn't contribute tofailures. This contradicts security hardening objectives.Proposed diffs are flawed: The refactored
warn_if_not_dedicated_mountfunction removes the successful mount detection logic (theif awk... return 0block). This would make the function always fail and print warnings even for properly configured mounts, causing false positives.Correct minimal fix: Change
exit 0toexit "${failures}"and setfailures=1when conf.active is not writable. The function itself needs no refactoring.Likely an incorrect or invalid review comment.
install/production-filesystem/services/start-crond.sh (1)
26-26: Echo looks good; keeps command visibility without behavior change..devcontainer/scripts/setup.sh (1)
28-65: LGTM—robust docker socket access configuration.The
ensure_docker_socket_accessfunction is well-structured with proper error handling, fallback mechanisms for group resolution, and idempotent group membership updates. The approach of resolving the socket GID and ensuring the netalertx user is in the corresponding group is sound..devcontainer/devcontainer.json (1)
26-28: LGTM—devcontainer lifecycle configuration is well-structured.The docker socket mount enables integration testing, and the lifecycle commands properly separate dependency installation (postCreateCommand) from environment setup (postStartCommand). This aligns well with the setup.sh changes and supports the PR's testing objectives.
Also applies to: 46-51
install/production-filesystem/services/scripts/check-persistent-storage.sh (1)
76-77: The review comment is incorrect and should be dismissed.The variables
NETALERTX_DB_FILEandNETALERTX_CONFIGare defined in the Dockerfile asENVvariables (lines 46 and 51), which automatically makes them available to all processes within the container at runtime. The entrypoint.sh sources check-persistent-storage.sh via a loop that runs allcheck-*.shscripts, and these variables are inherited by that process.This approach is consistent across the codebase: other scripts in the same directory (check-first-run-db.sh, check-first-run-config.sh, check-app-permissions.sh) use these same variables without explicit validation. Docker's
ENVdirective provides implicit guarantee of availability, eliminating the need for additional checks.Likely an incorrect or invalid review comment.
install/production-filesystem/services/scripts/check-nonpersistent-storage.sh
Show resolved
Hide resolved
install/production-filesystem/services/scripts/check-persistent-storage.sh
Show resolved
Hide resolved
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (8)
install/production-filesystem/entrypoint.sh (2)
86-86: Quote variable expansion to protect against spaces.
${SYSTEM_SERVICES_SCRIPTS}should be quoted to prevent word splitting if the path contains spaces.-bash ${SYSTEM_SERVICES_SCRIPTS}/update_vendors.sh & +bash "${SYSTEM_SERVICES_SCRIPTS}/update_vendors.sh" &
56-56: Initialize FAILED_STATUS to "0" for numeric consistency.Line 56 initializes
FAILED_STATUS=""(empty), but line 280 uses it with the numeric-eqoperator. When no service fails, FAILED_STATUS remains empty, and[ "" -eq 0 ]is fragile. Initializing to"0"clarifies the numeric intent and ensures consistent behavior.-FAILED_STATUS="" +FAILED_STATUS="0"Then at line 280, the logic remains the same but is now robust:
-if [ "${FAILED_STATUS}" -eq 0 ] && [ "${FAILED_NAME}" != "signal" ]; then +if [ "${FAILED_STATUS}" -eq 0 ] && [ "${FAILED_NAME}" != "signal" ]; then FAILED_STATUS=1 fiAlso applies to: 280-282
test/docker_tests/test_container_environment.py (6)
1-8: Use logger.mylog for debug output; drop broad except.Aligns with coding_guidelines and reduces noisy prints.
@@ -import os +import os @@ -import pytest +import pytest +import logger @@ - # Print the full Docker command for debugging - print("\n--- DOCKER CMD ---\n", " ".join(cmd), "\n--- END CMD ---\n") + # Debug: full Docker command + logger.mylog("debug", f"DOCKER CMD: {' '.join(cmd)}") @@ - # Print container output for debugging in every test run. - try: - print("\n--- CONTAINER out ---\n", result.output) - except Exception: - pass + # Debug: container output + logger.mylog("debug", f"CONTAINER OUT:\n{result.output}")Also applies to: 223-243
631-641: Make readonly nginx_conf test actually read-only.Currently simulates zero perms, not a read-only mount. Match other readonly tests.
def test_readonly_nginx_conf_mount(tmp_path: pathlib.Path) -> None: @@ - paths = _setup_mount_tree(tmp_path, "readonly_nginx_conf") - _setup_zero_perm_dir(paths, "nginx_conf") - volumes = _build_volume_args(paths) - try: - result = _run_container("readonly-nginx-conf", volumes) - _assert_contains(result, "Write permission denied", result.args) - _assert_contains(result, "/services/config/nginx/conf.active", result.args) - assert result.returncode != 0 - finally: - _restore_zero_perm_dir(paths, "nginx_conf") + paths = _setup_mount_tree(tmp_path, "readonly_nginx_conf") + volumes = _build_volume_args(paths, read_only={"nginx_conf"}) + result = _run_container("readonly-nginx-conf", volumes) + _assert_contains(result, "Write permission denied", result.args) + _assert_contains(result, str(VOLUME_MAP["nginx_conf"]), result.args) + assert result.returncode != 0
637-638: Use VOLUME_MAP for asserted paths to avoid drift.Prefer VOLUME_MAP lookups over hard-coded strings.
- _assert_contains(result, "/services/config/nginx/conf.active", result.args) + _assert_contains(result, str(VOLUME_MAP["nginx_conf"]), result.args) @@ - _assert_contains(result, "/services/config/nginx/conf.active", result.args) + _assert_contains(result, str(VOLUME_MAP["nginx_conf"]), result.args) @@ - _assert_contains(result, "/app/config", result.args) + _assert_contains(result, str(VOLUME_MAP["app_config"]), result.args) @@ - _assert_contains(result, "/app/db", result.args) + _assert_contains(result, str(VOLUME_MAP["app_db"]), result.args)Also applies to: 679-680, 905-907, 924-926
223-231: Optional: gate verbose logs behind env var.Keeps CI output clean; enable with NETALERTX_TEST_DEBUG=1.
- logger.mylog("debug", f"DOCKER CMD: {' '.join(cmd)}") + if os.environ.get("NETALERTX_TEST_DEBUG"): + logger.mylog("debug", f"DOCKER CMD: {' '.join(cmd)}") @@ - logger.mylog("debug", f"CONTAINER OUT:\n{result.output}") + if os.environ.get("NETALERTX_TEST_DEBUG"): + logger.mylog("debug", f"CONTAINER OUT:\n{result.output}")
25-26: Optional: autouse check for Docker CLI, skip if unavailable.Prevents confusing failures on dev machines/CI runners without Docker.
Add near top of file:
@pytest.fixture(autouse=True, scope="module") def _require_docker(): if shutil.which("docker") is None: pytest.skip("Docker CLI not found in PATH; skipping docker tests.")Want a PR to add this across docker test modules?
9-11: Track TODOs with issues.ALWAYS_FRESH_INSTALL and named volume mount still untested.
I can open issues and scaffold tests for these. Proceed?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.devcontainer/devcontainer.json(2 hunks).vscode/tasks.json(1 hunks)install/production-filesystem/entrypoint.sh(1 hunks)server/api_server/nettools_endpoint.py(3 hunks)test/docker_tests/test_container_environment.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- .vscode/tasks.json
🧰 Additional context used
📓 Path-based instructions (3)
**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use 'logger.mylog' for logging at levels: none, minimal, verbose, debug, or trace.
Files:
test/docker_tests/test_container_environment.pyserver/api_server/nettools_endpoint.py
test/**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Tests must reside under 'test/' and use pytest.
Files:
test/docker_tests/test_container_environment.py
server/api_server/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
When introducing new server functionality, add endpoints only within 'server/api_server/*' and keep authorization checks consistent.
Files:
server/api_server/nettools_endpoint.py
🧠 Learnings (3)
📚 Learning: 2025-09-20T03:01:19.912Z
Learnt from: adamoutler
PR: jokob-sk/NetAlertX#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:
.devcontainer/devcontainer.json
📚 Learning: 2025-10-26T16:45:41.228Z
Learnt from: adamoutler
PR: jokob-sk/NetAlertX#1235
File: install/production-filesystem/services/scripts/check-ramdisk.sh:0-0
Timestamp: 2025-10-26T16:45:41.228Z
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.sh
📚 Learning: 2025-10-26T15:39:36.689Z
Learnt from: adamoutler
PR: jokob-sk/NetAlertX#1235
File: install/production-filesystem/services/scripts/check-first-run-config.sh:0-0
Timestamp: 2025-10-26T15:39:36.689Z
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.sh
🪛 Ruff (0.14.1)
test/docker_tests/test_container_environment.py
34-34: subprocess call: check for execution of untrusted input
(S603)
35-35: Starting a process with a partial executable path
(S607)
44-44: subprocess call: check for execution of untrusted input
(S603)
45-45: Starting a process with a partial executable path
(S607)
55-55: Avoid specifying long messages outside the exception class
(TRY003)
76-76: subprocess call: check for execution of untrusted input
(S603)
83-83: Avoid specifying long messages outside the exception class
(TRY003)
92-92: Loop control variable target not used within loop body
Rename unused target to _target
(B007)
180-180: Probable insecure usage of temporary file or directory: "/tmp:mode=777"
(S108)
225-225: subprocess call: check for execution of untrusted input
(S603)
242-243: try-except-pass detected, consider logging the exception
(S110)
242-242: Do not catch blind exception: Exception
(BLE001)
249-249: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
252-256: Avoid specifying long messages outside the exception class
(TRY003)
698-698: Avoid specifying long messages outside the exception class
(TRY003)
server/api_server/nettools_endpoint.py
22-24: Avoid specifying long messages outside the exception class
(TRY003)
26-26: Avoid specifying long messages outside the exception class
(TRY003)
31-31: Do not catch blind exception: Exception
(BLE001)
115-115: subprocess call: check for execution of untrusted input
(S603)
133-133: Do not catch blind exception: Exception
(BLE001)
⏰ 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 (8)
install/production-filesystem/entrypoint.sh (1)
56-77: Shell test bug fixed.The prior review flagged the unsafe
[ ${FAILED_STATUS} ]pattern underset -u. Line 74 now correctly uses[ -n "${FAILED_STATUS}" ]with proper quoting and the non-empty check. ✓.devcontainer/devcontainer.json (3)
26-28: Verify Docker socket mounting intent and document security implications.Binding
/var/run/docker.sockgrants the container significant privileges (effectively root access to the host Docker daemon). Confirm this is necessary for integration tests and document the security trade-off.Consider adding a comment explaining why this is required:
"mounts": [ - "source=/var/run/docker.sock,target=/var/run/docker.sock,type=bind" //used for testing various conditions in docker + "source=/var/run/docker.sock,target=/var/run/docker.sock,type=bind" // WARNING: grants root-equivalent access for docker integration tests ],
49-52: Verify postStartCommand execution order and timing.The background build process (
nohup screen docker buildx build...) may have ordering issues or race conditions with other setup steps. Verify that:
- The setup.sh script (
${containerWorkspaceFolder}/.devcontainer/scripts/setup.sh) completes before dependent tasks- The docker buildx command properly handles Docker socket initialization from setup.sh
Consider making the docker buildx build sequential if test execution depends on the image being ready:
- "postStartCommand": { - "Start Environment":"${containerWorkspaceFolder}/.devcontainer/scripts/setup.sh", - "Build test-container":"nohup screen docker buildx build --platform linux/amd64 --tag netalertx-test . & disown" + "postStartCommand": { + "Start Environment":"${containerWorkspaceFolder}/.devcontainer/scripts/setup.sh && docker buildx build --platform linux/amd64 --tag netalertx-test ." },
46-48: Verify pip package versions.Ensure the installed packages (
pytest,docker,debugpy) are pinned to compatible versions or explicitly allow any version. Currently unpinned packages may introduce unexpected behavior changes.Consider pinning versions:
- "Install Pip Requirements": "/opt/venv/bin/pip3 install pytest docker debugpy" + "Install Pip Requirements": "/opt/venv/bin/pip3 install pytest>=7.0 docker>=6.0 debugpy>=1.6"server/api_server/nettools_endpoint.py (4)
5-6: LGTM: Appropriate imports for path resolution.
105-111: LGTM: Proper graceful degradation.Returning 503 when the CLI is unavailable clearly communicates service unavailability to callers.
113-120: LGTM: Security concern resolved.Using the validated absolute path
SPEEDTEST_CLI_PATHeliminates the PATH hijacking risk identified in the previous review. The S603 static analysis warning is a false positive here.
133-138: LGTM: Reasonable defensive error handling.The broad exception handler ensures unexpected errors return a proper HTTP 500 response rather than crashing the endpoint.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
test/docker_tests/test_container_environment.py (1)
114-127: Hardcoded seed paths still break portability — unresolved from prior review.These absolute
/workspaces/NetAlertX/...paths prevent running tests outside the devcontainer. Resolve paths relative to the repo root or use an environment variable override, and skip gracefully if source files are missing.Apply this diff:
+ repo_root = pathlib.Path(os.environ.get("NETALERTX_REPO_ROOT", str(pathlib.Path(__file__).resolve().parents[2]))) + if seed_config: config_file = paths["app_config"] / "app.conf" + cfg_src = repo_root / "back" / "app.conf" + if not cfg_src.exists(): + pytest.skip(f"Seed file not found: {cfg_src}") shutil.copyfile( - "/workspaces/NetAlertX/back/app.conf", + cfg_src, config_file, ) config_file.chmod(0o600) if seed_db: db_file = paths["app_db"] / "app.db" + db_src = repo_root / "db" / "app.db" + if not db_src.exists(): + pytest.skip(f"Seed file not found: {db_src}") shutil.copyfile( - "/workspaces/NetAlertX/db/app.db", + db_src, db_file, ) db_file.chmod(0o600)
🧹 Nitpick comments (2)
test/docker_tests/test_container_environment.py (2)
823-841: Consider asserting expected warning message.The test validates exit code but doesn't verify the expected "NetAlertX is running as UID 1000:1000" warning mentioned in the docstring. Adding an output assertion would make the test more robust.
result = _run_container( "run-as-1000", volumes, user="1000:1000", ) + _assert_contains(result, "NetAlertX is running as UID", result.args) assert result.returncode != 0
844-862: Consider asserting expected warning message.The test validates exit code but doesn't verify the expected "not running with --network=host" warning mentioned in the docstring. Adding an output assertion would strengthen the test.
result = _run_container( "missing-host-network", volumes, network_mode=None, ) + _assert_contains(result, "not running with --network=host", result.args) assert result.returncode != 0
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.devcontainer/devcontainer.json(2 hunks)test/docker_tests/test_container_environment.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- .devcontainer/devcontainer.json
🧰 Additional context used
📓 Path-based instructions (2)
**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use 'logger.mylog' for logging at levels: none, minimal, verbose, debug, or trace.
Files:
test/docker_tests/test_container_environment.py
test/**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Tests must reside under 'test/' and use pytest.
Files:
test/docker_tests/test_container_environment.py
🪛 Ruff (0.14.1)
test/docker_tests/test_container_environment.py
42-42: subprocess call: check for execution of untrusted input
(S603)
43-43: Starting a process with a partial executable path
(S607)
52-52: subprocess call: check for execution of untrusted input
(S603)
53-53: Starting a process with a partial executable path
(S607)
63-63: Avoid specifying long messages outside the exception class
(TRY003)
84-84: subprocess call: check for execution of untrusted input
(S603)
91-91: Avoid specifying long messages outside the exception class
(TRY003)
100-100: Loop control variable target not used within loop body
Rename unused target to _target
(B007)
188-188: Probable insecure usage of temporary file or directory: "/tmp:mode=777"
(S108)
233-233: subprocess call: check for execution of untrusted input
(S603)
250-251: try-except-pass detected, consider logging the exception
(S110)
250-250: Do not catch blind exception: Exception
(BLE001)
257-257: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
260-264: Avoid specifying long messages outside the exception class
(TRY003)
706-706: Avoid specifying long messages outside the exception class
(TRY003)
⏰ 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
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (9)
test/docker_tests/test_container_environment.py (9)
9-16: Add skip guards for non‑Linux or missing Docker.Prevents hard errors in CI/dev where Docker/host networking isn’t available.
@@ -import os +import os +import sys @@ -pytestmark = [pytest.mark.docker, pytest.mark.feature_complete] +pytestmark = [ + pytest.mark.docker, + pytest.mark.feature_complete, + pytest.mark.skipif(shutil.which("docker") is None, reason="Docker not installed"), + pytest.mark.skipif(sys.platform != "linux", reason="Requires Linux host (--network=host)"), +]Also applies to: 33-34
100-113: Silence unused loop var.
targetisn’t used in this loop.- for key, target in VOLUME_MAP.items(): + for key, _ in VOLUME_MAP.items():
114-146: Repo‑relative seed paths look good.Nice removal of hard‑coded /workspaces; fallback discovery is sane. Consider skipping with pytest.skip instead of printing when seeds are absent to avoid false positives.
83-91: Bound the chown helper.Add a timeout so a hung
docker rundoesn’t stall the suite.subprocess.run( cmd, - check=True, + check=True, stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL, + timeout=60, )
249-270: Use logger.mylog instead of print.Per coding_guidelines for **/*.py. Gate verbosity (e.g., minimal/debug) to keep CI logs readable.
275-283: Modernize Optional typing.Align with usage elsewhere in this file.
-def _assert_contains(result, snippet: str, cmd: list[str] = None) -> None: +def _assert_contains(result, snippet: str, cmd: list[str] | None = None) -> None:
187-229: Minor: make docker args explicit and resilient.Optionally add
--pull=neverto avoid slow accidental pulls and set--initto reap child if entrypoint forks.
316-436: Reduce duplication with parametrization.These blocks share the same structure. Parametrize over mount key, user, and expected snippet to cut runtime and maintenance.
Also applies to: 438-588, 590-682, 710-798, 883-911, 913-952
40-49: Remove unused test helper functions.
_create_docker_volumeand_remove_docker_volumeare not referenced anywhere in the codebase. Remove them.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
test/docker_tests/test_container_environment.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use 'logger.mylog' for logging at levels: none, minimal, verbose, debug, or trace.
Files:
test/docker_tests/test_container_environment.py
test/**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Tests must reside under 'test/' and use pytest.
Files:
test/docker_tests/test_container_environment.py
🪛 Ruff (0.14.1)
test/docker_tests/test_container_environment.py
42-42: subprocess call: check for execution of untrusted input
(S603)
43-43: Starting a process with a partial executable path
(S607)
52-52: subprocess call: check for execution of untrusted input
(S603)
53-53: Starting a process with a partial executable path
(S607)
63-63: Avoid specifying long messages outside the exception class
(TRY003)
84-84: subprocess call: check for execution of untrusted input
(S603)
91-91: Avoid specifying long messages outside the exception class
(TRY003)
100-100: Loop control variable target not used within loop body
Rename unused target to _target
(B007)
206-206: Probable insecure usage of temporary file or directory: "/tmp:mode=777"
(S108)
251-251: subprocess call: check for execution of untrusted input
(S603)
268-269: try-except-pass detected, consider logging the exception
(S110)
268-268: Do not catch blind exception: Exception
(BLE001)
275-275: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
278-282: Avoid specifying long messages outside the exception class
(TRY003)
724-724: Avoid specifying long messages outside the exception class
(TRY003)
⏰ 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 (1)
test/docker_tests/test_container_environment.py (1)
710-798: Path assertions now match skipped mounts.Good cleanup on mismatched paths.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (6)
test/docker_tests/test_container_environment.py (6)
129-146: Use logger.mylog; avoid unconditional prints.Replace prints with logger.mylog per guidelines and drop the broad except.
As per coding guidelines.@@ - print(f"[WARN] Seed file not found: {config_src}. Set NETALERTX_REPO_ROOT or run from repo root. Skipping copy.") + logger.mylog("minimal", f"Seed file not found: {config_src}. Set NETALERTX_REPO_ROOT or run from repo root. Skipping copy.") @@ - print(f"[WARN] Seed file not found: {db_src}. Set NETALERTX_REPO_ROOT or run from repo root. Skipping copy.") + logger.mylog("minimal", f"Seed file not found: {db_src}. Set NETALERTX_REPO_ROOT or run from repo root. Skipping copy.") @@ - print("\n--- DOCKER CMD ---\n", " ".join(cmd), "\n--- END CMD ---\n") + logger.mylog("debug", " ".join(cmd)) @@ - try: - print("\n--- CONTAINER out ---\n", result.output) - except Exception: - pass + try: + logger.mylog("trace", result.output) + except UnicodeEncodeError: + logger.mylog("trace", result.output.encode("utf-8", "replace").decode("utf-8"))Add near imports:
import logger # for logger.mylogAlso applies to: 249-251, 266-269
205-207: Use sticky bit for /tmp tmpfs./tmp should be 1777, not 0777.
- cmd.extend(["--tmpfs", "/tmp:mode=777"]) + cmd.extend(["--tmpfs", "/tmp:mode=1777"])
275-275: Explicit Optional typing for cmd.Fix implicit Optional per Ruff RUF013.
-def _assert_contains(result, snippet: str, cmd: list[str] = None) -> None: +def _assert_contains(result, snippet: str, cmd: list[str] | None = None) -> None:
100-101: Rename unused loop variable.Avoid shadowing and satisfy Ruff B007.
- for key, target in VOLUME_MAP.items(): + for key, _ in VOLUME_MAP.items():
20-23: Preflight skip when Docker or image missing.Add a session fixture to skip the module if
dockerorIMAGEis unavailable.# Add in this module or conftest.py import shutil, subprocess, pytest @pytest.fixture(scope="session", autouse=True) def _require_docker_image(): if not shutil.which("docker"): pytest.skip("Docker CLI not found") image = os.environ.get("NETALERTX_TEST_IMAGE", "netalertx-test") if subprocess.run(["docker","image","inspect",image], stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL).returncode != 0: pytest.skip(f"Test image not present: {image}")
710-725: Assert non‑zero exit for misconfig scenarios.Harden tests: also assert
result.returncode != 0to catch regressions.@@ def test_missing_mount_app_db(... - raise AssertionError("Expected persistent mount warning for /app/db in container output.") + raise AssertionError("Expected persistent mount warning for /app/db in container output.") + assert result.returncode != 0 @@ def test_missing_mount_app_config(... - _assert_contains(result, "/app/config", result.args) + _assert_contains(result, "/app/config", result.args) + assert result.returncode != 0 @@ def test_missing_mount_app_log(... - _assert_contains(result, "/app/log", result.args) + _assert_contains(result, "/app/log", result.args) + assert result.returncode != 0 @@ def test_missing_mount_app_api(... - _assert_contains(result, "/app/api", result.args) + _assert_contains(result, "/app/api", result.args) + assert result.returncode != 0 @@ def test_missing_mount_services_run(... - _assert_contains(result, "Container startup checks failed with exit code", result.args) + _assert_contains(result, "Container startup checks failed with exit code", result.args) + assert result.returncode != 0 @@ def test_tmpfs_config_mount_warns(... - _assert_contains(result, "/app/config", result.args) + _assert_contains(result, "/app/config", result.args) + assert result.returncode != 0Also applies to: 734-739, 749-753, 762-767, 791-797, 925-935
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
test/docker_tests/test_container_environment.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use 'logger.mylog' for logging at levels: none, minimal, verbose, debug, or trace.
Files:
test/docker_tests/test_container_environment.py
test/**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Tests must reside under 'test/' and use pytest.
Files:
test/docker_tests/test_container_environment.py
🪛 Ruff (0.14.1)
test/docker_tests/test_container_environment.py
42-42: subprocess call: check for execution of untrusted input
(S603)
43-43: Starting a process with a partial executable path
(S607)
52-52: subprocess call: check for execution of untrusted input
(S603)
53-53: Starting a process with a partial executable path
(S607)
63-63: Avoid specifying long messages outside the exception class
(TRY003)
84-84: subprocess call: check for execution of untrusted input
(S603)
91-91: Avoid specifying long messages outside the exception class
(TRY003)
100-100: Loop control variable target not used within loop body
Rename unused target to _target
(B007)
206-206: Probable insecure usage of temporary file or directory: "/tmp:mode=777"
(S108)
251-251: subprocess call: check for execution of untrusted input
(S603)
268-269: try-except-pass detected, consider logging the exception
(S110)
268-268: Do not catch blind exception: Exception
(BLE001)
275-275: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
278-282: Avoid specifying long messages outside the exception class
(TRY003)
724-724: Avoid specifying long messages outside the exception class
(TRY003)
⏰ 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
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
test/docker_tests/test_container_environment.py (2)
100-112: Rename unused loop variable to follow Python convention.The loop variable
targetis not used within the loop body. Per Python convention, unused variables should be prefixed with underscore.Apply this diff:
- for key, target in VOLUME_MAP.items(): + for key, _target in VOLUME_MAP.items(): folder_name = f"{label}_{key.upper()}_INTENTIONAL_NETALERTX_TEST" host_path = base / folder_name host_path.mkdir(parents=True, exist_ok=True)
275-275: Make Optional type hint explicit.The parameter
cmduses implicitOptionalwhich is prohibited by PEP 484.Apply this diff:
-def _assert_contains(result, snippet: str, cmd: list[str] = None) -> None: +def _assert_contains(result, snippet: str, cmd: list[str] | None = None) -> None:
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
test/docker_tests/test_container_environment.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use 'logger.mylog' for logging at levels: none, minimal, verbose, debug, or trace.
Files:
test/docker_tests/test_container_environment.py
test/**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Tests must reside under 'test/' and use pytest.
Files:
test/docker_tests/test_container_environment.py
🪛 Ruff (0.14.1)
test/docker_tests/test_container_environment.py
42-42: subprocess call: check for execution of untrusted input
(S603)
43-43: Starting a process with a partial executable path
(S607)
52-52: subprocess call: check for execution of untrusted input
(S603)
53-53: Starting a process with a partial executable path
(S607)
63-63: Avoid specifying long messages outside the exception class
(TRY003)
84-84: subprocess call: check for execution of untrusted input
(S603)
91-91: Avoid specifying long messages outside the exception class
(TRY003)
100-100: Loop control variable target not used within loop body
Rename unused target to _target
(B007)
206-206: Probable insecure usage of temporary file or directory: "/tmp:mode=777"
(S108)
251-251: subprocess call: check for execution of untrusted input
(S603)
268-269: try-except-pass detected, consider logging the exception
(S110)
268-268: Do not catch blind exception: Exception
(BLE001)
275-275: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
278-282: Avoid specifying long messages outside the exception class
(TRY003)
720-720: Avoid specifying long messages outside the exception class
(TRY003)
⏰ 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 (2)
test/docker_tests/test_container_environment.py (2)
706-720: LGTM! Robust assertion with helpful debug output.The custom assertion logic properly checks for both the warning message and the specific path, with clear debug output on failure. The use of root user (line 713) correctly bypasses permission checks to isolate the missing mount detection.
316-950: Comprehensive test coverage for container security hardening scenarios.The test suite thoroughly covers all security hardening scenarios outlined in the PR objectives:
- Root-owned mounts detection
- Zero-permission handling
- Read-only mount validation
- Missing mount warnings
- Capability requirements
- User context validation
- Network mode requirements
- Automatic seeding behavior
- Tmpfs vs persistent storage warnings
The consistent test structure and clear documentation make this excellent integration test coverage.
There was a problem hiding this comment.
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)
install/production-filesystem/entrypoint.sh (1)
291-293: Numeric comparison will fail on empty string.If the service monitoring loop exits naturally without failures,
FAILED_STATUSremains an empty string (initialized at line 56). The numeric comparison-eqwill error: "integer expression expected."Initialize with a number or use a safer comparison:
-if [ "${FAILED_STATUS}" -eq 0 ] && [ "${FAILED_NAME}" != "signal" ]; then +if [ -z "${FAILED_STATUS}" ] && [ "${FAILED_NAME}" != "signal" ]; then FAILED_STATUS=1 fi
♻️ Duplicate comments (1)
test/docker_tests/test_entrypoint.py (1)
36-82: Test coverage is comprehensive.The four test cases effectively verify entrypoint behavior for check-skipping, APP_CONF_OVERRIDE logic, and environment interactions. Well-structured with appropriate markers.
Note: The
SKIP_TESTSvs.DISABLE_STARTUP_CHECKSmismatch is addressed in the entrypoint.sh review.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
install/production-filesystem/entrypoint.sh(1 hunks)test/docker_tests/test_entrypoint.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use 'logger.mylog' for logging at levels: none, minimal, verbose, debug, or trace.
Files:
test/docker_tests/test_entrypoint.py
test/**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Tests must reside under 'test/' and use pytest.
Files:
test/docker_tests/test_entrypoint.py
🧠 Learnings (1)
📚 Learning: 2025-10-26T16:45:41.228Z
Learnt from: adamoutler
PR: jokob-sk/NetAlertX#1235
File: install/production-filesystem/services/scripts/check-ramdisk.sh:0-0
Timestamp: 2025-10-26T16:45:41.228Z
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.sh
🪛 Ruff (0.14.1)
test/docker_tests/test_entrypoint.py
21-21: Probable insecure usage of temporary file or directory: "/tmp:mode=777"
(S108)
33-33: subprocess call: check for execution of untrusted input
(S603)
⏰ 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 (4)
install/production-filesystem/entrypoint.sh (3)
83-87: LGTM!The APP_CONF_OVERRIDE logic correctly checks both conditions and the tests verify all scenarios properly.
90-93: LGTM!The check-only mode implementation is correct and properly integrated with the tests.
97-97: LGTM!The switch from
shtobashaligns with the broader shell-tooling improvements in this PR.test/docker_tests/test_entrypoint.py (1)
15-33: LGTM!The helper function is well-implemented for Docker integration testing. The static analysis warnings about tmpfs security and subprocess usage are false positives—both are appropriate in test containers.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
commit 90a07c6 Merge: 13341e3 031d810 Author: jokob-sk <jokob.sk@gmail.com> Date: Mon Nov 3 08:14:26 2025 +1100 Merge branch 'main' of https://github.com/jokob-sk/NetAlertX commit 13341e3 Author: jokob-sk <jokob.sk@gmail.com> Date: Mon Nov 3 08:14:15 2025 +1100 PLG: ARPSCAN prevent duplicates across subnets Signed-off-by: jokob-sk <jokob.sk@gmail.com> commit 031d810 Merge: cb69990 b806f84 Author: Jokob @netalertx <96159884+jokob-sk@users.noreply.github.com> Date: Sun Nov 2 22:20:13 2025 +1100 Merge branch `next_release` into main commit b806f84 Author: jokob-sk <jokob.sk@gmail.com> Date: Sun Nov 2 22:16:28 2025 +1100 BE: invlaid return netalertx#1251 Signed-off-by: jokob-sk <jokob.sk@gmail.com> commit 7c90c2e Author: jokob-sk <jokob.sk@gmail.com> Date: Sun Nov 2 22:12:30 2025 +1100 BE: spinner + timestamp work netalertx#1251 Signed-off-by: jokob-sk <jokob.sk@gmail.com> commit cb69990 Merge: 71646e1 7037cf1 Author: Jokob @netalertx <96159884+jokob-sk@users.noreply.github.com> Date: Sun Nov 2 21:48:27 2025 +1100 Merge pull request netalertx#1268 from adamoutler/synology-fix Fix permissions on Synology commit 7037cf1 Author: Adam Outler <adamoutler@gmail.com> Date: Sun Nov 2 10:26:21 2025 +0000 fxi permissions on synology inherited commit a27ee5c Author: jokob-sk <jokob.sk@gmail.com> Date: Sun Nov 2 13:55:51 2025 +1100 BE: changes netalertx#1251 Signed-off-by: jokob-sk <jokob.sk@gmail.com> commit c3c570e Author: jokob-sk <jokob.sk@gmail.com> Date: Sun Nov 2 13:51:17 2025 +1100 BE: added stateUpdated netalertx#1251 Signed-off-by: jokob-sk <jokob.sk@gmail.com> commit 71646e1 Merge: e7ed9e0 dde542c Author: Jokob @netalertx <96159884+jokob-sk@users.noreply.github.com> Date: Sun Nov 2 13:49:39 2025 +1100 Merge pull request netalertx#1263 from adamoutler/FEAT--Make-Errors-More-Helpful Feat: make errors more helpful commit 2215272 Author: jokob-sk <jokob.sk@gmail.com> Date: Sun Nov 2 11:57:08 2025 +1100 BE: short-circuit of name resolution netalertx#1251 Signed-off-by: jokob-sk <jokob.sk@gmail.com> commit dde542c Author: Adam Outler <adamoutler@gmail.com> Date: Sun Nov 2 00:12:50 2025 +0000 make /services/scripts executable by default commit 23a0fac Author: Adam Outler <adamoutler@gmail.com> Date: Sat Nov 1 23:54:54 2025 +0000 Address Coderabbit issue commit 2fdecce Author: jokob-sk <jokob.sk@gmail.com> Date: Sun Nov 2 09:07:59 2025 +1100 PLG: NMAPDEV stripping --vlan netalertx#1264 Signed-off-by: jokob-sk <jokob.sk@gmail.com> commit db5381d Author: Adam Outler <adamoutler@gmail.com> Date: Sat Nov 1 15:12:54 2025 -0400 Update test/docker_tests/test_docker_compose_scenarios.py Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> commit f1fbc47 Author: Adam Outler <adamoutler@gmail.com> Date: Sat Nov 1 19:04:31 2025 +0000 coderabbit required fix commit 2a9d352 Author: Adam Outler <adamoutler@gmail.com> Date: Sat Nov 1 14:57:57 2025 -0400 Update test/docker_tests/configurations/test_all_docker_composes.sh Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> commit 51aa3d4 Author: Adam Outler <adamoutler@gmail.com> Date: Sat Nov 1 18:53:07 2025 +0000 coderabbit commit 70373b1 Author: Adam Outler <adamoutler@gmail.com> Date: Sat Nov 1 18:18:32 2025 +0000 Address coderabbit-discoverd issues commit e7ed9e0 Author: jokob-sk <jokob.sk@gmail.com> Date: Sat Nov 1 17:58:22 2025 +1100 BE: logging fix and comments why eve_PendingAlertEmail not cleared Signed-off-by: jokob-sk <jokob.sk@gmail.com> commit 79887f0 Merge: a6bc96d ff96d38 Author: Adam Outler <adamoutler@gmail.com> Date: Fri Oct 31 23:59:45 2025 -0400 Merge branch 'jokob-sk:main' into FEAT--Make-Errors-More-Helpful commit a6bc96d Author: Adam Outler <adamoutler@gmail.com> Date: Fri Oct 31 22:47:35 2025 +0000 Corrections on testing and behaviors commit 8edef9e Author: Adam Outler <adamoutler@gmail.com> Date: Fri Oct 31 22:24:31 2025 +0000 All errors have documentation links commit 1e63cec Author: Adam Outler <adamoutler@gmail.com> Date: Fri Oct 31 22:24:08 2025 +0000 Revise tests. Use docker-compose.yml where possible commit ff96d38 Author: jokob-sk <jokob.sk@gmail.com> Date: Fri Oct 31 22:09:43 2025 +1100 DOCS:old docker installation guide Signed-off-by: jokob-sk <jokob.sk@gmail.com> commit 537be0f Author: jokob-sk <jokob.sk@gmail.com> Date: Fri Oct 31 22:01:16 2025 +1100 BE: typos Signed-off-by: jokob-sk <jokob.sk@gmail.com> commit b89917c Merge: daea3a2 f42186b Author: Hosted Weblate <hosted@weblate.org> Date: Fri Oct 31 11:55:36 2025 +0100 Merge branch 'origin/main' into Weblate. commit daea3a2 Author: jokob-sk <jokob.sk@gmail.com> Date: Fri Oct 31 21:55:15 2025 +1100 DOCS: WARNING use dockerhub docs Signed-off-by: jokob-sk <jokob.sk@gmail.com> commit b86f636 Author: jokob-sk <jokob.sk@gmail.com> Date: Fri Oct 31 21:46:59 2025 +1100 Revert "DOCS: clearer local_path instructions" This reverts commit dfc64fd. commit 0b08995 Author: jokob-sk <jokob.sk@gmail.com> Date: Fri Oct 31 21:46:25 2025 +1100 Revert "DOCS: install refactor work" This reverts commit fe69972. commit f42186b Merge: 88f889f bc9fb6b Author: Hosted Weblate <hosted@weblate.org> Date: Fri Oct 31 11:10:55 2025 +0100 Merge branch 'origin/main' into Weblate. commit bc9fb6b Author: jeet moh <jeetdevpc@gmail.com> Date: Thu Oct 30 13:07:48 2025 +0100 Translated using Weblate (Persian (fa_FA)) Currently translated at 0.1% (1 of 762 strings) Translation: NetAlertX/core Translate-URL: https://hosted.weblate.org/projects/pialert/core/fa_FA/ commit 88f889f Merge: 533c99e afa257f Author: jokob-sk <jokob.sk@gmail.com> Date: Fri Oct 31 20:56:36 2025 +1100 Merge branch 'next_release' of https://github.com/jokob-sk/NetAlertX into next_release commit 533c99e Merge: 78ab0fb 64e4586 Author: jokob-sk <jokob.sk@gmail.com> Date: Fri Oct 31 20:45:31 2025 +1100 LNG: Swedish (sv_sv) commit afa257f Merge: 78ab0fb 64e4586 Author: jokob-sk <jokob.sk@gmail.com> Date: Fri Oct 31 20:45:31 2025 +1100 Merge branch 'next_release' of https://github.com/jokob-sk/NetAlertX into next_release commit 78ab0fb Author: jokob-sk <jokob.sk@gmail.com> Date: Fri Oct 31 20:24:13 2025 +1100 PLG: SNMPDSC typo commit 64e4586 Author: jokob-sk <jokob.sk@gmail.com> Date: Fri Oct 31 20:24:13 2025 +1100 PLG: Encode SMTP_PASS using base64 netalertx#1253 commit 2f7d9a0 Author: jokob-sk <jokob.sk@gmail.com> Date: Fri Oct 31 15:02:51 2025 +1100 PLG: snmpwalk -OXsq clarification netalertx#1231 commit d29700a Author: Adam Outler <adamoutler@gmail.com> Date: Fri Oct 31 00:07:34 2025 +0000 New mount test structure. commit 75072da Author: jokob-sk <jokob.sk@gmail.com> Date: Fri Oct 31 08:16:54 2025 +1100 GIT: build dev container from next_release branch Signed-off-by: jokob-sk <jokob.sk@gmail.com> commit 19b1fc9 Merge: 63d6410 929eb16 Author: Jokob @netalertx <96159884+jokob-sk@users.noreply.github.com> Date: Fri Oct 31 08:15:12 2025 +1100 Merge pull request netalertx#1260 from jokob-sk/main BE: Devices Tiles SQL syntax error netalertx#1238 commit 63d6410 Author: jokob-sk <jokob.sk@gmail.com> Date: Fri Oct 31 08:12:38 2025 +1100 BE: handle missing buildtimestamp.txt Signed-off-by: jokob-sk <jokob.sk@gmail.com> commit b89a44d Author: Adam Outler <adamoutler@gmail.com> Date: Thu Oct 30 21:05:24 2025 +0000 Improve startup checks commit 929eb16 Author: Jokob @netalertx <96159884+jokob-sk@users.noreply.github.com> Date: Thu Oct 30 20:48:38 2025 +0000 BE: Devices Tiles SQL syntax error netalertx#1238 commit 8cb1836 Author: Adam Outler <adamoutler@gmail.com> Date: Wed Oct 29 23:49:37 2025 +0000 Move all check- scripts to /entrypoint.d/ for better organization commit 512dedf Author: jokob-sk <jokob.sk@gmail.com> Date: Fri Oct 31 06:39:55 2025 +1100 FE: increase filter debounce to 750ms netalertx#1254 Signed-off-by: jokob-sk <jokob.sk@gmail.com> commit 2a2782b Merge: 869f28b b726518 Author: jokob-sk <jokob.sk@gmail.com> Date: Thu Oct 30 14:52:34 2025 +1100 Merge branch 'next_release' of https://github.com/jokob-sk/NetAlertX into next_release commit b726518 Merge: f81a1b9 274beca Author: Jokob @netalertx <96159884+jokob-sk@users.noreply.github.com> Date: Thu Oct 30 14:52:19 2025 +1100 Merge pull request netalertx#1258 from jokob-sk/main BE: fix GRAPHQL_PORT commit 274beca Author: jokob-sk <jokob.sk@gmail.com> Date: Thu Oct 30 14:51:24 2025 +1100 BE: fix GRAPHQL_PORT Signed-off-by: jokob-sk <jokob.sk@gmail.com> commit 869f28b Author: jokob-sk <jokob.sk@gmail.com> Date: Thu Oct 30 14:50:13 2025 +1100 DOCS: typos Signed-off-by: jokob-sk <jokob.sk@gmail.com> commit f81a1b9 Author: jokob-sk <jokob.sk@gmail.com> Date: Thu Oct 30 14:31:22 2025 +1100 DOCS: Docker guides Signed-off-by: jokob-sk <jokob.sk@gmail.com> commit 58fe531 Merge: 50f9277 8da136f Author: Jokob @netalertx <96159884+jokob-sk@users.noreply.github.com> Date: Thu Oct 30 13:56:17 2025 +1100 Merge pull request netalertx#1257 from jokob-sk/main BE: Remove GraphQL check from healthcheck commit 8da136f Author: jokob-sk <jokob.sk@gmail.com> Date: Thu Oct 30 13:55:05 2025 +1100 BE: Remove GraphQL check from healthcheck Signed-off-by: jokob-sk <jokob.sk@gmail.com> commit 50f9277 Author: jokob-sk <jokob.sk@gmail.com> Date: Thu Oct 30 13:30:23 2025 +1100 DOCS: Docker guides (GRAPHQL_PORT fix) Signed-off-by: jokob-sk <jokob.sk@gmail.com> commit 7ca9d2a Merge: b76272b 55171e0 Author: Jokob @netalertx <96159884+jokob-sk@users.noreply.github.com> Date: Thu Oct 30 13:16:05 2025 +1100 Merge pull request netalertx#1256 from adamoutler/next_release update docker compose commit b76272b Merge: fba5359 22aa995 Author: jokob-sk <jokob.sk@gmail.com> Date: Thu Oct 30 13:14:12 2025 +1100 Merge branch 'next_release' of https://github.com/jokob-sk/NetAlertX into next_release commit fba5359 Author: jokob-sk <jokob.sk@gmail.com> Date: Thu Oct 30 13:14:06 2025 +1100 DOCS: Docker guides Signed-off-by: jokob-sk <jokob.sk@gmail.com> commit 55171e0 Author: Adam Outler <adamoutler@gmail.com> Date: Wed Oct 29 23:29:32 2025 +0000 update compose commit 22aa995 Merge: 647defb af80cff Author: Jokob @netalertx <96159884+jokob-sk@users.noreply.github.com> Date: Thu Oct 30 08:33:06 2025 +1100 Merge pull request netalertx#1255 from Tweebloesem/patch-2 Fix typo in PiHole integration guide commit af80cff Author: Tweebloesem <139498987+Tweebloesem@users.noreply.github.com> Date: Wed Oct 29 22:18:42 2025 +0100 Fix typo in PiHole integration guide commit 647defb Merge: 2148a7f ea5e236 Author: jokob-sk <jokob.sk@gmail.com> Date: Wed Oct 29 20:33:42 2025 +1100 Merge branch 'next_release' of https://github.com/jokob-sk/NetAlertX into next_release commit 2148a7f Author: jokob-sk <jokob.sk@gmail.com> Date: Wed Oct 29 20:33:32 2025 +1100 DOCS: Docker guides Signed-off-by: jokob-sk <jokob.sk@gmail.com> commit ea5e236 Merge: 61de637 0079ece Author: Jokob @netalertx <96159884+jokob-sk@users.noreply.github.com> Date: Wed Oct 29 19:26:36 2025 +1100 Merge pull request netalertx#1249 from jokob-sk/main Sync commit 0079ece Merge: 5962312 8d4c7ea Author: Jokob @netalertx <96159884+jokob-sk@users.noreply.github.com> Date: Wed Oct 29 19:25:32 2025 +1100 Merge pull request netalertx#1248 from adamoutler/Easy-Permissions Easy permissions commit 61de637 Author: jokob-sk <jokob.sk@gmail.com> Date: Wed Oct 29 15:51:31 2025 +1100 DOCS: Docker guides Signed-off-by: jokob-sk <jokob.sk@gmail.com> commit 57f3d6f Author: jokob-sk <jokob.sk@gmail.com> Date: Wed Oct 29 13:26:10 2025 +1100 DOCS: Security features - fix hierarchy Signed-off-by: jokob-sk <jokob.sk@gmail.com> commit 2e76ff1 Author: jokob-sk <jokob.sk@gmail.com> Date: Wed Oct 29 13:21:12 2025 +1100 DOCS: Migration and Security features navigation link commit 8d4c7ea Author: Adam Outler <adamoutler@gmail.com> Date: Wed Oct 29 00:32:08 2025 +0000 less invasive permission changes commit b4027b6 Author: Adam Outler <adamoutler@gmail.com> Date: Wed Oct 29 00:08:32 2025 +0000 docker-compose needed for fast container rebuilds commit b36b3be Author: Adam Outler <adamoutler@gmail.com> Date: Wed Oct 29 00:08:09 2025 +0000 Fix permissions messages and test parms commit 7ddb7d2 Author: Adam Outler <adamoutler@gmail.com> Date: Tue Oct 28 23:58:02 2025 +0000 new method of fixing permissions commit 40341a8 Merge: 304d4d0 6afa52e Author: Jokob @netalertx <96159884+jokob-sk@users.noreply.github.com> Date: Wed Oct 29 07:37:55 2025 +1100 Merge pull request netalertx#1247 from adamoutler/next_release Security features overview commit 304d4d0 Merge: a353acf 4d148f3 Author: jokob-sk <jokob.sk@gmail.com> Date: Wed Oct 29 07:33:59 2025 +1100 Merge branch 'next_release' of https://github.com/jokob-sk/NetAlertX into next_release commit a353acf Author: jokob-sk <jokob.sk@gmail.com> Date: Wed Oct 29 07:32:56 2025 +1100 DOCS: builds Signed-off-by: jokob-sk <jokob.sk@gmail.com> commit 6afa52e Author: Adam Outler <adamoutler@gmail.com> Date: Tue Oct 28 00:15:12 2025 +0000 Security features overview commit 5962312 Merge: 84183f0 3ba4100 Author: Jokob @netalertx <96159884+jokob-sk@users.noreply.github.com> Date: Tue Oct 28 08:31:30 2025 +1100 Merge pull request netalertx#1235 from adamoutler/hardening-fixes Hardening fixes commit 3ba4100 Author: Adam Outler <adamoutler@gmail.com> Date: Mon Oct 27 16:51:17 2025 -0400 Update install/production-filesystem/entrypoint.sh Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> commit a6ac492 Author: Adam Outler <adamoutler@gmail.com> Date: Mon Oct 27 20:19:17 2025 +0000 Add APP_CONF_OVERRIDE support commit 4d148f3 Author: Jokob @netalertx <96159884+jokob-sk@users.noreply.github.com> Date: Mon Oct 27 03:33:50 2025 +0000 DOCS: wording commit 9b0f45b Author: jokob-sk <jokob.sk@gmail.com> Date: Mon Oct 27 14:21:17 2025 +1100 DOCS: migration prep Signed-off-by: jokob-sk <jokob.sk@gmail.com> commit 84183f0 Author: jokob-sk <jokob.sk@gmail.com> Date: Mon Oct 27 12:58:48 2025 +1100 LANG: ru_ru updates Signed-off-by: jokob-sk <jokob.sk@gmail.com> commit 5dba0f1 Merge: 76419db 816b907 Author: Jokob @netalertx <96159884+jokob-sk@users.noreply.github.com> Date: Mon Oct 27 08:14:16 2025 +1100 Merge pull request netalertx#1244 from jokob-sk/main sync commit 095372a Author: Adam Outler <adamoutler@gmail.com> Date: Sun Oct 26 16:49:28 2025 -0400 Rename GRAPHQL_PORT to APP_CONF_OVERRIDE commit d8c2dc0 Author: Adam Outler <adamoutler@gmail.com> Date: Sun Oct 26 19:58:57 2025 +0000 Apply coderabit's latest hare-brained idea commit cfffaf4 Author: Adam Outler <adamoutler@gmail.com> Date: Sun Oct 26 19:40:17 2025 +0000 Strengthen tests commit 01b64cc Author: Adam Outler <adamoutler@gmail.com> Date: Sun Oct 26 19:34:28 2025 +0000 Changes requested by coderabbit. commit 63c4b0d Author: Adam Outler <adamoutler@gmail.com> Date: Sun Oct 26 14:15:12 2025 -0400 Update .devcontainer/devcontainer.json Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> commit 5ec35aa Author: Adam Outler <adamoutler@gmail.com> Date: Sun Oct 26 18:12:02 2025 +0000 Build the netalertx-test image on start so tests don't fail commit ededd39 Author: Adam Outler <adamoutler@gmail.com> Date: Sun Oct 26 17:53:46 2025 +0000 Coderabbit fixes commit 15bc163 Author: Adam Outler <adamoutler@gmail.com> Date: Sun Oct 26 12:45:42 2025 -0400 Update install/production-filesystem/services/scripts/check-root.sh Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> commit 74a67e3 Author: Adam Outler <adamoutler@gmail.com> Date: Sun Oct 26 16:10:17 2025 +0000 Added clarifying examples to dockerfile commit 52b747b Author: Adam Outler <adamoutler@gmail.com> Date: Sun Oct 26 15:54:01 2025 +0000 Remove warnings in devcontainer commit d2c28f6 Author: Adam Outler <adamoutler@gmail.com> Date: Sun Oct 26 15:30:03 2025 +0000 Changes for tests identified by CodeRabbit commit 816b907 Author: Almaz <almazgamer228@gmail.com> Date: Sat Oct 25 09:56:34 2025 +0200 Translated using Weblate (Russian) Currently translated at 100.0% (762 of 762 strings) Translation: NetAlertX/core Translate-URL: https://hosted.weblate.org/projects/pialert/core/ru/ commit fb02774 Author: Adam Outler <adamoutler@gmail.com> Date: Sun Oct 26 00:14:03 2025 +0000 Fix errors for tests commit 2663227 Author: jokob-sk <jokob.sk@gmail.com> Date: Sun Oct 26 11:07:34 2025 +1100 PLUG: SNMPDSC timeout multiplier netalertx#1231 Signed-off-by: jokob-sk <jokob.sk@gmail.com> commit dfc64fd Author: jokob-sk <jokob.sk@gmail.com> Date: Sun Oct 26 10:59:42 2025 +1100 DOCS: clearer local_path instructions Signed-off-by: jokob-sk <jokob.sk@gmail.com> commit b44369a Author: jokob-sk <jokob.sk@gmail.com> Date: Sun Oct 26 10:59:05 2025 +1100 PLUG: 0 in device tiles netalertx#1238 Signed-off-by: jokob-sk <jokob.sk@gmail.com> commit 8ada2c3 Author: jokob-sk <jokob.sk@gmail.com> Date: Sun Oct 26 10:58:34 2025 +1100 BE: 0 in device tiles netalertx#1238 Signed-off-by: jokob-sk <jokob.sk@gmail.com> commit c4a041e Author: Adam Outler <adamoutler@gmail.com> Date: Sat Oct 25 17:58:21 2025 +0000 Coderabit changes commit 170aeb0 Author: jokob-sk <jokob.sk@gmail.com> Date: Sat Oct 25 13:48:56 2025 +1100 PLUG: SNMPDSC timeout not respected netalertx#1231 Signed-off-by: jokob-sk <jokob.sk@gmail.com> commit fe69972 Author: jokob-sk <jokob.sk@gmail.com> Date: Sat Oct 25 09:28:03 2025 +1100 DOCS: install refactor work Signed-off-by: jokob-sk <jokob.sk@gmail.com> commit 32f9111 Author: Adam Outler <adamoutler@gmail.com> Date: Fri Oct 24 20:32:50 2025 +0000 Restore test_safe_builder_unit.py to upstream version (remove local changes) commit bb35417 Merge: fe69bc4 05890b3 Author: Jokob @netalertx <96159884+jokob-sk@users.noreply.github.com> Date: Sat Oct 25 07:07:12 2025 +1100 Merge pull request netalertx#1237 from JVKeller/patch-3 Change branch back to main. commit fe69bc4 Merge: 6a20128 c278865 Author: Jokob @netalertx <96159884+jokob-sk@users.noreply.github.com> Date: Sat Oct 25 07:06:41 2025 +1100 Merge pull request netalertx#1236 from AlmazzikDev/patch-1 Rename CONTRIBUTING to CONTRIBUTING.md commit 05890b3 Author: rell3k <keller.jeff@gmail.com> Date: Fri Oct 24 09:24:01 2025 -0400 Change branch back to main. Forgot to change git clone branch back to main. commit c278865 Author: Almaz <almaz@weissx.net> Date: Fri Oct 24 15:35:18 2025 +0300 Rename CONTRIBUTING to CONTRIBUTING.md commit 7f74c2d Author: Adam Outler <adamoutler@gmail.com> Date: Thu Oct 23 21:37:11 2025 -0400 docker compose changes commit 5a63b72 Merge: 0897c05 6a20128 Author: Adam Outler <adamoutler@gmail.com> Date: Thu Oct 23 21:19:30 2025 -0400 Merge main into hardening-fixes commit 0897c05 Author: Adam Outler <adamoutler@gmail.com> Date: Thu Oct 23 21:16:15 2025 -0400 Tidy up output commit 7a3bf67 Author: Adam Outler <adamoutler@gmail.com> Date: Thu Oct 23 20:46:39 2025 -0400 Remove code coverage from repository commit edd5bd2 Author: Adam Outler <adamoutler@gmail.com> Date: Thu Oct 23 23:33:04 2025 +0000 Devcontainer setup commit 3b7830b Author: Adam Outler <adamoutler@gmail.com> Date: Thu Oct 23 21:15:15 2025 +0000 Add unit tests and updated messages commit 356caca Author: Adam Outler <adamoutler@gmail.com> Date: Thu Oct 23 21:15:02 2025 +0000 Don't increment sqlite sequence commit d12ffb3 Author: Adam Outler <adamoutler@gmail.com> Date: Thu Oct 23 21:04:15 2025 +0000 Update readme with simple build instructions commit f70d3f3 Author: Adam Outler <adamoutler@gmail.com> Date: Thu Oct 23 20:36:04 2025 +0000 Limiter fix for older kernels commit 2789946 Author: Adam Outler <adamoutler@gmail.com> Date: Thu Oct 23 08:36:42 2025 +0000 use system speedtest, not un-updated & removed script commit 59c7d7b Author: Adam Outler <adamoutler@gmail.com> Date: Thu Oct 23 00:27:16 2025 +0000 Add test dependencies commit 0851680 Author: Adam Outler <adamoutler@gmail.com> Date: Wed Oct 22 23:51:36 2025 +0000 Add additional startup checks commit 1af19fe Author: Adam Outler <adamoutler@gmail.com> Date: Wed Oct 22 23:51:15 2025 +0000 Only nginx/python errors in docker logs. no stdout from backend. commit ce8bb53 Author: Adam Outler <adamoutler@gmail.com> Date: Wed Oct 22 19:48:58 2025 -0400 Refine devcontainer setup and docker tests commit 5636a15 Author: Adam Outler <adamoutler@gmail.com> Date: Wed Oct 22 00:02:03 2025 +0000 Add check permissions script commit 6a20128 Author: jokob-sk <jokob.sk@gmail.com> Date: Wed Oct 22 07:48:50 2025 +1100 BE: install refactor work Signed-off-by: jokob-sk <jokob.sk@gmail.com> commit 05f0837 Author: Adam Outler <adamoutler@gmail.com> Date: Tue Oct 21 19:18:59 2025 +0000 Fix missing storage check commit 3441f77 Author: Adam Outler <adamoutler@gmail.com> Date: Tue Oct 21 19:10:48 2025 +0000 Fix always fresh install env commit d6bcb27 Author: Adam Outler <adamoutler@gmail.com> Date: Tue Oct 21 19:05:47 2025 +0000 Missing devcontainer build timestamp commit 5d7af88 Merge: b916542 6f2e556 Author: Jokob @netalertx <96159884+jokob-sk@users.noreply.github.com> Date: Tue Oct 21 12:35:08 2025 +1100 Merge pull request netalertx#1230 from adamoutler/hardening Feat: Enterprise-Grade Security Hardening and Build Overhaul commit 6f2e556 Author: Adam Outler <adamoutler@gmail.com> Date: Sun Oct 19 12:18:16 2025 -0400 Remove duplicate file replacement logic in update_vendors.sh Dang it coderabbit. We expect more of your diffs. commit ea4c70e Author: Adam Outler <adamoutler@gmail.com> Date: Sun Oct 19 12:15:55 2025 -0400 Update install/production-filesystem/services/scripts/check-first-run-config.sh Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> commit 5ed46da Author: Adam Outler <adamoutler@gmail.com> Date: Sun Oct 19 15:55:28 2025 +0000 Set caps on actual python3.12 commit 628f35c Author: Adam Outler <adamoutler@gmail.com> Date: Sun Oct 19 15:41:57 2025 +0000 Remove unused pythonpathpath variable commit 066fecf Author: Adam Outler <adamoutler@gmail.com> Date: Sun Oct 19 15:39:54 2025 +0000 add caps to python instead of scapy. commit 660f0c2 Author: Adam Outler <adamoutler@gmail.com> Date: Sun Oct 19 11:37:04 2025 -0400 Update install/production-filesystem/services/scripts/update_vendors.sh Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> commit 999feb2 Author: Adam Outler <adamoutler@gmail.com> Date: Sun Oct 19 11:36:09 2025 -0400 Update install/production-filesystem/services/scripts/update_vendors.sh Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> commit 86bf0a3 Author: Adam Outler <adamoutler@gmail.com> Date: Sun Oct 19 11:35:27 2025 -0400 Update install/production-filesystem/services/scripts/check-first-run-config.sh Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> commit 8eab7ee Author: Adam Outler <adamoutler@gmail.com> Date: Sun Oct 19 11:33:07 2025 -0400 Update .devcontainer/scripts/setup.sh Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> commit 84f1283 Author: Adam Outler <adamoutler@gmail.com> Date: Sun Oct 19 15:27:55 2025 +0000 Add novel coderabit no-write database creation commit dcf250d Author: Adam Outler <adamoutler@gmail.com> Date: Sun Oct 19 15:12:27 2025 +0000 Coderabbit nitpicks. commit 131c0c0 Author: Adam Outler <adamoutler@gmail.com> Date: Sun Oct 19 14:28:09 2025 +0000 Fix fish terminal. Smarter code completion and other nicities. commit a58b3e3 Author: Adam Outler <adamoutler@gmail.com> Date: Sun Oct 19 14:18:07 2025 +0000 Coderabbit suggestions commit 14be7a2 Author: Adam Outler <adamoutler@gmail.com> Date: Sun Oct 19 02:45:19 2025 +0000 Missing Slash commit 9b3ddda Author: Adam Outler <adamoutler@gmail.com> Date: Sun Oct 19 02:35:57 2025 +0000 Fix persistent environment issues commit 1f46f20 Author: Adam Outler <adamoutler@gmail.com> Date: Sun Oct 19 01:06:42 2025 +0000 Generate devcontainer configs commit 80c1459 Author: Adam Outler <adamoutler@gmail.com> Date: Sun Oct 19 00:39:26 2025 +0000 Final touches on devcontainer commit 62536e4 Author: Adam Outler <adamoutler@gmail.com> Date: Sat Oct 18 14:07:27 2025 -0400 Coderabit suggestions commit 028335c Author: Adam Outler <adamoutler@gmail.com> Date: Sat Oct 18 13:45:48 2025 -0400 Coderabit suggestions commit 7483e46 Merge: c1b573f b916542 Author: Adam Outler <adamoutler@gmail.com> Date: Sat Oct 18 13:23:57 2025 -0400 Merge remote-tracking branch 'origin/main' into hardening commit c1b573f Author: Adam Outler <adamoutler@gmail.com> Date: Sat Oct 18 13:16:35 2025 -0400 Add some todos commit d11c9d7 Author: Adam Outler <adamoutler@gmail.com> Date: Fri Oct 17 16:36:48 2025 -0400 Improve warnings. commit b916542 Author: jokob-sk <jokob.sk@gmail.com> Date: Fri Oct 17 21:33:43 2025 +1100 BE: DB generate=ing script Signed-off-by: jokob-sk <jokob.sk@gmail.com> commit 6da3cfd Author: jokob-sk <jokob.sk@gmail.com> Date: Fri Oct 17 21:33:22 2025 +1100 FE: docs mikrotik Signed-off-by: jokob-sk <jokob.sk@gmail.com> commit d38e77f Author: jokob-sk <jokob.sk@gmail.com> Date: Fri Oct 17 21:32:53 2025 +1100 docs Signed-off-by: jokob-sk <jokob.sk@gmail.com> commit 18eaee4 Author: jokob-sk <jokob.sk@gmail.com> Date: Fri Oct 17 21:32:22 2025 +1100 FE: lang Signed-off-by: jokob-sk <jokob.sk@gmail.com> commit 59e7463 Author: Safeguard <yo-safeguard@yandex.ru> Date: Thu Oct 16 10:55:31 2025 +0200 Translated using Weblate (Russian) Currently translated at 100.0% (762 of 762 strings) Translation: NetAlertX/core Translate-URL: https://hosted.weblate.org/projects/pialert/core/ru/ commit dc44411 Author: Adam Outler <adamoutler@gmail.com> Date: Thu Oct 16 21:49:54 2025 -0400 Improve mount permissions commit a3dae08 Author: Adam Outler <adamoutler@gmail.com> Date: Thu Oct 16 19:51:57 2025 -0400 Fix debian docker start commit e733f8a Author: Adam Outler <adamoutler@gmail.com> Date: Thu Oct 16 16:17:37 2025 -0400 Relay failed status to docker. commit ad0ddda Merge: 3686a4a 28e0e4a Author: Jokob @netalertx <96159884+jokob-sk@users.noreply.github.com> Date: Thu Oct 16 12:50:08 2025 +1100 Merge pull request netalertx#1229 from adamoutler/patch-5 Add script to regenerate the database from schema commit 28e0e4a Author: Adam Outler <adamoutler@gmail.com> Date: Wed Oct 15 20:53:03 2025 -0400 Fix database regeneration script to use correct file commit 324cde9 Author: Adam Outler <adamoutler@gmail.com> Date: Wed Oct 15 20:50:42 2025 -0400 Add script to regenerate the database from schema This script recreates the database from schema code and imports the schema into the new database file. commit f57ec74 Author: Adam Outler <adamoutler@gmail.com> Date: Thu Oct 16 00:09:07 2025 +0000 Minor alterations to ddevcontainer. commit de92c95 Author: Adam Outler <adamoutler@gmail.com> Date: Wed Oct 15 18:18:30 2025 -0400 break apart services, fix startup commit 3686a4a Author: anton garcias <isaga.percompartir@gmail.com> Date: Mon Oct 13 22:37:42 2025 +0200 Translated using Weblate (Catalan) Currently translated at 100.0% (762 of 762 strings) Translation: NetAlertX/core Translate-URL: https://hosted.weblate.org/projects/pialert/core/ca/ commit 44ba945 Author: Ettore Atalan <atalanttore@googlemail.com> Date: Sun Oct 12 22:12:37 2025 +0200 Translated using Weblate (German) Currently translated at 81.3% (620 of 762 strings) Translation: NetAlertX/core Translate-URL: https://hosted.weblate.org/projects/pialert/core/de/ commit 5109a08 Author: Adam Outler <adamoutler@gmail.com> Date: Sun Oct 12 21:00:27 2025 -0400 Additional hardening commit 1be9155 Author: Adam Outler <adamoutler@gmail.com> Date: Sun Oct 12 15:05:20 2025 -0400 Set container parameters commit 3bf6ce6 Author: R <15691591183@163.com> Date: Sun Oct 12 15:49:48 2025 +0200 Translated using Weblate (Chinese (Simplified Han script)) Currently translated at 100.0% (762 of 762 strings) Translation: NetAlertX/core Translate-URL: https://hosted.weblate.org/projects/pialert/core/zh_Hans/ commit 1532256 Author: Massimo Pissarello <mapi68@gmail.com> Date: Sat Oct 11 01:39:43 2025 +0200 Translated using Weblate (Italian) Currently translated at 100.0% (762 of 762 strings) Translation: NetAlertX/core Translate-URL: https://hosted.weblate.org/projects/pialert/core/it/ commit be73e3a Author: Adam Outler <adamoutler@gmail.com> Date: Thu Oct 9 20:30:25 2025 -0400 debian dockerfile completed properly. commit 016a6ad Author: Adam Outler <adamoutler@gmail.com> Date: Wed Oct 8 19:55:16 2025 -0400 Dockerfile.debian building and running commit 558ab44 Author: Adam Outler <adamoutler@gmail.com> Date: Mon Oct 6 23:31:20 2025 +0000 Minimize differences between devcontainer and production commit 290b6c6 Author: Adam Outler <adamoutler@gmail.com> Date: Sat Oct 4 18:51:10 2025 +0000 Remove nohup.out commit ada9271 Author: Adam Outler <adamoutler@gmail.com> Date: Fri Oct 3 22:12:42 2025 +0000 all debugging online. commit 1e04e9f Author: Adam Outler <adamoutler@gmail.com> Date: Fri Oct 3 00:33:20 2025 +0000 Remove .git-placeholder, add dockerignore commit c81a054 Author: Adam Outler <adamoutler@gmail.com> Date: Fri Oct 3 00:08:26 2025 +0000 Coderabit commit 33aa849 Author: Adam Outler <adamoutler@gmail.com> Date: Thu Oct 2 21:19:29 2025 +0000 Debugging operational in vscode commit 0cd1dc8 Author: Adam Outler <adamoutler@gmail.com> Date: Tue Sep 30 22:01:03 2025 -0400 Scanning Operational with monitoring commit 044035e Author: Adam Outler <adamoutler@gmail.com> Date: Tue Sep 30 01:55:26 2025 +0000 Devcontainer overlay commit dc4848a Author: Adam Outler <adamoutler@gmail.com> Date: Sun Sep 28 21:59:06 2025 -0400 Information on default config and entrypoints for debug commit c6efe5a Author: Adam Outler <adamoutler@gmail.com> Date: Sun Sep 28 17:10:15 2025 -0400 All services moved to deployed filesystem commit d182a55 Author: Adam Outler <adamoutler@gmail.com> Date: Sat Sep 27 21:58:00 2025 -0400 Move filesystem to more generic name & add perms commit b47df7b Author: Adam Outler <adamoutler@gmail.com> Date: Sat Sep 27 19:48:36 2025 -0400 capcheck commit 46097bb Author: Adam Outler <adamoutler@gmail.com> Date: Sat Sep 27 19:15:07 2025 -0400 solid hardened config commit c5d7480 Merge: 2def3f1 d9feddd Author: Adam Outler <adamoutler@gmail.com> Date: Sat Sep 27 09:00:46 2025 -0400 Merge branch 'jokob-sk:main' into hardening commit 2def3f1 Author: Adam Outler <adamoutler@gmail.com> Date: Fri Sep 26 21:01:58 2025 -0400 Validated launch on runner & hardend commit 2419a26 Author: Adam Outler <adamoutler@gmail.com> Date: Fri Sep 26 17:52:17 2025 +0000 updated devcontainer dockerfile commit bad67b2 Author: Adam Outler <adamoutler@gmail.com> Date: Fri Sep 26 17:52:11 2025 +0000 fix dockerfile error commit 178fb54 Author: Adam Outler <adamoutler@gmail.com> Date: Fri Sep 26 17:32:58 2025 +0000 Python up and debuggable commit b0a6f88 Author: Adam Outler <adamoutler@gmail.com> Date: Fri Sep 26 17:14:20 2025 +0000 Update gitignore commit 798d246 Author: Adam Outler <adamoutler@gmail.com> Date: Fri Sep 26 11:56:27 2025 +0000 expand initial filesystem commit c228d45 Author: Adam Outler <adamoutler@gmail.com> Date: Thu Sep 25 23:03:55 2025 +0000 Devcontainer operational, services all down commit dfcc375 Author: Adam Outler <adamoutler@gmail.com> Date: Thu Sep 25 14:10:06 2025 -0400 Non-root launch commit 8ed21a8 Author: Adam Outler <adamoutler@gmail.com> Date: Thu Sep 25 07:43:42 2025 -0400 monolithic alpine container commit 2e694a7 Author: Adam Outler <adamoutler@gmail.com> Date: Wed Sep 24 19:46:11 2025 -0400 using 4 startup scripts instead of RC6 commit 29aa884 Author: Adam Outler <adamoutler@gmail.com> Date: Wed Sep 24 16:29:15 2025 -0400 architectural change 1
2 participants
Pull Request: Security Hardening and Proactive Issue Resolution
Summary
This PR implements critical security hardening measures and proactive user experience enhancements for NetAlertX, focusing on container security, SQL injection prevention, robust testing frameworks, and automated responses to common deployment and configuration issues. As a network monitoring tool handling sensitive device data, these changes significantly reduce the attack surface, improve resilience against exploitation vectors, and provide users with fail-safe mechanisms that prevent broken experiences through early detection and corrective guidance.
Security Impact Assessment
🔒 Container Security Hardening
Risk Addressed: Docker deployments with insecure configurations can expose host networks, lose persistent data, or allow privilege escalation. Common user issues include data loss from improper volume mounting, network isolation failures, and silent configuration errors leading to broken functionality.
Changes:
test/test_docker_integration.pyto validate secure configurations by systematically testing container configurations against identified and potential failure modes--network=hostrequirement for ARP scanning (prevents network isolation bypasses that cause scanning failures)/app/configand/app/db(prevents data loss attacks and common user data loss issues)Impact: Prevents unauthorized network access, ensures data integrity in containerized deployments, and eliminates common Docker-related support tickets by failing fast with clear guidance.
🛡️ SQL Injection Prevention
Risk Addressed: The reporting system was vulnerable to SQL injection attacks through user-controlled filter conditions, potentially allowing unauthorized data access or manipulation.
Changes:
Impact: Eliminates SQL injection vectors in device filtering and reporting features, protecting against data breaches.
🧪 Testing Framework Modernization
Risk Addressed: Legacy testing frameworks may miss security edge cases and provide inadequate coverage, leading to undetected vulnerabilities in production.
Changes:
Impact: Ensures ongoing security validation and reduces likelihood of regression vulnerabilities that could affect users.
Files Changed
test/test_docker_integration.py- New Docker security validation tests addressing common deployment issuestest/test_compound_conditions.py- Converted to pytest, enhanced SQL safety teststest/test_safe_builder_unit.py- Converted to pytest, expanded injection prevention testsValidation Steps
Risk Assessment
Proactive Issue Resolution
These changes implement a "fail-fast with guidance" approach to common user issues:
Compliance Considerations
These changes align with security best practices for:
Breaking Changes
None. All changes are backward-compatible and enhance existing functionality.
Future Enhancements
/install/production-filesystem/service/scripts/check-*.shscripts to halt users on misconfigurations and provide corrective guidance, reducing support overhead and improving user success ratesSummary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests