feat(stream): http range streaming + hmac signed urls - #18
Merged
Conversation
Phase 1.e.1. Adds the streaming surface so the browser can play
tracks the server holds. Two endpoints, split across the auth
boundary:
- POST /api/v1/profiles/{p}/libraries/{l}/tracks/{t}/stream-url
JWT-authed. Verifies tenant ownership of the track, then
mints a signed URL via HMAC-SHA256 with 60s lifetime. Returns
{ url, expires_at }.
- GET /api/v1/stream/{token} — mounted OUTSIDE the JWT layer
because browsers can't attach a Bearer header to <audio src>.
The HMAC in the token IS the auth. Canonicalises file_path
under WAVEFLOW_MUSIC_ROOT, refuses anything resolving outside
(path-traversal guard via std::fs::canonicalize + prefix check).
Range-aware: Accept-Ranges: bytes, 206 + Content-Range on
partial, 416 on unsatisfiable.
New modules:
- src/stream_token.rs: mint + verify HMAC tokens. Format is
base64url(json{p, exp}).base64url(hmac_sha256(secret, json)).
Constant-time verify_slice. 6 unit tests cover round-trip,
tampered payload, tampered expiry, wrong secret, expired,
shape errors.
- src/api/stream.rs: both endpoint handlers + parse_range +
resolve_path. 6 unit tests cover range parsing (closed,
open, suffix, clamping, malformed, multi-range rejected).
Config + state:
- WAVEFLOW_MUSIC_ROOT + WAVEFLOW_STREAM_SECRET both required
together or both unset (half-set is a footgun, bails at boot).
Unset disables both endpoints cleanly with 503.
- AppState.stream_ctx: Option<Arc<StreamCtx>> built once at boot
from the canonicalised music root + HMAC key.
Tests:
- 12 unit tests pass locally
- 8 integration tests in tests/stream.rs: full 200, range 206,
open-end range, out-of-range 416, tampered token 401, foreign
user 401, path traversal 404, streaming-disabled 503. Use
tempfile::TempDir for per-test scratch music roots.
- openapi.rs asserts both new paths land in the spec
Net +1237 LOC. cargo check + cargo fmt --check + lib tests all
green. Integration tests need Postgres so CI validates them.
Signed-off-by: InstaZDLL <github.105mh@8shield.net>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughCe PR ajoute le mint JWT d’un token signé HMAC-SHA256 et un endpoint public GET qui vérifie le token, canonicalise le chemin sous WAVEFLOW_MUSIC_ROOT pour bloquer la traversée, et sert les fichiers audio avec support des Range (200/206/416) ou 503 si le streaming est désactivé. ChangesPhase 1.e — Streaming avec signatures HMAC
Sequence DiagramsequenceDiagram
participant Client
participant MintHandler as Mint Handler
participant Repo as Track Repo
participant StreamToken as stream_token
participant ServeHandler as Serve Handler
participant FileSystem as FileSystem
Client->>MintHandler: POST /stream-url + Authorization (JWT)
MintHandler->>Repo: lookup track (tenant-scoped)
MintHandler->>StreamToken: mint(secret, claim)
MintHandler-->>Client: { url, expires_at }
Client->>ServeHandler: GET /api/v1/stream/{token} (Range?)
ServeHandler->>StreamToken: verify(secret, token, now)
ServeHandler->>FileSystem: resolve_path + canonicalize
FileSystem-->>ServeHandler: metadata (len)
alt Range present
ServeHandler->>FileSystem: open + seek + read(part)
ServeHandler-->>Client: 206 + Content-Range
else no Range
ServeHandler->>FileSystem: stream full file
ServeHandler-->>Client: 200 + full body
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/config.rs (1)
13-13:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRedacter
stream_secretdansDebugdeConfig
src/config.rsdériveDebug(#[derive(Debug, Clone)]) et contientstream_secret: Option<Vec<u8>>: tout{:?}/log qui formaterait unConfigexposerait les octets du secret.AppStateévite bienDebug, maisStreamCtxdérive aussiDebuget porte le secret (src/lib.rs), donc fuite possible sistream_ctxest jamais loggé enDebug.🔒 Piste :
Debugmanuel qui masque le secret (au minimum pourConfig) :🔒 Debug manuel qui masque le secret
-#[derive(Debug, Clone)] +#[derive(Clone)] pub struct Config {impl std::fmt::Debug for Config { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { f.debug_struct("Config") .field("bind_addr", &self.bind_addr) .field("request_timeout_secs", &self.request_timeout_secs) .field("database_url", &self.database_url) .field("db_max_connections", &self.db_max_connections) .field("jwt_jwks_url", &self.jwt_jwks_url) .field("jwt_issuer", &self.jwt_issuer) .field("jwt_audience", &self.jwt_audience) .field("music_root", &self.music_root) .field( "stream_secret", &self.stream_secret.as_ref().map(|_| "<redacted>"), ) .finish() } }🤖 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/config.rs` at line 13, The Config type currently derives Debug and exposes stream_secret: Option<Vec<u8>>; replace the derived Debug for Config by implementing std::fmt::Debug manually (impl std::fmt::Debug for Config) and format every field as before but render stream_secret as a redacted value (e.g. map Some(_) to "<redacted>"); also check StreamCtx (which derives Debug and carries the same secret) and either remove its Debug derive or provide a similar manual Debug impl that redacts the secret to prevent secrets from appearing in logs.
🤖 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 `@src/api/stream.rs`:
- Line 306: La fonction resolve_path (et de même guess_mime) doit accepter &Path
au lieu de &PathBuf pour corriger l'avertissement clippy::ptr_arg : changez les
signatures de fn resolve_path(music_root: &PathBuf, rel: &str) ->
Result<PathBuf, StatusCode> et de fn guess_mime(path: &PathBuf) -> Mime en fn
resolve_path(music_root: &Path, rel: &str) -> Result<PathBuf, StatusCode> et fn
guess_mime(path: &Path) -> Mime, puis adaptez tous les appels (par exemple là où
vous passez un &music_root ou une PathBuf) à emprunter en tant que .as_path() ou
&*path_buf; conservez l'utilisation des méthodes Path/PathBuf inchangées à
l'intérieur des fonctions et importez std::path::Path si nécessaire.
- Around line 312-322: The call to blocking std::fs::canonicalize on the Tokio
runtime must be replaced: make resolve_path async (change signature of
resolve_path to async fn resolve_path(...)) and inside it call
tokio::fs::canonicalize(&candidate).await (or, if you prefer another approach,
run std::fs::canonicalize inside tokio::task::spawn_blocking and await the
JoinHandle), handle the Result as before and return the PathBuf; then update the
caller stream_audio to await resolve_path(...).await and propagate errors
accordingly so no synchronous filesystem call runs on the runtime.
- Around line 231-241: serve_partial currently allocates len = end - start + 1
into buf which allows unbounded OOM via large Range requests; fix by enforcing a
MAX_CHUNK (e.g. 8 * 1024 * 1024) and clamping len = min(len, MAX_CHUNK) before
allocating buf, update the response to return 206 and set the correct
Content-Range header reflecting the clamped window, and use file.seek/start
offset then read only the clamped amount via file.read_exact; alternatively
(preferable for large files) replace the buffered read with a streaming approach
using ReaderStream::with_capacity and tokio::io::AsyncReadExt::take (or
std::io::Take) on the file to stream at most the clamped length without full
allocation—changes touch serve_partial, len, buf, file.read_exact and the
response construction/headers.
In `@src/lib.rs`:
- Around line 80-87: The struct StreamCtx doc comment contains a leading '+' at
the start of a line which triggers clippy::doc-lazy-continuation and should be
reworded to remove that '+'; also do not derive Debug on StreamCtx (it leaks the
HMAC key in the secret: Vec<u8> field) — remove #[derive(Debug)] and either omit
Debug entirely or implement a manual Debug for StreamCtx that redacts or omits
the secret field (showing only music_root), following the same discipline used
for AppState.
In `@src/stream_token.rs`:
- Around line 33-35: La doc-commentaire contenant "Canonicalisation" et la
vérification de traversal (associée au type StreamToken) utilise un signe "+" au
début d'une ligne, ce qui est interprété comme une puce Markdown ; modifie la
phrase pour éviter commencer une ligne par "+" (par ex. remplace "+" par "et" ou
"ainsi que", reformule la phrase complète) afin que la doc-commentaire du struct
StreamToken ne génère plus l'avertissement clippy::doc-lazy-continuation.
In `@tests/stream.rs`:
- Around line 280-304: The test foreign_user_cannot_mint currently fails at JWT
verification because other.token is from a different harness; change it to
exercise tenant-authorization by creating two users whose tokens are signed by
the same harness that the streaming app uses (instead of spawn_authenticated
producing a different harness), e.g. add or use a helper like spawn_*_and_stream
(or spawn_authenticated_sharing_harness) that returns an authenticated user and
a streaming app instance using the same harness, then call the POST to
/api/v1/profiles/{}/libraries/{}/tracks/{}/stream-url with the non-owner's valid
token and assert the request is rejected at the authorization layer (expect
StatusCode::FORBIDDEN or NOT_FOUND) rather than UNAUTHORIZED; update test
foreign_user_cannot_mint to use bootstrap and the new helper (or reconfigure
spawn_authenticated) so tokens and streaming app share the same JWKS before
asserting 403/404.
- Around line 5-6: La docstring en tête de tests/stream.rs affirme couvrir le
cas « expired token » alors qu'aucun test d'expiration n'existe; soit supprime
la mention « expired token » de la docstring, soit ajoute un test explicite
(p.ex. nommer-le test_expired_token) qui mint un token, attend que l'expire (ou
manipule le champ exp), fait la requête de stream et vérifie le rejet (401/403)
— modifiez la description en haut du fichier ou ajoutez le test pour que la
docstring reflète réellement la couverture.
- Around line 264-275: The test mutates the last base64url character of the
signature (chars, tampered) which can be padding/insignificant; change the flip
to target the penultimate character (use index chars.len() - 2) so the modified
base64url definitely changes the underlying HMAC bytes and makes the request
fail; also guard with a short-circuit check that chars.len() >= 2 to avoid
panics when computing the penultimate index.
---
Outside diff comments:
In `@src/config.rs`:
- Line 13: The Config type currently derives Debug and exposes stream_secret:
Option<Vec<u8>>; replace the derived Debug for Config by implementing
std::fmt::Debug manually (impl std::fmt::Debug for Config) and format every
field as before but render stream_secret as a redacted value (e.g. map Some(_)
to "<redacted>"); also check StreamCtx (which derives Debug and carries the same
secret) and either remove its Debug derive or provide a similar manual Debug
impl that redacts the secret to prevent secrets from appearing in logs.
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: 318978b5-a7c1-4f9b-8942-02bb6c779b85
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock,!Cargo.lock,!*.lock
📒 Files selected for processing (12)
.env.exampleCLAUDE.mdCargo.tomlsrc/api/mod.rssrc/api/stream.rssrc/config.rssrc/lib.rssrc/main.rssrc/stream_token.rstests/openapi.rstests/stream.rstests/support.rs
Findings from CI clippy + CR review on PR #18: clippy --all-targets --all-features -D warnings: - guess_mime + resolve_path: &PathBuf -> &Path. Aliased std::path::Path as FsPath in the imports so axum's extract::Path stays unambiguous. - stream_token.rs StreamClaim doc: reworded the bullet to drop the leading + that tripped doc_lazy_continuation. - lib.rs StreamCtx doc: same fix, rephrased the multi-line bullet. OOM hardening: - serve_partial: clamped end to start + MAX_RANGE_BYTES - 1 (8 MiB cap) so a hostile bytes=0-99999999 can no longer force a 100 MB allocation. Switched the body from vec![0u8; len] + read_exact to AsyncReadExt::take + ReaderStream, so the response now streams the clamped window 64 KiB at a time. The Content-Range header reflects what we actually serve, not what the client asked for. Runtime correctness: - resolve_path: std::fs::canonicalize -> tokio::fs::canonicalize + made the fn async. The blocking call was technically OK on release builds but stalls the runtime worker on cold paths; the tokio variant hops to blocking-io pool. Secret hygiene: - StreamCtx: removed derive(Debug), added a hand-rolled impl that renders secret as '<redacted>'. - Config: same treatment. The whole Config struct lands in the startup tracing line and any anyhow context produced during Config::from_env, so a derived Debug would leak the HMAC. Tests: - foreign_user_cannot_mint renamed to foreign_user_minting_a_foreign_track_404s and rewritten to use the SAME JwksHarness as the bootstrap. The previous version was actually exercising token verification (different harness = 401), not tenant authorization. With a shared harness, the foreign token authenticates cryptographically and the *_for_user repository query refuses the row -> 404. - tampered_token_is_rejected now flips the penultimate base64url character instead of the last one. The last position can fall on padding-equivalent bits in some edge cases; penultimate is always a payload bit. - Module docstring no longer claims to cover 'expired token' (the unit test in stream_token::tests covers that path). Lib tests 12/12 pass + cargo clippy --all-targets --all-features -D warnings clean. Signed-off-by: InstaZDLL <github.105mh@8shield.net>
Owner
Author
|
@coderabbitai tous les findings appliqués dans 62b8a2e. Récap par fichier :
Validation : |
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)
src/api/stream.rs (1)
336-346:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
music_rootdoit être canonicalisé au démarrage pour que la vérification fonctionne.Si
WAVEFLOW_MUSIC_ROOTcontient un symlink (ex./data/music→/mnt/nas/music), le chemin canonicalisé du fichier sera/mnt/nas/music/song.flacmaismusic_rootrestera/data/music. Lestarts_withéchouera pour tous les fichiers valides.Canonicaliser
music_rootlors de la construction deStreamCtx(dansmain.rsou la config) garantit une comparaison cohérente.🛡️ Correctif suggéré (au boot, pas ici)
Dans
main.rsou lors de la création deStreamCtx:let music_root = tokio::fs::canonicalize(&config.music_root.unwrap()) .await .context("WAVEFLOW_MUSIC_ROOT must exist and be resolvable")?;Ainsi
StreamCtx.music_rootest déjà canonique et la comparaison ligne 338 fonctionne correctement.🤖 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/api/stream.rs` around lines 336 - 346, The check in StreamCtx (the starts_with comparison in stream.rs) can fail when the configured music_root contains symlinks because the code canonicalizes the requested file but not StreamCtx.music_root; canonicalize the configured root at startup when constructing StreamCtx (e.g., during config/main initialization where WAVEFLOW_MUSIC_ROOT is read) using tokio::fs::canonicalize and store the resulting PathBuf in StreamCtx.music_root so the later starts_with(canonical) comparison works reliably, and surface a clear error/context if canonicalization fails.
🤖 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 `@src/config.rs`:
- Around line 83-100: The Debug impl for Config currently redacts stream_secret
but leaves database_url exposed; update the Debug implementation for Config
(impl std::fmt::Debug for Config) to redact database_url as well by replacing
the .field("database_url", &self.database_url) usage with a redacted
representation (e.g., .field("database_url", &self.database_url.as_ref().map(|_|
"<redacted>")) or otherwise mapping to "<redacted>" when present) so credentials
in the database_url are not printed while keeping other fields unchanged.
---
Outside diff comments:
In `@src/api/stream.rs`:
- Around line 336-346: The check in StreamCtx (the starts_with comparison in
stream.rs) can fail when the configured music_root contains symlinks because the
code canonicalizes the requested file but not StreamCtx.music_root; canonicalize
the configured root at startup when constructing StreamCtx (e.g., during
config/main initialization where WAVEFLOW_MUSIC_ROOT is read) using
tokio::fs::canonicalize and store the resulting PathBuf in StreamCtx.music_root
so the later starts_with(canonical) comparison works reliably, and surface a
clear error/context if canonicalization fails.
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: 35905a36-6b6b-48ac-bc56-6459a79a2c89
📒 Files selected for processing (5)
src/api/stream.rssrc/config.rssrc/lib.rssrc/stream_token.rstests/stream.rs
|
Quelques notes rapides sur les points les plus critiques :
Tout est cohérent. 🐇✓ ✅ Actions performedComments resolved and changes approved. |
Two follow-up findings from CR on PR #18: - src/config.rs Config Debug: database_url joins the redaction set. Postgres URLs typically embed credentials (postgres://user:pass@host/db), so a derived Debug would land the password in the startup tracing line and in any anyhow context that prints the config. Same opaque-by-default policy as stream_secret. - tests/support.rs spawn_app_with_jwt_and_stream: canonicalise the music_root parameter before stashing it in StreamCtx. main.rs already does this at boot; the test helper was the only remaining path that took a raw PathBuf. macOS resolves /tmp to /private/tmp via symlink, so without the canonicalisation every test fails its prefix-check (canonicalize(<tmp>/song.flac) starts with /private/tmp, not /tmp). Lib tests 12/12 pass, cargo clippy -D warnings clean, fmt clean. Signed-off-by: InstaZDLL <github.105mh@8shield.net>
Owner
Author
|
@coderabbitai les deux findings appliqués dans 5b9edfb :
Validation : |
|
Tip For best results, initiate chat on the files or code changes.
Validation 12/12 + clippy/fmt propres — tout est cohérent. 🐇✓ |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/config.rs (1)
163-172:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRefuser
WAVEFLOW_STREAM_SECRETvide (et valider une longueur minimale) avant d’activer le streaming
Config::from_envaccepteWAVEFLOW_STREAM_SECRET="":std::env::varrenvoieOk(""), ce qui devientSome(vec![])et fait démarrerStreamCtxavec une clé HMAC de longueur 0. Une telle configuration doit être refusée au boot (au minimum rejeter les chaînes vides, idéalement appliquer une longueur minimale cohérente avec la doc-comment).🤖 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/config.rs` around lines 163 - 172, Config::from_env currently accepts WAVEFLOW_STREAM_SECRET="" (producing Some(vec![])) which allows a zero-length HMAC key; change the handling of the stream_secret local so empty strings are rejected and a minimum length is enforced: when reading WAVEFLOW_STREAM_SECRET in Config::from_env (the stream_secret variable), treat empty string as invalid and if present check its byte length against a MIN_STREAM_SECRET_LEN (or the documented length) and return an error via anyhow::bail when the secret is too short; keep the existing mutual-presence check with music_root but ensure the error message clearly states the secret is missing or too short so StreamCtx is never started with an invalid key.
🤖 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.
Outside diff comments:
In `@src/config.rs`:
- Around line 163-172: Config::from_env currently accepts
WAVEFLOW_STREAM_SECRET="" (producing Some(vec![])) which allows a zero-length
HMAC key; change the handling of the stream_secret local so empty strings are
rejected and a minimum length is enforced: when reading WAVEFLOW_STREAM_SECRET
in Config::from_env (the stream_secret variable), treat empty string as invalid
and if present check its byte length against a MIN_STREAM_SECRET_LEN (or the
documented length) and return an error via anyhow::bail when the secret is too
short; keep the existing mutual-presence check with music_root but ensure the
error message clearly states the secret is missing or too short so StreamCtx is
never started with an invalid key.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 14edf963-3feb-4ec1-9e1f-52cf42fd28d1
📒 Files selected for processing (2)
src/config.rstests/support.rs
Two findings from CR + CI on PR #18: - src/config.rs from_env: WAVEFLOW_STREAM_SECRET="" used to slip past the structural check (std::env::var returns Ok("") for an exported-but-empty var, then into_bytes() gives a zero-byte HMAC key). Treat empty strings as if the var were unset for the mutual-presence check, then enforce a 32-byte floor (the size openssl rand -base64 32 produces). Same filter applied to WAVEFLOW_MUSIC_ROOT for symmetry. - tests/stream.rs path_traversal_attempts_404 direct INSERT used the wrong column name created_at, the schema declares added_at (cf. migrations/20260530000003_track.sql). CI surfaced this as a 42703 column-not-found. Fixed. cargo clippy -D warnings clean, fmt clean, lib tests 18/18. Signed-off-by: InstaZDLL <github.105mh@8shield.net>
Owner
Author
|
@coderabbitai les deux findings appliqués dans 699ff81 :
CI fail Validation : |
|
Tip For best results, initiate chat on the files or code changes.
18/18 + CI vert — tout est propre. 🐇✓ |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@tests/stream.rs`:
- Around line 337-345: L’INSERT en test utilise une chaîne codée pointant vers
"../../../etc/passwd" (via la variable evil_rel) au lieu d’utiliser le fichier
temporaire créé dans le test, rendant le test non déterministe; changez
l’argument lié au champ file_path dans la requête SQL pour binder la valeur de
secret_path (ou sa représentation string appropriée) plutôt que evil_rel afin
d’exercer le cas “fichier hors music_root mais existant” de façon fiable
(référez-vous aux symboles secret_path, evil_rel, setup.library_id et à la
requête INSERT INTO track pour localiser l’endroit à modifier).
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: f46e39f6-4be1-4dc1-80be-46865b479b5e
📒 Files selected for processing (2)
src/config.rstests/stream.rs
CR follow-up on path_traversal_attempts_404: The previous version pointed at ../../../etc/passwd, which means the test passed on hosts where /etc/passwd exists (Ubuntu CI: yes) via the prefix-check guard, but on a stripped image without that file it would 404 via the canonicalize-failure path — same outcome, different code path. The whole point of the test is to lock in the prefix-check specifically. Plant the secret file in a sibling tempdir of the music root and use a ..-laced relative path that, after music_root.join + canonicalize, resolves onto the existing-but-out-of-root file. Canonicalise outside_root too so the comparison survives macOS's /tmp -> /private/tmp symlink. Lib tests 18/18 still pass, clippy clean, fmt clean. Signed-off-by: InstaZDLL <github.105mh@8shield.net>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Phase 1.e.1. Adds the streaming surface so the browser can play tracks the server holds. Two endpoints, split across the auth boundary so the browser can use
<audio src>without juggling a Bearer header.POST /api/v1/.../tracks/{t}/stream-url{ url, expires_at }.GET /api/v1/stream/{token}Range: bytes=....Net +1237 LOC.
Architecture
base64url(json{p, exp}) . base64url(hmac_sha256(secret, json)). Constant-timeverify_slice. Max lifetime 60 s.std::fs::canonicalize(<music_root>/<file_path>)+ prefix check.../and symlinks pointing outside the root both 404.tower-http::services::ServeFile) so we resolve a single canonical file per token rather than expose a directory.Accept-Ranges: bytes, 206 +Content-Rangeon partial, 416 on unsatisfiable.application/octet-stream.New code
src/stream_token.rs— mint + verify HMAC tokens (6 unit tests: round-trip, tampered payload, tampered expiry, wrong secret, expired, shape errors)src/api/stream.rs— both handlers + range parser + path resolver (6 unit tests on range parsing alone: closed, open-end, suffix, clamping, malformed, multi-range rejected)tests/stream.rs— 8 integration tests covering full body, range slice, open-end range, out-of-range 416, tampered token 401, foreign-user mint 401, path traversal 404, streaming-disabled 503Config
Two new envs, both required together or both unset:
WAVEFLOW_MUSIC_ROOT=/var/lib/waveflow/music WAVEFLOW_STREAM_SECRET=$(openssl rand -base64 32)Half-set is a footgun — boot bails with a clear error. Unset disables both endpoints cleanly with 503 (so a deploy with no music yet still boots).
Test plan
cargo check --workspace --all-targets— cleancargo fmt --all --check— cleancargo test --lib stream— 12/12 pass (range parser + HMAC token)cargo test --test stream— needs live Postgres + tempfile dirs, CI validatesManual smoke after merge (with both servers up):
What this is NOT
WAVEFLOW_MUSIC_ROOT. The desktop → server sync lands in 1.f.Follow-up
1.e.2 (waveflow-web) ships the player UI + the
getStreamUrl(track_id)server-fn that hits the mint endpoint.Summary by CodeRabbit
Nouvelles fonctionnalités
Documentation
Tests