Init #3

Merged

navicore merged 4 commits from init into main

2026-02-25 23:28:32 +00:00

navicore commented

2026-02-25 13:59:18 +00:00

(Migrated from github.com)

No description provided.

navicore commented

2026-02-25 16:54:06 +00:00

(Migrated from github.com)

@claude where is my review???!!

👀 1

claude[bot] commented

2026-02-25 16:54:20 +00:00

(Migrated from github.com)

Claude finished @navicore's task in 1m 23s —— View job

Code Review

Gather context (read CLAUDE.md, changed files, git diff)
Review src/cli/user.rs changes
Review src/server/authorize.rs changes
Review database files added to repo
Post final review

Summary

This PR contains compilation fixes and a meaningful security improvement, but also includes two files that must not be committed.

🚨 Critical: Database WAL files committed

anz.db-shm and anz.db-wal are SQLite Write-Ahead Logging auxiliary files — transient shared memory and journal files generated at runtime. They should never be in source control:

They may contain sensitive data (user records, session tokens, password hashes)
They are environment-specific and will cause corruption if shared across machines
They change on every database write

These need to be removed from the PR and added to .gitignore:

# SQLite runtime files
*.db-shm
*.db-wal
*.db

Also note: .gitignore line 20 contains a stray EOF < /dev/null — a heredoc artifact that should be removed. Fix this →

`src/cli/user.rs` — Potential UX regression

Lines 52–58: The change splits rpassword::prompt_password("Password: ") into eprint!("Password: ") + rpassword::read_password().

// Before (explicit, atomic)
let password = rpassword::prompt_password("Password: ")?;

// After (split into two calls)
eprint!("Password: ");
let password = rpassword::read_password()?;

eprint! does not guarantee a stderr flush. On some systems, the prompt could be buffered and displayed after the terminal is already in no-echo mode, leaving the user staring at a blank prompt with no indication they should type. rpassword::prompt_password handles this internally by flushing before disabling echo.

Unless there's a specific reason for this split (e.g. test harness interference), reverting to prompt_password is safer. If the split is intentional, add an explicit flush: Fix this →

eprint!("Password: ");
let _ = std::io::stderr().flush();
let password = rpassword::read_password()?;

`src/server/authorize.rs` — Mostly positive

.as_slice() additions (lines 97, 203, 269): These are correct API compatibility fixes. Sha256::digest() returns a GenericArray, and .as_slice() gives the expected &[u8]. No concerns.

Explicit askama::Error annotations (lines 134, 322): These help the type inferencer and improve clarity. Fine.

Redirect URL construction (lines 287–293): This is a genuine security improvement.

// Before — manual string formatting
let redirect_url = format!(
    "{}?code={}&state={}",
    q.redirect_uri,
    urlencoding::encode(&raw_code),
    urlencoding::encode(state_param),
);

// After — structured URL building
let mut redirect_parsed = url::Url::parse(&q.redirect_uri)
    .map_err(|e| AppError::Internal(format!("invalid redirect_uri: {e}")))?;
redirect_parsed
    .query_pairs_mut()
    .append_pair("code", &raw_code)
    .append_pair("state", state_param);

The new approach: validates redirect_uri is a well-formed URL, correctly handles pre-existing query parameters, and delegates encoding to the url crate. Good change.

One minor note: this code path is reached only after redirect_uri has already passed the registered URI allowlist check (line 88/180), so the map_err(...Internal...) error should be unreachable in practice. Consider whether it should be a debug_assert! or just left as-is for defense in depth.

Verdict

	Status
`anz.db-shm` / `anz.db-wal` committed	❌ Must fix before merge
`.gitignore` has `EOF < /dev/null` artifact	⚠️ Minor cleanup
Password prompt stderr flush	⚠️ Potential UX issue
`.as_slice()` fixes	✅ Correct
`askama::Error` annotations	✅ Fine
URL-based redirect construction	✅ Security improvement

Do not merge until the SQLite WAL files are removed from the commit history and .gitignore is updated to prevent re-adding them.

**Claude finished @navicore's task in 1m 23s** —— [View job](https://github.com/navicore/anz/actions/runs/22406973390) --- ### Code Review - [x] Gather context (read CLAUDE.md, changed files, git diff) - [x] Review `src/cli/user.rs` changes - [x] Review `src/server/authorize.rs` changes - [x] Review database files added to repo - [x] Post final review --- ### Summary This PR contains compilation fixes and a meaningful security improvement, but also includes **two files that must not be committed**. --- ### 🚨 Critical: Database WAL files committed `anz.db-shm` and `anz.db-wal` are SQLite [Write-Ahead Logging](https://www.sqlite.org/wal.html) auxiliary files — transient shared memory and journal files generated at runtime. They should never be in source control: - They may contain sensitive data (user records, session tokens, password hashes) - They are environment-specific and will cause corruption if shared across machines - They change on every database write These need to be removed from the PR and added to `.gitignore`: ``` # SQLite runtime files *.db-shm *.db-wal *.db ``` Also note: `.gitignore` line 20 contains a stray `EOF < /dev/null` — a heredoc artifact that should be removed. [Fix this →](https://claude.ai/code?q=Remove%20the%20stray%20%60EOF%20%3C%20%2Fdev%2Fnull%60%20line%20from%20.gitignore%20in%20the%20navicore%2Fanz%20repository%20on%20branch%20init%2C%20and%20add%20SQLite%20runtime%20file%20patterns%20%28*.db-shm%2C%20*.db-wal%2C%20*.db%29%20to%20.gitignore%2C%20then%20remove%20anz.db-shm%20and%20anz.db-wal%20from%20the%20repository) --- ### `src/cli/user.rs` — Potential UX regression **Lines 52–58:** The change splits `rpassword::prompt_password("Password: ")` into `eprint!("Password: ")` + `rpassword::read_password()`. ```rust // Before (explicit, atomic) let password = rpassword::prompt_password("Password: ")?; // After (split into two calls) eprint!("Password: "); let password = rpassword::read_password()?; ``` `eprint!` does **not** guarantee a stderr flush. On some systems, the prompt could be buffered and displayed _after_ the terminal is already in no-echo mode, leaving the user staring at a blank prompt with no indication they should type. `rpassword::prompt_password` handles this internally by flushing before disabling echo. Unless there's a specific reason for this split (e.g. test harness interference), reverting to `prompt_password` is safer. If the split is intentional, add an explicit flush: [Fix this →](https://claude.ai/code?q=In%20src%2Fcli%2Fuser.rs%20on%20branch%20init%20in%20navicore%2Fanz%2C%20add%20%60use%20std%3A%3Aio%3A%3AWrite%3B%60%20and%20flush%20stderr%20after%20each%20eprint!%20call%20with%20%60let%20_%20%3D%20std%3A%3Aio%3A%3Astderr%28%29.flush%28%29%3B%60%20before%20calling%20rpassword%3A%3Aread_password%28%29%20at%20lines%2052-58) ```rust eprint!("Password: "); let _ = std::io::stderr().flush(); let password = rpassword::read_password()?; ``` --- ### `src/server/authorize.rs` — Mostly positive **`.as_slice()` additions (lines 97, 203, 269):** These are correct API compatibility fixes. `Sha256::digest()` returns a `GenericArray`, and `.as_slice()` gives the expected `&[u8]`. No concerns. **Explicit `askama::Error` annotations (lines 134, 322):** These help the type inferencer and improve clarity. Fine. **Redirect URL construction (lines 287–293):** This is a genuine **security improvement**. ```rust // Before — manual string formatting let redirect_url = format!( "{}?code={}&state={}", q.redirect_uri, urlencoding::encode(&raw_code), urlencoding::encode(state_param), ); // After — structured URL building let mut redirect_parsed = url::Url::parse(&q.redirect_uri) .map_err(|e| AppError::Internal(format!("invalid redirect_uri: {e}")))?; redirect_parsed .query_pairs_mut() .append_pair("code", &raw_code) .append_pair("state", state_param); ``` The new approach: validates `redirect_uri` is a well-formed URL, correctly handles pre-existing query parameters, and delegates encoding to the `url` crate. Good change. One minor note: this code path is reached only after `redirect_uri` has already passed the registered URI allowlist check (line 88/180), so the `map_err(...Internal...)` error should be unreachable in practice. Consider whether it should be a `debug_assert!` or just left as-is for defense in depth. --- ### Verdict | | Status | |---|---| | `anz.db-shm` / `anz.db-wal` committed | ❌ Must fix before merge | | `.gitignore` has `EOF < /dev/null` artifact | ⚠️ Minor cleanup | | Password prompt stderr flush | ⚠️ Potential UX issue | | `.as_slice()` fixes | ✅ Correct | | `askama::Error` annotations | ✅ Fine | | URL-based redirect construction | ✅ Security improvement | **Do not merge** until the SQLite WAL files are removed from the commit history and `.gitignore` is updated to prevent re-adding them.