feat: authorize and audit comment deletion (Closes #779)
This commit is contained in:
1 parent
9d14fa5ffb
commit
3896f17a58
5 files changed
+285
-6
No files matched your search
@@ -777,6 +777,12 @@ def test_security_activity_explains_pending_outcome_confirmation():
|
||||
assert "event.status === 'pending'" in source
|
||||
|
||||
|
||||
def test_security_activity_labels_permanent_comment_deletion():
|
||||
source = SECURITY_CENTER_JS.read_text()
|
||||
|
||||
assert "comment_deleted: 'Comment deleted'" in source
|
||||
|
||||
|
||||
def test_security_activity_labels_passkey_enrollment_without_html_rendering():
|
||||
source = SECURITY_CENTER_JS.read_text()
|
||||
|
||||
|
||||
@@ -401,6 +401,217 @@ async def test_successful_protected_actions_record_only_bounded_targets(
|
||||
assert b"abc123" not in persisted
|
||||
|
||||
|
||||
@pytest.mark.anyio
|
||||
@pytest.mark.parametrize(
|
||||
"path",
|
||||
[
|
||||
"/api/v1/repos/stackchain/api/issues/7/comments/42",
|
||||
"/api/v1/repos/stackchain/api/pulls/7/comments/42",
|
||||
"/api/v1/notifications/91/comments/42",
|
||||
],
|
||||
)
|
||||
async def test_comment_deletion_requires_exact_fresh_authorization_before_gitea(
|
||||
security_access, monkeypatch, path
|
||||
):
|
||||
calls = []
|
||||
|
||||
async def assigned(*_args):
|
||||
return True
|
||||
|
||||
async def notification_target(thread_id):
|
||||
assert thread_id == 91
|
||||
return "stackchain/api", 7
|
||||
|
||||
async def delete(*args):
|
||||
calls.append(args)
|
||||
return {"id": 42, "deleted": True}
|
||||
|
||||
monkeypatch.setattr(main.gitea_proxy, "is_assigned_issue", assigned)
|
||||
monkeypatch.setattr(main.gitea_proxy, "is_assigned_pull", assigned)
|
||||
monkeypatch.setattr(main.gitea_proxy, "notification_conversation_target", notification_target)
|
||||
monkeypatch.setattr(main.gitea_proxy, "delete_owned_comment", delete)
|
||||
transport = httpx.ASGITransport(app=main.app)
|
||||
async with httpx.AsyncClient(transport=transport, base_url="https://test") as client:
|
||||
await client.post(
|
||||
"/api/v1/session",
|
||||
json={"access_token": "correct horse battery staple", "device_label": "Phone"},
|
||||
)
|
||||
headers = {
|
||||
"Origin": "https://test",
|
||||
"X-CSRF-Token": client.cookies["stackchain_csrf"],
|
||||
}
|
||||
response = await client.delete(path, headers=headers)
|
||||
|
||||
assert response.status_code == 428
|
||||
assert response.json()["detail"] == {
|
||||
"detail": "Fresh authorization required",
|
||||
"code": "step_up_required",
|
||||
"action": "delete_comment",
|
||||
"target": "stackchain/api#7:42",
|
||||
}
|
||||
assert calls == []
|
||||
|
||||
|
||||
@pytest.mark.anyio
|
||||
async def test_authorized_comment_deletion_records_only_privacy_safe_target(
|
||||
security_access, monkeypatch
|
||||
):
|
||||
async def assigned(*_args):
|
||||
return True
|
||||
|
||||
async def delete(*_args):
|
||||
return {"id": 42, "deleted": True}
|
||||
|
||||
monkeypatch.setattr(main.gitea_proxy, "is_assigned_issue", assigned)
|
||||
monkeypatch.setattr(main.gitea_proxy, "delete_owned_comment", delete)
|
||||
transport = httpx.ASGITransport(app=main.app)
|
||||
async with httpx.AsyncClient(transport=transport, base_url="https://test") as client:
|
||||
await client.post(
|
||||
"/api/v1/session",
|
||||
json={"access_token": "correct horse battery staple", "device_label": "Phone"},
|
||||
)
|
||||
headers = {
|
||||
"Origin": "https://test",
|
||||
"X-CSRF-Token": client.cookies["stackchain_csrf"],
|
||||
}
|
||||
grant = await client.post(
|
||||
"/api/v1/fresh-authorization",
|
||||
json={
|
||||
"access_token": "correct horse battery staple",
|
||||
"action": "delete_comment",
|
||||
"target": "stackchain/api#7:42",
|
||||
},
|
||||
headers=headers,
|
||||
)
|
||||
response = await client.delete(
|
||||
"/api/v1/repos/stackchain/api/issues/7/comments/42",
|
||||
headers={**headers, "X-Step-Up-Grant": grant.json()["grant"]},
|
||||
)
|
||||
events = (await client.get("/api/v1/security-events")).json()["events"]
|
||||
|
||||
assert response.status_code == 200
|
||||
assert (events[0]["kind"], events[0]["target"], events[0]["status"]) == (
|
||||
"comment_deleted", "stackchain/api#7:42", "completed"
|
||||
)
|
||||
persisted = (security_access / "security.sqlite3").read_bytes()
|
||||
assert b"private comment body" not in persisted
|
||||
assert b"correct horse battery staple" not in persisted
|
||||
|
||||
|
||||
@pytest.mark.anyio
|
||||
async def test_comment_deletion_reservation_failure_prevents_gitea_mutation(
|
||||
security_access, monkeypatch
|
||||
):
|
||||
calls = []
|
||||
|
||||
async def assigned(*_args):
|
||||
return True
|
||||
|
||||
async def delete(*args):
|
||||
calls.append(args)
|
||||
return {"id": 42, "deleted": True}
|
||||
|
||||
class UnavailableJournal:
|
||||
def reserve(self, *_args, **_kwargs):
|
||||
raise SecurityEventStoreError("unavailable")
|
||||
|
||||
monkeypatch.setattr(main.gitea_proxy, "is_assigned_issue", assigned)
|
||||
monkeypatch.setattr(main.gitea_proxy, "delete_owned_comment", delete)
|
||||
transport = httpx.ASGITransport(app=main.app)
|
||||
async with httpx.AsyncClient(transport=transport, base_url="https://test") as client:
|
||||
await client.post(
|
||||
"/api/v1/session",
|
||||
json={"access_token": "correct horse battery staple", "device_label": "Phone"},
|
||||
)
|
||||
headers = {
|
||||
"Origin": "https://test",
|
||||
"X-CSRF-Token": client.cookies["stackchain_csrf"],
|
||||
}
|
||||
grant = await client.post(
|
||||
"/api/v1/fresh-authorization",
|
||||
json={
|
||||
"access_token": "correct horse battery staple",
|
||||
"action": "delete_comment",
|
||||
"target": "stackchain/api#7:42",
|
||||
},
|
||||
headers=headers,
|
||||
)
|
||||
monkeypatch.setattr(main, "_security_event_store", lambda: UnavailableJournal())
|
||||
response = await client.delete(
|
||||
"/api/v1/repos/stackchain/api/issues/7/comments/42",
|
||||
headers={**headers, "X-Step-Up-Grant": grant.json()["grant"]},
|
||||
)
|
||||
|
||||
assert response.status_code == 503
|
||||
assert calls == []
|
||||
|
||||
|
||||
@pytest.mark.anyio
|
||||
@pytest.mark.parametrize("finalize_fails", [False, True])
|
||||
async def test_comment_deletion_journal_tracks_failed_and_confirmed_outcomes(
|
||||
security_access, monkeypatch, finalize_fails
|
||||
):
|
||||
journal_calls = []
|
||||
|
||||
async def assigned(*_args):
|
||||
return True
|
||||
|
||||
async def delete(*_args):
|
||||
if not finalize_fails:
|
||||
raise RuntimeError("upstream rejected deletion")
|
||||
return {"id": 42, "deleted": True}
|
||||
|
||||
class Journal:
|
||||
def reserve(self, kind, *, target):
|
||||
journal_calls.append(("reserve", kind, target))
|
||||
return "operation-42"
|
||||
|
||||
def discard(self, operation_id):
|
||||
journal_calls.append(("discard", operation_id))
|
||||
|
||||
def finalize(self, operation_id):
|
||||
journal_calls.append(("finalize", operation_id))
|
||||
raise SecurityEventStoreError("unavailable")
|
||||
|
||||
monkeypatch.setattr(main.gitea_proxy, "is_assigned_issue", assigned)
|
||||
monkeypatch.setattr(main.gitea_proxy, "delete_owned_comment", delete)
|
||||
transport = httpx.ASGITransport(app=main.app)
|
||||
async with httpx.AsyncClient(transport=transport, base_url="https://test") as client:
|
||||
await client.post(
|
||||
"/api/v1/session",
|
||||
json={"access_token": "correct horse battery staple", "device_label": "Phone"},
|
||||
)
|
||||
headers = {
|
||||
"Origin": "https://test",
|
||||
"X-CSRF-Token": client.cookies["stackchain_csrf"],
|
||||
}
|
||||
grant = await client.post(
|
||||
"/api/v1/fresh-authorization",
|
||||
json={
|
||||
"access_token": "correct horse battery staple",
|
||||
"action": "delete_comment",
|
||||
"target": "stackchain/api#7:42",
|
||||
},
|
||||
headers=headers,
|
||||
)
|
||||
monkeypatch.setattr(main, "_security_event_store", lambda: Journal())
|
||||
response = await client.delete(
|
||||
"/api/v1/repos/stackchain/api/issues/7/comments/42",
|
||||
headers={**headers, "X-Step-Up-Grant": grant.json()["grant"]},
|
||||
)
|
||||
|
||||
assert journal_calls[0] == (
|
||||
"reserve", "comment_deleted", "stackchain/api#7:42"
|
||||
)
|
||||
if finalize_fails:
|
||||
assert response.status_code == 200
|
||||
assert response.json() == {"id": 42, "deleted": True}
|
||||
assert journal_calls[1] == ("finalize", "operation-42")
|
||||
else:
|
||||
assert response.status_code == 503
|
||||
assert journal_calls[1] == ("discard", "operation-42")
|
||||
|
||||
|
||||
@pytest.mark.anyio
|
||||
async def test_approved_pull_review_records_a_privacy_safe_completed_event(
|
||||
security_access, monkeypatch
|
||||
|
||||
Reference in new issue
Block a user