Skip to content

Commit 2dbf21c

Browse files
committed
fix(api): adressiere Copilot+Gemini Review-Findings auf PR #92
1. batch.py: check_printer_access(auth, printer.id) nach Resolve. Pattern aus printers.py (Z.243, 392, 418). Schliesst ACL-Bypass: api-key mit allowed_printer_ids=[A] kann jetzt nicht POST /api/print/B/batch. (Copilot + Gemini: security-critical) 2. batch.py: validiere printer.id == app.state.printer_id. Hub ist single-printer at startup (main.py:329 wired PrintService auf einen UUID). Ohne Check wuerde unsere Route silently die falsche Hardware ansprechen. Bei Mismatch: 404 printer_not_active. Future-proof fuer Multi-Printer durch klare Error-Message. (Copilot: correctness-critical) 3. test_job_state_fragment.py: Jinja2 autoescape=True. CodeQL py/jinja2-autoescape. Kein funktionaler Unterschied fuer UUID/String-Substitutionen. (CodeQL) 4. test_batch_endpoint_auth.py: nutzlose Variablen entfernt. RUF059 unused unpacks weil test sowieso skipped. Lokal: 831 passed, 6 skipped, ruff/format/mypy alle grün. Refs strausmann/hangar#78
1 parent 84232ce commit 2dbf21c

7 files changed

Lines changed: 37 additions & 24 deletions

backend/app/api/routes/batch.py

Lines changed: 26 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
from fastapi import APIRouter, Depends, HTTPException, Path, Request, status
99
from sqlalchemy.ext.asyncio import AsyncSession
1010

11-
from app.auth.dependencies import AuthContext
11+
from app.auth.dependencies import AuthContext, check_printer_access
1212
from app.auth.scope_deps import require_print
1313
from app.db.session import get_session
1414
from app.models.print_batch import PrintBatch
@@ -55,12 +55,34 @@ async def create_batch(
5555
session: SessionDep,
5656
auth: Annotated[AuthContext, Depends(require_print)],
5757
) -> BatchResponse:
58-
# 1. Resolve printer
58+
# 1. Resolve printer (404 if unknown slug/uuid)
5959
printer = await printers_repo.resolve_by_slug_or_uuid(session, printer_key)
6060
if printer is None:
6161
raise HTTPException(404, detail={"error_code": "printer_not_found"})
6262

63-
# 2. Best-effort dispatch
63+
# 2. ACL: api-key may be restricted to a subset of printer_ids
64+
check_printer_access(auth, printer.id)
65+
66+
# 3. Verify the resolved printer matches the singleton wired into
67+
# app.state.print_service. The Hub is currently single-printer at
68+
# startup (main.py wires PrintService to app.state.printer_id).
69+
# If the URL slug points to a different printer row, the dispatch
70+
# would silently route to the wrong device. Reject explicitly.
71+
seeded_printer_id = getattr(http.app.state, "printer_id", None)
72+
if seeded_printer_id is not None and printer.id != seeded_printer_id:
73+
raise HTTPException(
74+
404,
75+
detail={
76+
"error_code": "printer_not_active",
77+
"error_message": (
78+
"Resolved printer is not the currently-seeded device. "
79+
"Hub is single-printer at startup; multi-printer routing "
80+
"is a future enhancement."
81+
),
82+
},
83+
)
84+
85+
# 4. Best-effort dispatch
6486
service = http.app.state.print_service
6587
try:
6688
job_ids, errors = await dispatch_batch(service, body.items)
@@ -73,7 +95,7 @@ async def create_batch(
7395
},
7496
) from exc
7597

76-
# 3. Persist tracking row
98+
# 5. Persist tracking row
7799
# auth.subject_id does NOT exist — use api_key_id or source
78100
created_by = str(auth.api_key_id) if auth.api_key_id else auth.source
79101
batch_row = PrintBatch(

backend/tests/integration/test_batch_endpoint_auth.py

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -73,10 +73,6 @@ async def auth_db_session():
7373
@pytest.mark.asyncio
7474
async def test_batch_requires_auth(auth_client, auth_db_session):
7575
"""Genuine 401 requires unauthenticated client; covered by Phase 7c auth tests."""
76-
client, inner_app = auth_client
77-
p = Printer(name="X", slug="x", model="X", backend="mock")
78-
await printers_repo.create(auth_db_session, p)
79-
8076
pytest.skip("401 requires unauthenticated client; covered by Phase 7c auth tests")
8177

8278

backend/tests/integration/test_batch_endpoint_happy.py

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -87,9 +87,7 @@ async def test_batch_happy_path(batch_client, batch_db_session, batch_auth_heade
8787
for i in range(3)
8888
]
8989
}
90-
resp = await client.post(
91-
f"/api/print/{p.slug}/batch", json=body, headers=batch_auth_headers
92-
)
90+
resp = await client.post(f"/api/print/{p.slug}/batch", json=body, headers=batch_auth_headers)
9391
assert resp.status_code == 202, resp.text
9492
data = resp.json()
9593
assert "batch_id" in data

backend/tests/integration/test_batch_endpoint_partial_failure.py

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -60,9 +60,7 @@ def partial_auth_headers() -> dict:
6060

6161

6262
@pytest.mark.asyncio
63-
async def test_batch_partial_failure(
64-
partial_client, partial_db_session, partial_auth_headers
65-
):
63+
async def test_batch_partial_failure(partial_client, partial_db_session, partial_auth_headers):
6664
client, inner_app = partial_client
6765
p = Printer(name="Brother PT-P750W", slug="brother-p750w", model="PT-P750W", backend="mock")
6866
await printers_repo.create(partial_db_session, p)
@@ -86,9 +84,7 @@ async def test_batch_partial_failure(
8684
},
8785
]
8886
}
89-
resp = await client.post(
90-
f"/api/print/{p.slug}/batch", json=body, headers=partial_auth_headers
91-
)
87+
resp = await client.post(f"/api/print/{p.slug}/batch", json=body, headers=partial_auth_headers)
9288
assert resp.status_code == 202, resp.text
9389
data = resp.json()
9490
assert len(data["job_ids"]) == 2

backend/tests/integration/test_batch_endpoint_printer_offline.py

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -87,8 +87,6 @@ async def _raise(self, req):
8787
}
8888
]
8989
}
90-
resp = await client.post(
91-
f"/api/print/{p.slug}/batch", json=body, headers=offline_auth_headers
92-
)
90+
resp = await client.post(f"/api/print/{p.slug}/batch", json=body, headers=offline_auth_headers)
9391
assert resp.status_code == 409, resp.text
9492
assert resp.json()["detail"]["error_code"] == "printer_offline"

backend/tests/integration/test_batch_endpoint_tape_mismatch.py

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -101,9 +101,7 @@ async def _maybe_raise(self, req):
101101
},
102102
]
103103
}
104-
resp = await client.post(
105-
f"/api/print/{p.slug}/batch", json=body, headers=tape_auth_headers
106-
)
104+
resp = await client.post(f"/api/print/{p.slug}/batch", json=body, headers=tape_auth_headers)
107105
assert resp.status_code == 202, resp.text
108106
data = resp.json()
109107
assert len(data["job_ids"]) == 2

backend/tests/unit/test_job_state_fragment.py

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,12 @@
1313
def jinja_env():
1414
# Hub-Templates liegen unter backend/app/templates/
1515
template_dir = Path(__file__).parent.parent.parent / "app" / "templates"
16-
return Environment(loader=FileSystemLoader(str(template_dir)))
16+
# autoescape=True via select_autoescape default — no functional change
17+
# for our UUID/string substitutions, satisfies CodeQL py/jinja2-autoescape.
18+
return Environment(
19+
loader=FileSystemLoader(str(template_dir)),
20+
autoescape=True,
21+
)
1722

1823

1924
def test_job_state_fragment_has_data_job_id_and_state(jinja_env):

0 commit comments

Comments
 (0)