fix/storage-encryption - #80
Conversation
# Conflicts: # docker-compose.yml
📝 WalkthroughWalkthroughThe change adds AES-256-GCM decryption for encrypted agent storage status data, makes storage fields backward-compatible during deserialization, integrates decryption into ChangesEncrypted storage status handling
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant StatusService
participant AgentAPI
participant decrypt_json_gcm
participant DatabaseStatus
StatusService->>AgentAPI: agent_status(...)
AgentAPI-->>StatusService: encrypted DatabaseStatus
StatusService->>decrypt_json_gcm: decrypt storages_ciphertext
decrypt_json_gcm-->>StatusService: plaintext JSON
StatusService->>DatabaseStatus: parse and assign storages
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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.yml`:
- Line 22: The docker-compose EDGE_KEY embeds a reusable AES-256 master key.
Remove the hardcoded value, source EDGE_KEY from an untracked environment or
secret configuration, and provide a clearly documented local-development
placeholder or setup path; verify the exposed key is revoked and never reused by
real edge or agent deployments.
In `@src/services/status.rs`:
- Around line 50-55: Replace the `.unwrap()` in the `agent_status` call within
the ping/status handling method with explicit `Option`-to-error conversion,
returning a descriptive error when the response is `None` while preserving the
existing `Result<PingResult, Box<dyn Error>>` flow.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 37545fd7-2042-4054-8619-6e086d1f7108
📒 Files selected for processing (6)
docker-compose.ymlsrc/services/api/models/agent/status.rssrc/services/status.rssrc/tests/services/api_models_tests.rssrc/tests/utils/file_tests.rssrc/utils/file.rs
| LOG: debug | ||
| TZ: "Europe/Paris" | ||
| EDGE_KEY: "eyJzZXJ2ZXJVcmwiOiJodHRwOi8vbG9jYWxob3N0Ojg4ODciLCJhZ2VudElkIjoiMWM4NmQ5NGEtMGVjOC00NzkxLTk0ZTEtOWRlYmJmMWY0M2I5IiwibWFzdGVyS2V5QjY0IjoiMUh0djdtWCtYVkJxL0IzUEV2WDlZZjlQeUdVZW5oRHlXemo5THRqNW90WT0ifQ==" | ||
| EDGE_KEY: "eyJzZXJ2ZXJVcmwiOiJodHRwOi8vbG9jYWxob3N0Ojg4ODciLCJhZ2VudElkIjoiZWZhYTM0YTQtZDY1NC00OGQ3LTgwNDYtNjRkMWExYTA1M2FlIiwibWFzdGVyS2V5QjY0IjoiMUh0djdtWCtYVkJxL0IzUEV2WDlZZjlQeUdVZW5oRHlXemo5THRqNW90WT0ifQ==" |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
Committed EDGE_KEY embeds a real AES-256 master key.
This value base64-decodes to JSON containing masterKeyB64 — the actual AES-256 master key used by decrypt_json_gcm. Even for a local dev compose, committing a working master key to VCS is a secret-hygiene risk (and it triggers the secret scanner). Confirm this key is dev-only, is never reused for any real edge/agent, and consider sourcing it from an untracked .env / secret instead of inlining it.
🧰 Tools
🪛 Betterleaks (1.6.0)
[high] 22-22: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 22-22: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🤖 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.yml` at line 22, The docker-compose EDGE_KEY embeds a reusable
AES-256 master key. Remove the hardcoded value, source EDGE_KEY from an
untracked environment or secret configuration, and provide a clearly documented
local-development placeholder or setup path; verify the exposed key is revoked
and never reused by real edge or agent deployments.
Source: Linters/SAST tools
| let mut result = self | ||
| .ctx | ||
| .api | ||
| .agent_status(&edge_key.agent_id, &version_str, databases_payload) | ||
| .await? | ||
| .unwrap(); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Avoid .unwrap() on the status response — convert None to an error.
agent_status(...).await?.unwrap() panics if the endpoint returns None, taking down the ping task instead of surfacing a recoverable error via the Result<PingResult, Box<dyn Error>> return type.
🛡️ Proposed fix
- let mut result = self
- .ctx
- .api
- .agent_status(&edge_key.agent_id, &version_str, databases_payload)
- .await?
- .unwrap();
+ let mut result = self
+ .ctx
+ .api
+ .agent_status(&edge_key.agent_id, &version_str, databases_payload)
+ .await?
+ .ok_or("agent_status returned no PingResult")?;🤖 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 `@src/services/status.rs` around lines 50 - 55, Replace the `.unwrap()` in the
`agent_status` call within the ping/status handling method with explicit
`Option`-to-error conversion, returning a descriptive error when the response is
`None` while preserving the existing `Result<PingResult, Box<dyn Error>>` flow.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
1 participant
Summary by CodeRabbit
New Features
Bug Fixes