bugfix: third-pass audit follow-ups (F1-F4 + dockerignore)
- F1: backend/Dockerfile now copies Cargo.lock alongside Cargo.toml
and builds with --locked, so the production image runs against the
exact crate versions CI tested. Without this, cargo silently
resolved fresh on each image build and "we tested it" stopped being
true for the binary you ship.
- F2: POST /api/v1/mangas/{id}/chapters rejects chapter `number < 1`
with 422 validation_failed. Mirrors the bookmark page>=1 rule from
0.9.4 — chapter numbers are 1-indexed everywhere (URLs, upload
form, reader) and 0/negative numbers had no legitimate use. Three
cases (0, -1, -100) in api_uploads.rs.
- F3: bookmarks/+page.ts no longer re-throws non-401 ApiErrors as
SvelteKit's generic 500 page. Surfaces the error message inline via
a new `data.error` field; the page renders an alert when present.
Same UX shape as the home page's existing error handling.
- F4: dropped Space from the reader keyboard binding. On portrait
phones and narrow desktop windows the page image overflows the
viewport and the user expects Space to scroll — preventDefaulting
it skipped past unread content. ArrowRight + j remain.
- New backend/.dockerignore and frontend/.dockerignore so the local
target/ and node_modules/ don't get shipped into the build context
on every `docker compose build`.
Lockstep version bump to 0.10.2.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
10
backend/.dockerignore
Normal file
10
backend/.dockerignore
Normal file
@@ -0,0 +1,10 @@
|
|||||||
|
# Shrink the build context the daemon ships to Docker — without this,
|
||||||
|
# the multi-GB `target/` from a local `cargo build` would be uploaded
|
||||||
|
# on every `docker compose build`.
|
||||||
|
|
||||||
|
target/
|
||||||
|
.sqlx/
|
||||||
|
tests/
|
||||||
|
*.md
|
||||||
|
.env
|
||||||
|
.env.*
|
||||||
2
backend/Cargo.lock
generated
2
backend/Cargo.lock
generated
@@ -1033,7 +1033,7 @@ checksum = "5e5032e24019045c762d3c0f28f5b6b8bbf38563a65908389bf7978758920897"
|
|||||||
|
|
||||||
[[package]]
|
[[package]]
|
||||||
name = "mangalord"
|
name = "mangalord"
|
||||||
version = "0.10.1"
|
version = "0.10.2"
|
||||||
dependencies = [
|
dependencies = [
|
||||||
"anyhow",
|
"anyhow",
|
||||||
"argon2",
|
"argon2",
|
||||||
|
|||||||
@@ -1,6 +1,6 @@
|
|||||||
[package]
|
[package]
|
||||||
name = "mangalord"
|
name = "mangalord"
|
||||||
version = "0.10.1"
|
version = "0.10.2"
|
||||||
edition = "2021"
|
edition = "2021"
|
||||||
|
|
||||||
[lib]
|
[lib]
|
||||||
|
|||||||
@@ -5,15 +5,18 @@ RUN apt-get update \
|
|||||||
&& apt-get install -y --no-install-recommends pkg-config libssl-dev ca-certificates \
|
&& apt-get install -y --no-install-recommends pkg-config libssl-dev ca-certificates \
|
||||||
&& rm -rf /var/lib/apt/lists/*
|
&& rm -rf /var/lib/apt/lists/*
|
||||||
|
|
||||||
# Cache deps separately from sources.
|
# Cache deps separately from sources. `--locked` makes cargo refuse to
|
||||||
COPY Cargo.toml ./
|
# update the lockfile, so the production image is built against the
|
||||||
|
# exact crate versions CI tested. Without Cargo.lock + the flag, cargo
|
||||||
|
# would silently resolve fresh on every image build.
|
||||||
|
COPY Cargo.toml Cargo.lock ./
|
||||||
RUN mkdir src && echo "fn main() {}" > src/main.rs && echo "" > src/lib.rs \
|
RUN mkdir src && echo "fn main() {}" > src/main.rs && echo "" > src/lib.rs \
|
||||||
&& cargo build --release \
|
&& cargo build --locked --release \
|
||||||
&& rm -rf src
|
&& rm -rf src
|
||||||
|
|
||||||
COPY src ./src
|
COPY src ./src
|
||||||
COPY migrations ./migrations
|
COPY migrations ./migrations
|
||||||
RUN touch src/main.rs src/lib.rs && cargo build --release
|
RUN touch src/main.rs src/lib.rs && cargo build --locked --release
|
||||||
|
|
||||||
FROM debian:bookworm-slim
|
FROM debian:bookworm-slim
|
||||||
RUN apt-get update \
|
RUN apt-get update \
|
||||||
|
|||||||
@@ -105,6 +105,15 @@ async fn create(
|
|||||||
message: "metadata part is required".into(),
|
message: "metadata part is required".into(),
|
||||||
details: json!({ "metadata": "required" }),
|
details: json!({ "metadata": "required" }),
|
||||||
})?;
|
})?;
|
||||||
|
// Chapter number is 1-indexed everywhere (URLs, upload form,
|
||||||
|
// reader). Reject 0 / negative numbers up front so the row never
|
||||||
|
// makes it into the DB. Mirrors the page>=1 rule on bookmarks.
|
||||||
|
if metadata.number < 1 {
|
||||||
|
return Err(AppError::ValidationFailed {
|
||||||
|
message: "chapter number must be 1 or greater".into(),
|
||||||
|
details: json!({ "number": "must be >= 1" }),
|
||||||
|
});
|
||||||
|
}
|
||||||
if pages.is_empty() {
|
if pages.is_empty() {
|
||||||
return Err(AppError::ValidationFailed {
|
return Err(AppError::ValidationFailed {
|
||||||
message: "at least one page is required".into(),
|
message: "at least one page is required".into(),
|
||||||
|
|||||||
@@ -242,6 +242,36 @@ async fn create_chapter_with_pages_stores_each(pool: PgPool) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[sqlx::test(migrations = "./migrations")]
|
||||||
|
async fn create_chapter_rejects_non_positive_number_with_422(pool: PgPool) {
|
||||||
|
let h = common::harness(pool);
|
||||||
|
let (_, cookie) = common::register_user(&h.app).await;
|
||||||
|
let manga_id = common::seed_manga_via_api(&h.app, &cookie, "Berserk").await;
|
||||||
|
|
||||||
|
for bad_number in [0i32, -1, -100] {
|
||||||
|
let resp = h
|
||||||
|
.app
|
||||||
|
.clone()
|
||||||
|
.oneshot(common::post_multipart_with_cookie(
|
||||||
|
&format!("/api/v1/mangas/{manga_id}/chapters"),
|
||||||
|
MultipartBuilder::new()
|
||||||
|
.add_json("metadata", json!({ "number": bad_number }))
|
||||||
|
.add_file("page", "1.png", "image/png", &common::fake_png_bytes()),
|
||||||
|
&cookie,
|
||||||
|
))
|
||||||
|
.await
|
||||||
|
.unwrap();
|
||||||
|
assert_eq!(
|
||||||
|
resp.status(),
|
||||||
|
StatusCode::UNPROCESSABLE_ENTITY,
|
||||||
|
"number={bad_number} should be rejected"
|
||||||
|
);
|
||||||
|
let body = common::body_json(resp).await;
|
||||||
|
assert_eq!(body["error"]["code"], "validation_failed");
|
||||||
|
assert!(body["error"]["details"]["number"].is_string());
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
#[sqlx::test(migrations = "./migrations")]
|
#[sqlx::test(migrations = "./migrations")]
|
||||||
async fn create_chapter_rejects_when_no_pages_with_422(pool: PgPool) {
|
async fn create_chapter_rejects_when_no_pages_with_422(pool: PgPool) {
|
||||||
let h = common::harness(pool);
|
let h = common::harness(pool);
|
||||||
|
|||||||
13
frontend/.dockerignore
Normal file
13
frontend/.dockerignore
Normal file
@@ -0,0 +1,13 @@
|
|||||||
|
# Shrink the build context the daemon ships to Docker. node_modules in
|
||||||
|
# particular would otherwise be re-uploaded on every build despite the
|
||||||
|
# Dockerfile re-installing it from scratch inside the builder.
|
||||||
|
|
||||||
|
node_modules/
|
||||||
|
build/
|
||||||
|
.svelte-kit/
|
||||||
|
e2e/
|
||||||
|
test-results/
|
||||||
|
playwright-report/
|
||||||
|
*.md
|
||||||
|
.env
|
||||||
|
.env.*
|
||||||
@@ -1,6 +1,6 @@
|
|||||||
{
|
{
|
||||||
"name": "mangalord-frontend",
|
"name": "mangalord-frontend",
|
||||||
"version": "0.10.1",
|
"version": "0.10.2",
|
||||||
"private": true,
|
"private": true,
|
||||||
"type": "module",
|
"type": "module",
|
||||||
"scripts": {
|
"scripts": {
|
||||||
|
|||||||
@@ -2,6 +2,7 @@
|
|||||||
let { data } = $props();
|
let { data } = $props();
|
||||||
const authenticated = $derived(data.authenticated);
|
const authenticated = $derived(data.authenticated);
|
||||||
const bookmarks = $derived(data.bookmarks);
|
const bookmarks = $derived(data.bookmarks);
|
||||||
|
const error = $derived(data.error);
|
||||||
</script>
|
</script>
|
||||||
|
|
||||||
<svelte:head>
|
<svelte:head>
|
||||||
@@ -10,7 +11,11 @@
|
|||||||
|
|
||||||
<h1>Bookmarks</h1>
|
<h1>Bookmarks</h1>
|
||||||
|
|
||||||
{#if !authenticated}
|
{#if error}
|
||||||
|
<p role="alert" data-testid="bookmarks-error">
|
||||||
|
Couldn't load bookmarks: {error}
|
||||||
|
</p>
|
||||||
|
{:else if !authenticated}
|
||||||
<p data-testid="bookmarks-signin">
|
<p data-testid="bookmarks-signin">
|
||||||
<a href="/login">Sign in</a> to see your bookmarks.
|
<a href="/login">Sign in</a> to see your bookmarks.
|
||||||
</p>
|
</p>
|
||||||
|
|||||||
@@ -7,10 +7,18 @@ export const ssr = false;
|
|||||||
export const load: PageLoad = async () => {
|
export const load: PageLoad = async () => {
|
||||||
try {
|
try {
|
||||||
const page = await listMyBookmarks();
|
const page = await listMyBookmarks();
|
||||||
return { bookmarks: page.items, authenticated: true };
|
return { bookmarks: page.items, authenticated: true, error: null };
|
||||||
} catch (e) {
|
} catch (e) {
|
||||||
if (e instanceof ApiError && e.status === 401) {
|
if (e instanceof ApiError && e.status === 401) {
|
||||||
return { bookmarks: [], authenticated: false };
|
return { bookmarks: [], authenticated: false, error: null };
|
||||||
|
}
|
||||||
|
// Anything else (502 upstream_unavailable from a backend
|
||||||
|
// restart, 500 internal_error) is rendered inline rather than
|
||||||
|
// re-thrown — SvelteKit's generic error.html is not the right
|
||||||
|
// UX for a transient API blip and the user is already
|
||||||
|
// authenticated as far as we know.
|
||||||
|
if (e instanceof ApiError) {
|
||||||
|
return { bookmarks: [], authenticated: true, error: e.message };
|
||||||
}
|
}
|
||||||
throw e;
|
throw e;
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -32,10 +32,14 @@
|
|||||||
// Don't hijack keys while the user is typing in an input.
|
// Don't hijack keys while the user is typing in an input.
|
||||||
const target = e.target as HTMLElement | null;
|
const target = e.target as HTMLElement | null;
|
||||||
if (target && (target.tagName === 'INPUT' || target.tagName === 'TEXTAREA')) return;
|
if (target && (target.tagName === 'INPUT' || target.tagName === 'TEXTAREA')) return;
|
||||||
|
// Space is deliberately *not* bound — on viewports where a page
|
||||||
|
// image overflows (portrait phones, narrow desktop windows),
|
||||||
|
// users expect Space to scroll the page, and stealing it for
|
||||||
|
// next-page would skip past unread content. ArrowRight + j
|
||||||
|
// are the discoverable bindings.
|
||||||
switch (e.key) {
|
switch (e.key) {
|
||||||
case 'ArrowRight':
|
case 'ArrowRight':
|
||||||
case 'j':
|
case 'j':
|
||||||
case ' ':
|
|
||||||
e.preventDefault();
|
e.preventDefault();
|
||||||
next();
|
next();
|
||||||
break;
|
break;
|
||||||
|
|||||||
Reference in New Issue
Block a user