feat(devops): add observability compose profile - #594
AbdulmalikAlayande merged 1 commit into
Conversation
|
@Ekpemark Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds a Docker Compose observability overlay with Prometheus scraping Sorokeep metrics and Grafana displaying a provisioned endpoint-status dashboard. Documentation explains setup, and Docker tests validate services, scrape targets, provisioning files, and dashboard content. ChangesObservability stack
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related issues
Possibly related PRs
Sequence Diagram(s)sequenceDiagram
participant Sorokeep
participant Prometheus
participant Grafana
Sorokeep->>Prometheus: Expose metrics on sorokeep:9464
Prometheus->>Prometheus: Scrape up{job="sorokeep"}
Grafana->>Prometheus: Query Prometheus datasource
Prometheus-->>Grafana: Return endpoint status
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docker-compose.observability.yml`:
- Around line 17-19: Harden the observability overlay by binding the default
Prometheus and Grafana port mappings to 127.0.0.1 instead of all host
interfaces. Remove --web.enable-lifecycle from the Prometheus command unless
authenticated proxy protection is present, and require non-default Grafana admin
credentials before enabling the overlay.
In `@tests/docker/docker-compose.test.ts`:
- Around line 158-180: Extend the observability compose tests around the
existing OBSERVABILITY_COMPOSE_FILE and observabilityComposeConfig checks with a
smoke test that runs the actual Docker Compose command using the base and
observability files, including the observability profile, and executes config.
Assert the command succeeds so merged networks, overrides, mounts, and profiles
are validated rather than only parsing individual files.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: ac8db3a4-b618-47cf-90ed-9df45d11aca1
📒 Files selected for processing (7)
devops/grafana/provisioning/dashboards/dashboard.ymldevops/grafana/provisioning/dashboards/sorokeep-overview.jsondevops/grafana/provisioning/datasources/prometheus.ymldevops/prometheus/prometheus.ymldocker-compose.observability.ymldocs/observability.mdtests/docker/docker-compose.test.ts
📜 Review details
🧰 Additional context used
🪛 ast-grep (0.45.0)
tests/docker/docker-compose.test.ts
[warning] 20-20: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.readFileSync(COMPOSE_FILE, "utf8")
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename-typescript)
[warning] 24-24: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.readFileSync(OBSERVABILITY_COMPOSE_FILE, "utf8")
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename-typescript)
[warning] 166-166: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.readFileSync(PROMETHEUS_CONFIG_FILE, "utf8")
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename-typescript)
[warning] 176-176: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.readFileSync(GRAFANA_DASHBOARD_FILE, "utf8")
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename-typescript)
🔇 Additional comments (7)
docker-compose.observability.yml (1)
1-13: LGTM!Also applies to: 20-31, 38-47
devops/prometheus/prometheus.yml (1)
1-9: LGTM!devops/grafana/provisioning/datasources/prometheus.yml (1)
1-10: LGTM!devops/grafana/provisioning/dashboards/dashboard.yml (1)
1-11: LGTM!devops/grafana/provisioning/dashboards/sorokeep-overview.json (1)
1-38: LGTM!docs/observability.md (1)
1-9: LGTM!tests/docker/docker-compose.test.ts (1)
8-26: LGTM!
| - "--web.enable-lifecycle" | ||
| ports: | ||
| - "${PROMETHEUS_PORT:-9090}:9090" |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Files:\n'
git ls-files | rg '(^|/)docker-compose\.observability\.yml$|docker-compose.*observability|prometheus|grafana'
printf '\nObservability compose snippet:\n'
if [ -f docker-compose.observability.yml ]; then
nl -ba docker-compose.observability.yml | sed -n '1,120p'
fi
printf '\nGrep lifecycle/port patterns in repo:\n'
rg -n --hidden --glob '!node_modules/**' --glob '!dist/**' --glob '!build/**' 'web\.enable-lifecycle|PROMETHEUS_PORT|GRAFANA_PORT|prometheus.*restart|--web\.enable-lifecycle|admin/admin|GF_SECURITY_ADMIN_PASSWORD|GRAFANA_ADMIN|PROMETHEUS_ADMIN' . || true
printf '\nDocker publish port semantics reference from local install docs (if available):\n'
sed -n '17,22p' docker-compose.observability.yml 2>/dev/null || trueRepository: AbdulmalikAlayande/sorokeep
Length of output: 480
🌐 Web query:
Docker Compose ports short syntax published on all interfaces host IP 127.0.0.1 documentation
💡 Result:
In Docker Compose, the short syntax for ports follows the format [HOST:]CONTAINER[/PROTOCOL] [1][2][3]. If you do not specify a host IP in the short syntax, Docker Compose defaults to binding the port to all available network interfaces (0.0.0.0), which may expose the container to the public internet [1][4][3]. To restrict access so that the container is only reachable from the host machine, you must explicitly include 127.0.0.1 in the port mapping [1][4][3]. For example: ports: - "127.0.0.1:8080:80" This configuration maps the container's port 80 to the host's port 8080, but only on the 127.0.0.1 loopback address [1][2][3]. Key considerations include: - Security: Publishing to 0.0.0.0 by default can bypass host firewall rules if not properly managed [1][3]. - YAML Syntax: When using the short syntax with an IP address, it is recommended to wrap the entire string in quotes to prevent the YAML parser from misinterpreting the values (e.g., as a base-60 float) [2][5][6]. - Alternatives: If you need to avoid specifying the IP for every service, you can configure the Docker daemon to use 127.0.0.1 as the default binding address for published ports, though this is a global setting that affects all containers [4].
Citations:
- 1: https://docs.docker.com/reference/compose-file/services
- 2: https://github.com/compose-spec/compose-spec/blob/a4a7e7c/spec.md
- 3: https://github.com/docker/docs/blob/main/content/reference/compose-file/services.md
- 4: https://docs.docker.com/engine/network/port-publishing/
- 5: https://github.com/compose-spec/compose-spec/blob/a5cd401cbdca20f044d11a05707d206dbbacf22c/spec.md
- 6: https://github.com/compose-spec/compose-spec/blob/e8db8022c0b2e3d5eb007d629ff684cbe49a17a4/spec.md
🌐 Web query:
Prometheus --web.enable-lifecycle endpoint security unauthenticated POST /-/reload /-/quit documentation
💡 Result:
When the --web.enable-lifecycle flag is enabled in Prometheus, the /-/reload and /-/quit endpoints become accessible via HTTP requests [1][2]. These endpoints are disabled by default [2][3]. Crucially, Prometheus does not natively enforce authentication on these lifecycle endpoints [1]. If this flag is enabled, any party with network access to the Prometheus HTTP server can trigger a configuration reload or a graceful shutdown of the process [1][4]. To mitigate the security risks associated with enabling these endpoints, the Prometheus documentation recommends the following: 1. Network Isolation: Ensure that the Prometheus server is not exposed to untrusted networks [1]. 2. Reverse Proxy Authentication: If you need to enable these endpoints but require authentication, use an external reverse proxy (e.g., Nginx, Apache, or a dedicated sidecar proxy) to intercept requests and enforce authentication before forwarding them to the Prometheus instance [1]. Alternatively, you can perform these actions without exposing the HTTP endpoints by using system signals: - Reload configuration: Send a SIGHUP signal to the Prometheus process [2][5]. - Shutdown: Send a SIGTERM signal to the Prometheus process [2][5]. Using these signals is the recommended, more secure alternative to using the HTTP lifecycle endpoints when local access to the server is available [2][5].
Citations:
- 1: https://prometheus.io/docs/operating/security/
- 2: https://prometheus.io/docs/prometheus/latest/management_api/
- 3: https://prometheus.io/docs/prometheus/latest/command-line/prometheus/
- 4: Security: Prometheus --web.enable-lifecycle allows unauthenticated reload/shutdown HomericIntelligence/Argus#19
- 5: https://prometheus.io/docs/prometheus/3.13/management_api/
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
p = Path("docker-compose.observability.yml")
text = p.read_text()
print("docker-compose.observability.yml lines:")
for i, line in enumerate(text.splitlines(), 1):
print(f"{i:3}: {line}")
print("\nRelevant patterns:")
for word in ["--web.enable-lifecycle", "PROMETHEUS_PORT", "GRAFANA_PORT", "admin/admin"]:
print(f"{word}: {word in text}")
PY
printf '\nGrafana-related security/env defaults:\n'
rg -n --hidden --glob '!node_modules/**' --glob '!dist/**' --glob '!build/**' 'admin/admin|GF_SECURITY_ADMIN|ADMIN_PASSWORD|PROMETHEUS_PORT|GRAFANA_PORT|--web\.enable-lifecycle|web\.enable-lifecycle' . || trueRepository: AbdulmalikAlayande/sorokeep
Length of output: 2680
Keep the observability stack local by default.
Docker Compose short port mappings publish on all host interfaces, so the Prometheus lifecycle endpoint and Grafana login are reachable externally unless protected. Bind both defaults to 127.0.0.1 and remove --web.enable-lifecycle unless lifecycle controls are required and wrapped in authentication/proxy controls; also require non-default Grafana admin credentials before enabling this overlay.
Proposed hardening
command:
- "--config.file=/etc/prometheus/prometheus.yml"
- "--storage.tsdb.path=/prometheus"
- - "--web.enable-lifecycle"
...
- - "${PROMETHEUS_PORT:-9090}:9090"
+ - "127.0.0.1:${PROMETHEUS_PORT:-9090}:9090"
...
- - "${GRAFANA_PORT:-3000}:3000"
+ - "127.0.0.1:${GRAFANA_PORT:-3000}:3000"Also applies to lines 33-37.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - "--web.enable-lifecycle" | |
| ports: | |
| - "${PROMETHEUS_PORT:-9090}:9090" | |
| ports: | |
| - "127.0.0.1:${PROMETHEUS_PORT:-9090}:9090" |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docker-compose.observability.yml` around lines 17 - 19, Harden the
observability overlay by binding the default Prometheus and Grafana port
mappings to 127.0.0.1 instead of all host interfaces. Remove
--web.enable-lifecycle from the Prometheus command unless authenticated proxy
protection is present, and require non-default Grafana admin credentials before
enabling the overlay.
| describe("observability compose overlay", () => { | ||
| it("defines profiled Prometheus and Grafana services", () => { | ||
| expect(fs.existsSync(OBSERVABILITY_COMPOSE_FILE)).toBe(true); | ||
| expect(observabilityComposeConfig.services.prometheus.profiles).toContain("observability"); | ||
| expect(observabilityComposeConfig.services.grafana.profiles).toContain("observability"); | ||
| }); | ||
|
|
||
| it("wires Prometheus to Sorokeep's metrics endpoint", () => { | ||
| expect(fs.existsSync(PROMETHEUS_CONFIG_FILE)).toBe(true); | ||
| const prometheusConfig = YAML.parse(fs.readFileSync(PROMETHEUS_CONFIG_FILE, "utf8")); | ||
| const targets = prometheusConfig.scrape_configs.flatMap((job: any) => | ||
| job.static_configs.flatMap((config: any) => config.targets), | ||
| ); | ||
| expect(targets).toContain("sorokeep:9464"); | ||
| }); | ||
|
|
||
| it("provisions the Sorokeep Grafana dashboard", () => { | ||
| expect(fs.existsSync(GRAFANA_DASHBOARD_PROVIDER_FILE)).toBe(true); | ||
| expect(fs.existsSync(GRAFANA_DASHBOARD_FILE)).toBe(true); | ||
| const dashboard = JSON.parse(fs.readFileSync(GRAFANA_DASHBOARD_FILE, "utf8")); | ||
| expect(dashboard.title).toBe("Sorokeep Overview"); | ||
| expect(dashboard.panels.length).toBeGreaterThan(0); | ||
| }); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick win
Validate the merged Compose configuration, not only individual files.
These tests never run docker compose with both Compose files, so they cannot catch merge-time failures in networks, service overrides, mounts, or profiles. Add a docker compose ... config smoke test (or CI validation) for the actual observability command.
🧰 Tools
🪛 ast-grep (0.45.0)
[warning] 166-166: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.readFileSync(PROMETHEUS_CONFIG_FILE, "utf8")
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename-typescript)
[warning] 176-176: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.readFileSync(GRAFANA_DASHBOARD_FILE, "utf8")
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename-typescript)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/docker/docker-compose.test.ts` around lines 158 - 180, Extend the
observability compose tests around the existing OBSERVABILITY_COMPOSE_FILE and
observabilityComposeConfig checks with a smoke test that runs the actual Docker
Compose command using the base and observability files, including the
observability profile, and executes config. Assert the command succeeds so
merged networks, overrides, mounts, and profiles are validated rather than only
parsing individual files.
…rimmed to scope) Adds src/alerts/googlechat.ts and registers it in builtins.ts, per #313. Registration and the sender itself were already correct. Trimmed from the original PR before merging: a bundled Grafana/Prometheus observability stack (devops/grafana/*, devops/prometheus/*, docker-compose. observability.yml, docs/observability.md) - unrelated to this issue, shared branch lineage with several other open PRs (#594, #595, #596) that also carry the identical bundle. Left tests/docker/docker-compose.test.ts untouched by reverting to main's version. Updated tests/alerts/builtins.test.ts for the 10th channel (the PR's branch predated matrix/teams/email, so its own copy of this file didn't know about them). Note for a follow-up: src/alerts/discord.ts has the same hostname- validation weakness fixed in #557 (`hostname.includes("discord")`, even looser than Teams's check) - pre-existing, not part of this PR, flagging separately.
…rimmed to scope) Adds src/alerts/googlechat.ts and registers it in builtins.ts, per #313. Registration and the sender itself were already correct. Trimmed from the original PR before merging: a bundled Grafana/Prometheus observability stack (devops/grafana/*, devops/prometheus/*, docker-compose. observability.yml, docs/observability.md) - unrelated to this issue, shared branch lineage with several other open PRs (#594, #595, #596) that also carry the identical bundle. Left tests/docker/docker-compose.test.ts untouched by reverting to main's version. Updated tests/alerts/builtins.test.ts for the 10th channel (the PR's branch predated matrix/teams/email, so its own copy of this file didn't know about them). Note for a follow-up: src/alerts/discord.ts has the same hostname- validation weakness fixed in #557 (`hostname.includes("discord")`, even looser than Teams's check) - pre-existing, not part of this PR, flagging separately.
Recovers real work from PR #594 (Ekpemark), which GitHub shows as merged on 2026-07-31 but whose commit is absent from main's current history — main appears to have diverged/been reset since then, dropping this and two other merged PRs from the same contributor. The other two (#595/#337, #596/#316) turned out to be empty placeholder commits with no real content, so only this one is worth recovering. Adds docker-compose.observability.yml (a --profile observability overlay wiring Prometheus + Grafana against the existing docker-compose.yaml), Prometheus scrape config, and Grafana dashboard provisioning — so the stack comes up pre-wired with zero manual setup, per issue #343. Resolved conflicts with docs/observability.md (a much larger doc was added separately since this PR was originally merged; kept it and added a short section for this profile) and tests/docker/docker-compose.test.ts (kept the typed interfaces added since, extended them with the fields this PR's tests need).
2 participants
What does this PR do?
Does this touch secret-key handling or transaction submission?
Checklist
npm test)npx tsc --noEmit)npm run lint)console.login core logic