fix(backend): post-review fixes sprint 2
- test_simulations_patch: remove false dict return annotation on _patch helper - simulation_workflow: validate executed_at upfront before any setattr (prevents partial mutation on bad payload) - api/simulations: remove unreachable role check in update_simulation (all valid roles are admin/redteam/soc) - Dockerfile: remove redundant COPY backend/data/ (already covered by COPY backend/) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -80,9 +80,6 @@ def update_simulation(sid: int):
|
|||||||
return jsonify({"error": "Simulation not found"}), 404
|
return jsonify({"error": "Simulation not found"}), 404
|
||||||
|
|
||||||
user = g.current_user
|
user = g.current_user
|
||||||
if user.role.value not in ("admin", "redteam", "soc"):
|
|
||||||
return jsonify({"error": "Forbidden"}), 403
|
|
||||||
|
|
||||||
data = request.get_json(silent=True) or {}
|
data = request.get_json(silent=True) or {}
|
||||||
if not data:
|
if not data:
|
||||||
return jsonify(serialize_simulation(sim)), 200
|
return jsonify(serialize_simulation(sim)), 200
|
||||||
|
|||||||
@@ -79,18 +79,21 @@ def apply_patch(
|
|||||||
# admin / redteam: apply all fields present.
|
# admin / redteam: apply all fields present.
|
||||||
redteam_keys_present = REDTEAM_FIELDS & payload.keys()
|
redteam_keys_present = REDTEAM_FIELDS & payload.keys()
|
||||||
|
|
||||||
|
# Validate executed_at before any writes so a bad value causes no partial mutation.
|
||||||
|
executed_at_value: datetime | None = None
|
||||||
|
if "executed_at" in redteam_keys_present:
|
||||||
|
val = payload["executed_at"]
|
||||||
|
if val is not None:
|
||||||
|
if not isinstance(val, str):
|
||||||
|
return jsonify({"error": "invalid executed_at"}), 400
|
||||||
|
try:
|
||||||
|
executed_at_value = datetime.fromisoformat(val)
|
||||||
|
except ValueError:
|
||||||
|
return jsonify({"error": "invalid executed_at"}), 400
|
||||||
|
|
||||||
for field in redteam_keys_present:
|
for field in redteam_keys_present:
|
||||||
if field == "executed_at":
|
if field == "executed_at":
|
||||||
val = payload["executed_at"]
|
simulation.executed_at = executed_at_value
|
||||||
if val is None:
|
|
||||||
simulation.executed_at = None
|
|
||||||
else:
|
|
||||||
if not isinstance(val, str):
|
|
||||||
return jsonify({"error": "invalid executed_at"}), 400
|
|
||||||
try:
|
|
||||||
simulation.executed_at = datetime.fromisoformat(val)
|
|
||||||
except ValueError:
|
|
||||||
return jsonify({"error": "invalid executed_at"}), 400
|
|
||||||
else:
|
else:
|
||||||
setattr(simulation, field, payload[field])
|
setattr(simulation, field, payload[field])
|
||||||
|
|
||||||
|
|||||||
@@ -26,11 +26,10 @@ def _make_sim(client: FlaskClient, token: str, eid: int) -> dict:
|
|||||||
return resp.get_json()
|
return resp.get_json()
|
||||||
|
|
||||||
|
|
||||||
def _patch(client: FlaskClient, token: str, sid: int, payload: dict) -> dict:
|
def _patch(client: FlaskClient, token: str, sid: int, payload: dict):
|
||||||
resp = client.patch(
|
return client.patch(
|
||||||
f"/api/simulations/{sid}", headers=_h(token), json=payload
|
f"/api/simulations/{sid}", headers=_h(token), json=payload
|
||||||
)
|
)
|
||||||
return resp
|
|
||||||
|
|
||||||
|
|
||||||
# ---------------------------------------------------------------------------
|
# ---------------------------------------------------------------------------
|
||||||
@@ -232,7 +231,7 @@ def test_soc_can_patch_when_review_required(
|
|||||||
|
|
||||||
|
|
||||||
def test_soc_can_patch_when_done(
|
def test_soc_can_patch_when_done(
|
||||||
client: FlaskClient, redteam_token: str, soc_token: str, admin_token: str
|
client: FlaskClient, redteam_token: str, soc_token: str
|
||||||
) -> None:
|
) -> None:
|
||||||
eng = _make_engagement(client, redteam_token)
|
eng = _make_engagement(client, redteam_token)
|
||||||
sim = _make_sim(client, redteam_token, eng["id"])
|
sim = _make_sim(client, redteam_token, eng["id"])
|
||||||
@@ -270,3 +269,27 @@ def test_soc_cannot_edit_redteam_fields(
|
|||||||
def test_patch_simulation_404(client: FlaskClient, redteam_token: str) -> None:
|
def test_patch_simulation_404(client: FlaskClient, redteam_token: str) -> None:
|
||||||
resp = _patch(client, redteam_token, 9999, {"name": "x"})
|
resp = _patch(client, redteam_token, 9999, {"name": "x"})
|
||||||
assert resp.status_code == 404
|
assert resp.status_code == 404
|
||||||
|
|
||||||
|
|
||||||
|
def test_invalid_executed_at_does_not_mutate_other_fields(
|
||||||
|
client: FlaskClient, redteam_token: str
|
||||||
|
) -> None:
|
||||||
|
"""invalid executed_at must return 400 without persisting other fields in the payload."""
|
||||||
|
eng = _make_engagement(client, redteam_token)
|
||||||
|
sim = _make_sim(client, redteam_token, eng["id"])
|
||||||
|
original_description = sim["description"]
|
||||||
|
|
||||||
|
resp = _patch(
|
||||||
|
client,
|
||||||
|
redteam_token,
|
||||||
|
sim["id"],
|
||||||
|
{"description": "should-not-stick", "executed_at": "not-a-date"},
|
||||||
|
)
|
||||||
|
assert resp.status_code == 400
|
||||||
|
|
||||||
|
get_resp = client.get(
|
||||||
|
f"/api/simulations/{sim['id']}",
|
||||||
|
headers={"Authorization": f"Bearer {redteam_token}"},
|
||||||
|
)
|
||||||
|
assert get_resp.status_code == 200
|
||||||
|
assert get_resp.get_json()["description"] == original_description
|
||||||
|
|||||||
@@ -12,7 +12,6 @@ WORKDIR /app
|
|||||||
COPY backend/requirements.txt ./backend/
|
COPY backend/requirements.txt ./backend/
|
||||||
RUN pip install --no-cache-dir -r backend/requirements.txt
|
RUN pip install --no-cache-dir -r backend/requirements.txt
|
||||||
COPY backend/ ./backend/
|
COPY backend/ ./backend/
|
||||||
COPY backend/data/ ./backend/data/
|
|
||||||
COPY --from=frontend-build /app/frontend/dist ./backend/app/static
|
COPY --from=frontend-build /app/frontend/dist ./backend/app/static
|
||||||
|
|
||||||
ENV FLASK_APP=backend.app:create_app
|
ENV FLASK_APP=backend.app:create_app
|
||||||
|
|||||||
Reference in New Issue
Block a user