feat: total remove of database encrytpion logic#676
Conversation
WalkthroughThis PR removes database encryption and password handling infrastructure throughout the codebase. The Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Important Merge conflicts detected (Beta)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
📝 Coding Plan
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: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docker/README.md (1)
65-77:⚠️ Potential issue | 🟡 MinorAvoid telling readers to start Compose twice.
Both Step 4 examples already invoke
make docker-up, so Step 5 asks readers to run the same startup command again. Either make Step 4 only setMOSTRO_RELAY_LOCAL_PORT, or make Step 5 conditional on not using the inline form.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docker/README.md` around lines 65 - 77, The README currently shows two examples in Step 4 that both run `make docker-up`, causing Step 5 to repeat the same startup command; update the examples so Step 4 only demonstrates setting the MOSTRO_RELAY_LOCAL_PORT environment variable (e.g., use `export MOSTRO_RELAY_LOCAL_PORT=7000` and `MOSTRO_RELAY_LOCAL_PORT=7000` forms without `make docker-up`) or alternatively make Step 5 state “If you haven’t already run `make docker-up`, run it now” to avoid instructing users to start Compose twice; modify the text around MOSTRO_RELAY_LOCAL_PORT and `make docker-up` accordingly to remove the duplicate invocation.
🧹 Nitpick comments (1)
docs/STARTUP_AND_CONFIG.md (1)
86-88: Drop the hardcoded line range in this section.
src/config/types.rs:21-26will drift quickly. Please reference justsrc/config/types.rsor the relevant config type instead.As per coding guidelines: "Do not hardcode source code line numbers in documentation. Reference file paths (e.g.,
src/app/take_buy.rs) or function names (e.g.,fn take_buy_action) instead, which are stable and searchable."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/STARTUP_AND_CONFIG.md` around lines 86 - 88, Remove the hardcoded line range "src/config/types.rs:21-26" from the Database docs and instead reference the source file and config type (e.g., src/config/types.rs and the Database config struct/type that defines `url`) or the specific config field names (`url`) so the doc points to stable, searchable identifiers rather than volatile line numbers; update the text to say something like "see src/config/types.rs (Database config/type) for the `url` field" and keep the existing examples.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Cargo.toml`:
- Line 73: The Cargo.toml dependency points to a non-existent release
"mostro-core = { version = \"0.8.0\", features = [\"sqlx\"] }"; update it to a
published version or revert to the last released version (e.g., change the
mostro-core version string from "0.8.0" to "0.7.1") or replace it with a
path/git dependency until upstream publishes 0.8.0; look for the dependency
entry named mostro-core in Cargo.toml and either set version = "0.7.1" or switch
to an appropriate git/path override referencing the branch/commit that contains
the 0.8.0 changes.
In `@docs/RPC_RATE_LIMITING.md`:
- Around line 5-12: Update the docs to clearly state that failed-attempt
backoff/lockout are generic capabilities of the in-memory rate limiter
(src/rpc/rate_limiter.rs) and not exercised by the current ValidateDbPassword
RPC stub; explicitly note that the validate_db_password handler in
src/rpc/service.rs only calls record_success and ignores the password, and
either remove or re-label paragraphs that describe failure/lockout behavior as
“generic limiter behavior (not used by ValidateDbPassword)”; ensure references
to ValidateDbPassword, validate_db_password, and record_success are used so
readers can map the doc to the implementation.
In `@docs/STARTUP_AND_CONFIG.md`:
- Around line 87-88: The docs example shows a SQLite URL using a tilde
(`sqlite://~/.mostro/mostro.db`) which SQLx does not expand; update the `url`
example in the STARTUP_AND_CONFIG.md section describing the `url` (String)
database connection to remove the tilde example and replace it with either a
fully expanded absolute path (e.g., `sqlite:///home/user/.mostro/mostro.db`) or
a relative path (e.g., `sqlite://mostro.db`) so the `url` example is valid for
SQLx's parser.
---
Outside diff comments:
In `@docker/README.md`:
- Around line 65-77: The README currently shows two examples in Step 4 that both
run `make docker-up`, causing Step 5 to repeat the same startup command; update
the examples so Step 4 only demonstrates setting the MOSTRO_RELAY_LOCAL_PORT
environment variable (e.g., use `export MOSTRO_RELAY_LOCAL_PORT=7000` and
`MOSTRO_RELAY_LOCAL_PORT=7000` forms without `make docker-up`) or alternatively
make Step 5 state “If you haven’t already run `make docker-up`, run it now” to
avoid instructing users to start Compose twice; modify the text around
MOSTRO_RELAY_LOCAL_PORT and `make docker-up` accordingly to remove the duplicate
invocation.
---
Nitpick comments:
In `@docs/STARTUP_AND_CONFIG.md`:
- Around line 86-88: Remove the hardcoded line range "src/config/types.rs:21-26"
from the Database docs and instead reference the source file and config type
(e.g., src/config/types.rs and the Database config struct/type that defines
`url`) or the specific config field names (`url`) so the doc points to stable,
searchable identifiers rather than volatile line numbers; update the text to say
something like "see src/config/types.rs (Database config/type) for the `url`
field" and keep the existing examples.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8bef6ff8-89cc-4d55-b8a5-d140cf05dec6
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (20)
.gitignoreCargo.tomldocker/README.mddocker/compose.ymldocs/RPC.mddocs/RPC_RATE_LIMITING.mddocs/STARTUP_AND_CONFIG.mdexamples/rpc_client.rsproto/admin.protosrc/app/admin_take_dispute.rssrc/app/rate_user.rssrc/app/release.rssrc/app/take_buy.rssrc/app/take_sell.rssrc/app/trade_pubkey.rssrc/cli.rssrc/db.rssrc/main.rssrc/rpc/service.rssrc/util.rs
💤 Files with no reviewable changes (1)
- docker/compose.yml
|
@coderabbitai check again https://crates.io/crates/mostro-core |
|
Tip For best results, initiate chat on the files or code changes. The search results show the versions page listing only up to
If 🧠 Learnings used |
|
Ok @coderabbitai try fetching from version API: |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chainThe web search still shows 🏁 Script executed: Length of output: 731
The 🧠 Learnings used |
![mostronatorcoder[bot]](https://avatars.githubusercontent.com/in/2922838?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Summary
Scope
Documentation and cleanup PR completing the SQLite encryption removal story. Removes unused:
MOSTRO_DB_PASSWORDfrom Docker configrpassword,argon2,secrecycrates- Obsolete documentation
CI Status
✅ All checks passing (build, test, MSRV, fmt, clippy)
Code Changes
src/util.rs: Variable rename user_decrypted_key → master_key. Purely cosmetic — aligns naming with the fact that there is no decryption happening anymore. LGTM.
Cargo.toml: Bumps mostro-core to 0.8.0. CI passes with this version, confirming it exists on crates.io.
Documentation
| File | Assessment |
|---|---|
docs/RPC.md |
Good, clarifies ValidateDbPassword is backward-compat stub |
docs/RPC_RATE_LIMITING.md |
Needs clarification (see below) |
docs/STARTUP_AND_CONFIG.md |
Good, SQLx tilde note is accurate |
docker/README.md, docker/compose.yml |
Good cleanup |
proto/admin.proto |
Good comment updates |
examples/rpc_client.rs |
Good, empty password with comment |
CodeRabbit Issue (Minor)
docs/RPC_RATE_LIMITING.md: The intro now correctly states that validate_db_password only calls record_success and ignores the password. However, the table showing "Failed Attempts" and "Lockout" behavior still reads as if it applies to this endpoint.
Suggestion: Add a note before the table like:
> **Note:** The following describes the generic `RateLimiter` capabilities. The current `ValidateDbPassword` handler does not call `record_failure`, so these behaviors are not exercised by that endpoint.Verdict
Approve — The changes are correct and CI passes. The documentation clarification is minor and can be addressed in a follow-up if desired.
LGTM 🦀
Database encryption was removed from Mostro (PR #676). The MOSTRO_DB_PASSWORD environment variable is no longer used. Removes Step 9 (Set Database Password) and renumbers Step 10 -> Step 9 across all 5 languages: EN, ES, FR, IT, PT. Related: MostroP2P/mostro#676
2 participants
Summary
Completes the SQLite database encryption removal story: runtime already used plain SQLite; this PR removes leftover Docker/env/password documentation drift, aligns RPC and startup docs with actual behavior, and clarifies the backward-compatible
ValidateDbPasswordstub.Changes
MOSTRO_DB_PASSWORDfromdocker/compose.yml; trimmeddocker/ENV_VARIABLES.mdanddocker/README.md(wording; no DB password env).docs/STARTUP_AND_CONFIG.md— global state snippet matchessrc/config/mod.rs; removed obsoleteMOSTRO_DB_PASSWORD; SQLite-only DB URL examples; note that there is no DB password global.docs/RPC.md—ValidateDbPasswordrequest fields corrected (passwordonly, ignored).docs/RPC_RATE_LIMITING.md— aligned withvalidate_db_password(ignored password, per-IP check, always success).proto/admin.proto— comments only (backward compat, no DB encryption).examples/rpc_client.rs— empty password with comment that server ignores it.Summary by CodeRabbit
Release Notes
Chores
--decrypt-dbCLI option.mostro-coredependency to version 0.8.0..gitignorefor IDE configuration.Documentation
ValidateDbPasswordRPC endpoint now serves as backward-compatibility stub.