Skip to content

Commit d465611

Browse files
committed
feat(mysql): move DSN to env var override with percent-encoding (Bug 1)
Phase 3 fix from issue torrust#410. - Add percent-encoding = "2.0" to Cargo.toml - TrackerServiceConfig: add optional database_path field (Some for MySQL, None for SQLite) with serde skip_serializing_if so it only appears in template context for MySQL - EnvContext::new_with_mysql: percent-encode username and password using a custom USERINFO_ENCODE AsciiSet (encodes @, :, /, #, ?, %, + but NOT unreserved chars like _ so tracker_user stays tracker_user, not tracker%5Fuser) - Build full MySQL DSN and store in tracker.database_path - EnvContext::new_with_mysql: add mysql_host (String) and mysql_port (u16) params so the DSN can be constructed without needing domain types at the infra layer - templates/docker-compose/.env.tera: add TORRUST_TRACKER_CONFIG_OVERRIDE_CORE__DATABASE__PATH inside the Tracker Service Configuration section (inside {%- if mysql %} block); move before MySQL section - templates/docker-compose/docker-compose.yml.tera: inject the new env var into the tracker service environment section, conditionally on {%- if mysql %} - templates/tracker/tracker.toml.tera: remove mysql path = line; replace with a comment explaining the env var override (no Tera tag needed — comment is static) - TrackerContext: remove MysqlTemplateConfig struct and mysql field entirely; the MySQL case only needs database_driver = mysql now - Add tests: percent-encoded DSN, alphanumeric DSN, database_path None for SQLite - Update project_generator.rs test: hardcoded template updated and assertions changed to confirm driver = mysql and absence of path/password lines - Update issue doc: mark all Phase 1-4 items [x]; add Phase 5 status note - 2314 tests pass
1 parent 81ed6d1 commit d465611

9 files changed

Lines changed: 173 additions & 108 deletions

File tree

Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ clap = { version = "4.0", features = [ "derive" ] }
5050
derive_more = { version = "2.1", features = [ "display", "from" ] }
5151
figment = { version = "0.10", features = [ "json" ] }
5252
parking_lot = "0.12"
53+
percent-encoding = "2.0"
5354
rand = "0.9"
5455
reqwest = "0.12"
5556
rust-embed = "8.0"

docs/issues/410-bug-multiple-mysql-configuration-issues.md

Lines changed: 37 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -35,14 +35,14 @@ value.
3535

3636
### Bug 1 — DSN in `tracker.toml`
3737

38-
- [ ] Move the MySQL DSN out of `tracker.toml` and into an environment variable override,
38+
- [x] Move the MySQL DSN out of `tracker.toml` and into an environment variable override,
3939
consistent with `TORRUST_TRACKER_CONFIG_OVERRIDE_CORE__DATABASE__DRIVER` and
4040
`TORRUST_TRACKER_CONFIG_OVERRIDE_HTTP_API__ACCESS_TOKENS__ADMIN`
41-
- [ ] Build the percent-encoded DSN in Rust and expose it in the `.env` file as
41+
- [x] Build the percent-encoded DSN in Rust and expose it in the `.env` file as
4242
`TORRUST_TRACKER_CONFIG_OVERRIDE_CORE__DATABASE__PATH`
43-
- [ ] Pass the new env var into the tracker container via `docker-compose.yml.tera`
44-
- [ ] Remove the raw DSN line from `tracker.toml.tera` for the MySQL case
45-
- [ ] Remove the now-unused `MysqlTemplateConfig` from `TrackerContext`
43+
- [x] Pass the new env var into the tracker container via `docker-compose.yml.tera`
44+
- [x] Remove the raw DSN line from `tracker.toml.tera` for the MySQL case
45+
- [x] Remove the now-unused `MysqlTemplateConfig` from `TrackerContext`
4646

4747
### Bug 2 — Root password not configurable
4848

@@ -55,8 +55,8 @@ value.
5555

5656
### Bug 3 — Reserved username not rejected
5757

58-
- [ ] Add a `ReservedUsername` variant to `MysqlConfigError`
59-
- [ ] Reject `"root"` as the app DB username in `MysqlConfig::new()` with a clear,
58+
- [x] Add a `ReservedUsername` variant to `MysqlConfigError`
59+
- [x] Reject `"root"` as the app DB username in `MysqlConfig::new()` with a clear,
6060
actionable error message
6161

6262
## Specifications
@@ -285,37 +285,39 @@ Tasks are ordered from simplest to most complex.
285285

286286
### Phase 3: Move DSN to env var override and add URL-encoding (Bug 1)
287287

288-
- [ ] Add `percent-encoding` to `Cargo.toml`
289-
- [ ] `TrackerServiceConfig` (`env/context.rs`): add optional database path field
290-
- [ ] `EnvContext::new_with_mysql`: percent-encode username and password with
291-
`utf8_percent_encode(..., NON_ALPHANUMERIC)`, build the full DSN string, store in
292-
the new field
293-
- [ ] `TrackerContext` (`tracker_config/context.rs`): remove `MysqlTemplateConfig` and
288+
- [x] Add `percent-encoding` to `Cargo.toml`
289+
- [x] `TrackerServiceConfig` (`env/context.rs`): add optional database path field
290+
- [x] `EnvContext::new_with_mysql`: percent-encode username and password with
291+
`utf8_percent_encode(..., USERINFO_ENCODE)` (custom AsciiSet preserving RFC 3986
292+
unreserved chars), build the full DSN string, store in the new field
293+
- [x] `TrackerContext` (`tracker_config/context.rs`): remove `MysqlTemplateConfig` and
294294
the MySQL branch that builds it
295-
- [ ] `templates/docker-compose/.env.tera`: add
296-
`TORRUST_TRACKER_CONFIG_OVERRIDE_CORE__DATABASE__PATH` inside `{%- if mysql %}`
297-
- [ ] `templates/docker-compose/docker-compose.yml.tera`: inject the new env var into
298-
the tracker service `environment:` section, conditionally on
299-
`{%- if database.mysql %}`
300-
- [ ] `templates/tracker/tracker.toml.tera`: remove the MySQL `path =` line; add a
295+
- [x] `templates/docker-compose/.env.tera`: add
296+
`TORRUST_TRACKER_CONFIG_OVERRIDE_CORE__DATABASE__PATH` inside `{%- if mysql %}`,
297+
placed in the Tracker Service Configuration section
298+
- [x] `templates/docker-compose/docker-compose.yml.tera`: inject the new env var into
299+
the tracker service `environment:` section, conditionally on `{%- if mysql %}`
300+
- [x] `templates/tracker/tracker.toml.tera`: remove the MySQL `path =` line; add a
301301
comment explaining that the connection path is injected via the env var override
302302

303303
### Phase 4: Tests
304304

305-
- [ ] `mysql.rs`: add `it_should_reject_root_as_username` unit test (Phase 1)
306-
- [ ] `env/context.rs`: add test that `new_with_mysql` produces a correctly
305+
- [x] `mysql.rs`: add `it_should_reject_root_as_username` unit test (Phase 1)
306+
- [x] `env/context.rs`: add test that `new_with_mysql` produces a correctly
307307
percent-encoded DSN for a password containing special characters
308-
- [ ] `env/context.rs`: add test that `new` (SQLite) leaves the database path field as
308+
- [x] `env/context.rs`: add test that `new` (SQLite) leaves the database path field as
309309
`None`
310-
- [ ] `tracker_config/context.rs`: remove or update the test that referenced
310+
- [x] `tracker_config/context.rs`: remove or update the test that referenced
311311
`mysql_password` on `MysqlTemplateConfig`
312-
- [ ] Run `cargo test` to verify all tests pass
312+
- [x] Run `cargo test` to verify all tests pass (2314 passed)
313313

314314
### Phase 5: Linting and pre-commit
315315

316316
- [ ] Run linters: `cargo run --bin linter all`
317317
- [ ] Run pre-commit: `./scripts/pre-commit.sh`
318318

319+
> **Status**: Phases 1–4 complete and committed. Phase 5 pending.
320+
319321
## Acceptance Criteria
320322

321323
> **Note for Contributors**: These criteria define what the PR reviewer will check.
@@ -328,10 +330,10 @@ Tasks are ordered from simplest to most complex.
328330

329331
**Task-Specific Criteria — Bug 3 (reserved username)**:
330332

331-
- [ ] `MysqlConfig::new()` returns `Err(MysqlConfigError::ReservedUsername)` when
333+
- [x] `MysqlConfig::new()` returns `Err(MysqlConfigError::ReservedUsername)` when
332334
username is `"root"`
333-
- [ ] `MysqlConfigError::ReservedUsername` has a `help()` message with an actionable fix
334-
- [ ] A unit test for the reserved username rejection exists and passes
335+
- [x] `MysqlConfigError::ReservedUsername` has a `help()` message with an actionable fix
336+
- [x] A unit test for the reserved username rejection exists and passes
335337

336338
**Task-Specific Criteria — Bug 2 (root password)**:
337339

@@ -349,20 +351,20 @@ Tasks are ordered from simplest to most complex.
349351

350352
**Task-Specific Criteria — Bug 1 (DSN in tracker.toml)**:
351353

352-
- [ ] The rendered `tracker.toml` for a MySQL deployment does **not** contain the
354+
- [x] The rendered `tracker.toml` for a MySQL deployment does **not** contain the
353355
database password
354-
- [ ] The rendered `.env` file contains
356+
- [x] The rendered `.env` file contains
355357
`TORRUST_TRACKER_CONFIG_OVERRIDE_CORE__DATABASE__PATH` with a correctly
356358
percent-encoded DSN when MySQL is configured
357-
- [ ] The rendered `.env` file does **not** contain
359+
- [x] The rendered `.env` file does **not** contain
358360
`TORRUST_TRACKER_CONFIG_OVERRIDE_CORE__DATABASE__PATH` when SQLite is configured
359-
- [ ] The rendered `docker-compose.yml` injects
361+
- [x] The rendered `docker-compose.yml` injects
360362
`TORRUST_TRACKER_CONFIG_OVERRIDE_CORE__DATABASE__PATH` into the tracker service
361363
environment when MySQL is configured
362-
- [ ] A MySQL password containing URL-reserved characters (e.g. `@`, `+`, `/`) produces
363-
a valid, correctly encoded DSN in the `.env` file
364-
- [ ] A MySQL password with only alphanumeric characters is rendered unchanged
365-
- [ ] `MysqlTemplateConfig` no longer exists in `tracker_config/context.rs`
364+
- [x] A MySQL password containing URL-reserved characters (e.g. `@`, `+`, `/`) produces
365+
a valid, correctly encoded DSN in the `.env` file (verified with `tracker_p@ss!word#1`)
366+
- [x] A MySQL password with only alphanumeric characters is rendered unchanged
367+
- [x] `MysqlTemplateConfig` no longer exists in `tracker_config/context.rs`
366368
- [ ] `cargo machete` reports no unused dependencies
367369

368370
## Manual E2E Verification Test

src/application/services/rendering/docker_compose.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,7 @@ impl DockerComposeTemplateRenderingService {
107107
DatabaseConfig::Mysql(mysql_config) => self.create_mysql_contexts(
108108
admin_token.to_string(),
109109
tracker,
110+
mysql_config.host().to_string(),
110111
mysql_config.port(),
111112
mysql_config.database_name().to_string(),
112113
mysql_config.username().to_string(),
@@ -214,6 +215,7 @@ impl DockerComposeTemplateRenderingService {
214215
&self,
215216
admin_token: String,
216217
tracker: TrackerServiceContext,
218+
host: String,
217219
port: u16,
218220
database_name: String,
219221
username: String,
@@ -228,6 +230,8 @@ impl DockerComposeTemplateRenderingService {
228230
database_name.clone(),
229231
username.clone(),
230232
password.clone(),
233+
host,
234+
port,
231235
);
232236

233237
let mysql_config = MysqlSetupConfig {

src/infrastructure/templating/docker_compose/template/wrappers/env/context.rs

Lines changed: 106 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,34 @@
77
//! - Tracker service configuration
88
//! - `MySQL` service configuration (optional)
99
10+
use percent_encoding::{utf8_percent_encode, AsciiSet, CONTROLS};
1011
use serde::Serialize;
1112

1213
use crate::infrastructure::templating::TemplateMetadata;
1314

15+
/// Characters that must be percent-encoded in the userinfo (user/password) part of a MySQL DSN URL.
16+
///
17+
/// Encodes delimiters that have structural meaning in the URL authority component:
18+
/// - `@` — userinfo/host separator
19+
/// - `:` — user/password separator in the DSN
20+
/// - `/` — path separator
21+
/// - `#` — fragment delimiter
22+
/// - `?` — query delimiter
23+
/// - `%` — percent-encoding prefix (must itself be encoded)
24+
/// - `+` — interpreted as space in some URL parsers
25+
///
26+
/// Unreserved characters per RFC 3986 (`-`, `_`, `.`, `~`) and ASCII alphanumerics
27+
/// are NOT encoded, so common usernames like `tracker_user` remain readable.
28+
const USERINFO_ENCODE: AsciiSet = CONTROLS
29+
.add(b' ')
30+
.add(b':')
31+
.add(b'@')
32+
.add(b'/')
33+
.add(b'#')
34+
.add(b'?')
35+
.add(b'%')
36+
.add(b'+');
37+
1438
/// Configuration for the Tracker service
1539
///
1640
/// Contains environment variables for the Torrust Tracker container.
@@ -21,6 +45,13 @@ pub struct TrackerServiceConfig {
2145
/// Database driver type ("sqlite3" or "mysql")
2246
/// Controls which config template the container entrypoint uses
2347
pub database_driver: String,
48+
/// Percent-encoded MySQL DSN for `TORRUST_TRACKER_CONFIG_OVERRIDE_CORE__DATABASE__PATH`
49+
///
50+
/// Only populated when MySQL is configured; `None` for SQLite.
51+
/// Username and password are percent-encoded with `NON_ALPHANUMERIC` to handle
52+
/// URL-reserved characters (e.g. `@`, `+`, `/`, `:`).
53+
#[serde(skip_serializing_if = "Option::is_none")]
54+
pub database_path: Option<String>,
2455
}
2556

2657
/// Configuration for the `MySQL` service
@@ -105,6 +136,7 @@ impl EnvContext {
105136
tracker: TrackerServiceConfig {
106137
api_admin_token: tracker_api_admin_token,
107138
database_driver: "sqlite3".to_string(),
139+
database_path: None,
108140
},
109141
mysql: None,
110142
grafana: None,
@@ -119,8 +151,10 @@ impl EnvContext {
119151
/// * `tracker_api_admin_token` - The admin token for tracker API authentication
120152
/// * `mysql_root_password` - `MySQL` root password
121153
/// * `mysql_database` - `MySQL` database name
122-
/// * `mysql_user` - `MySQL` user
123-
/// * `mysql_password` - `MySQL` password
154+
/// * `mysql_user` - `MySQL` username (percent-encoded in the DSN)
155+
/// * `mysql_password` - `MySQL` password (percent-encoded in the DSN)
156+
/// * `mysql_host` - `MySQL` host (e.g. `"mysql"`, `"localhost"`)
157+
/// * `mysql_port` - `MySQL` port (typically `3306`)
124158
///
125159
/// # Examples
126160
///
@@ -137,6 +171,8 @@ impl EnvContext {
137171
/// "tracker_db".to_string(),
138172
/// "tracker_user".to_string(),
139173
/// "user_pass".to_string(),
174+
/// "mysql".to_string(),
175+
/// 3306,
140176
/// );
141177
/// assert_eq!(context.tracker.database_driver, "mysql");
142178
/// assert!(context.mysql.is_some());
@@ -149,12 +185,20 @@ impl EnvContext {
149185
mysql_database: String,
150186
mysql_user: String,
151187
mysql_password: String,
188+
mysql_host: String,
189+
mysql_port: u16,
152190
) -> Self {
191+
let encoded_user = utf8_percent_encode(&mysql_user, &USERINFO_ENCODE).to_string();
192+
let encoded_password = utf8_percent_encode(&mysql_password, &USERINFO_ENCODE).to_string();
193+
let database_path = format!(
194+
"mysql://{encoded_user}:{encoded_password}@{mysql_host}:{mysql_port}/{mysql_database}"
195+
);
153196
Self {
154197
metadata,
155198
tracker: TrackerServiceConfig {
156199
api_admin_token: tracker_api_admin_token,
157200
database_driver: "mysql".to_string(),
201+
database_path: Some(database_path),
158202
},
159203
mysql: Some(MySqlServiceConfig {
160204
root_password: mysql_root_password,
@@ -277,6 +321,8 @@ mod tests {
277321
"tracker_db".to_string(),
278322
"tracker_user".to_string(),
279323
"user_pass".to_string(),
324+
"mysql".to_string(),
325+
3306,
280326
);
281327

282328
assert_eq!(context.tracker.api_admin_token, "AdminToken456");
@@ -312,6 +358,8 @@ mod tests {
312358
"db".to_string(),
313359
"user".to_string(),
314360
"pass".to_string(),
361+
"mysql".to_string(),
362+
3306,
315363
);
316364

317365
let serialized = serde_json::to_string(&context).unwrap();
@@ -339,6 +387,8 @@ mod tests {
339387
"tracker_db".to_string(),
340388
"tracker_user".to_string(),
341389
"user_pass".to_string(),
390+
"mysql".to_string(),
391+
3306,
342392
);
343393

344394
// Backward compatible getter methods
@@ -349,4 +399,58 @@ mod tests {
349399
assert_eq!(context.mysql_user(), Some("tracker_user"));
350400
assert_eq!(context.mysql_password(), Some("user_pass"));
351401
}
402+
403+
#[test]
404+
fn it_should_produce_percent_encoded_dsn_for_password_with_special_chars() {
405+
let metadata = create_test_metadata();
406+
// password contains URL-reserved characters: @ : / +
407+
let context = EnvContext::new_with_mysql(
408+
metadata,
409+
"Token123".to_string(),
410+
"root_pass".to_string(),
411+
"tracker".to_string(),
412+
"tracker_user".to_string(),
413+
"p@ss:w/ord+1".to_string(),
414+
"mysql".to_string(),
415+
3306,
416+
);
417+
418+
let database_path = context.tracker.database_path.as_deref().unwrap();
419+
// @ -> %40, : -> %3A, / -> %2F, + -> %2B
420+
// _ in username is NOT encoded (unreserved per RFC 3986)
421+
assert_eq!(
422+
database_path,
423+
"mysql://tracker_user:p%40ss%3Aw%2Ford%2B1@mysql:3306/tracker"
424+
);
425+
}
426+
427+
#[test]
428+
fn it_should_produce_unencoded_dsn_for_alphanumeric_password() {
429+
let metadata = create_test_metadata();
430+
let context = EnvContext::new_with_mysql(
431+
metadata,
432+
"Token123".to_string(),
433+
"root_pass".to_string(),
434+
"tracker".to_string(),
435+
"tracker_user".to_string(),
436+
"plainpassword123".to_string(),
437+
"mysql".to_string(),
438+
3306,
439+
);
440+
441+
let database_path = context.tracker.database_path.as_deref().unwrap();
442+
// Alphanumeric password and underscore username are not encoded
443+
assert_eq!(
444+
database_path,
445+
"mysql://tracker_user:plainpassword123@mysql:3306/tracker"
446+
);
447+
}
448+
449+
#[test]
450+
fn it_should_leave_database_path_none_for_sqlite() {
451+
let metadata = create_test_metadata();
452+
let context = EnvContext::new(metadata, "Token123".to_string());
453+
454+
assert!(context.tracker.database_path.is_none());
455+
}
352456
}

src/infrastructure/templating/tracker/template/renderer/project_generator.rs

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -316,9 +316,10 @@ mod tests {
316316
.expect("Failed to read tracker.toml");
317317

318318
assert!(content.contains(r#"driver = "mysql""#));
319-
assert!(
320-
content.contains("path = \"mysql://tracker_user:secure_pass@mysql:3306/tracker_db\"")
321-
);
319+
// DSN is no longer in tracker.toml — it is injected via
320+
// TORRUST_TRACKER_CONFIG_OVERRIDE_CORE__DATABASE__PATH in .env
321+
assert!(!content.contains("path = \"mysql://"));
322+
assert!(!content.contains("secure_pass"));
322323
}
323324

324325
#[test]
@@ -401,7 +402,7 @@ driver = "{{ database_driver }}"
401402
{% if database_driver == "sqlite3" %}
402403
path = "/var/lib/torrust/tracker/database/{{ tracker_database_name }}"
403404
{% elif database_driver == "mysql" %}
404-
path = "mysql://{{ mysql_user }}:{{ mysql_password }}@{{ mysql_host }}:{{ mysql_port }}/{{ mysql_database }}"
405+
{# DSN is injected via TORRUST_TRACKER_CONFIG_OVERRIDE_CORE__DATABASE__PATH in .env #}
405406
{% endif %}
406407
407408
{% for udp_tracker in udp_trackers %}

0 commit comments

Comments
 (0)