From 8294c6531d43c2e0bc3e8253a95b2a1cb7fa06ab Mon Sep 17 00:00:00 2001 From: Manish Gupta Date: Fri, 28 Aug 2026 09:51:18 +0530 Subject: [PATCH] fix: address Copilot review on PR #9704 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three findings, all in the test file: - Module docstring referenced test_patch_then_delete_forgery_chain_is_blocked, which doesn't exist — the actual test is test_patch_then_delete_is_still_403_for_a_plain_member. - Issue-create and comment-create tests only asserted created_by wasn't forged, not created_at, even though the same removed override blocks covered both fields. Added a 10-years-back forged created_at to each POST body and asserted it lands nowhere near that value. Co-authored-by: Plane AI --- .../contract/api/test_created_by_forgery.py | 27 ++++++++++++++++--- 1 file changed, 23 insertions(+), 4 deletions(-) diff --git a/apps/api/plane/tests/contract/api/test_created_by_forgery.py b/apps/api/plane/tests/contract/api/test_created_by_forgery.py index 843197d89f..f3ab40fd1e 100644 --- a/apps/api/plane/tests/contract/api/test_created_by_forgery.py +++ b/apps/api/plane/tests/contract/api/test_created_by_forgery.py @@ -20,17 +20,21 @@ IssueSerializer.Meta.read_only_fields, closing the PATCH vector directly. The escalation chain: forge created_by via PATCH, then pass IssueDetailAPIEndpoint.delete's "admin OR creator" gate as a plain member. -Test `test_patch_then_delete_forgery_chain_is_blocked` exercises the whole -chain end to end, not just the individual forgery. +Test `test_patch_then_delete_is_still_403_for_a_plain_member` exercises the +whole chain end to end, not just the individual forgery. """ +from datetime import timedelta from uuid import uuid4 import pytest +from django.utils import timezone from rest_framework import status from plane.db.models import APIToken, Issue, IssueComment, IssueLink, Project, ProjectMember, State +FORGED_CREATED_AT = timezone.now() - timedelta(days=3650) # 10 years back — unmistakably not "now" + def _create_issue_as(creator, **kwargs): """BaseModel.save() sets created_by from crum's current request/user, and @@ -126,7 +130,12 @@ class TestIssueCreateIgnoresBodyCreatedBy: url = _issues_url(workspace.slug, project.id) response = member_api_client.post( url, - {"name": "Spoofed issue", "state": str(state.id), "created_by": str(admin_user.id)}, + { + "name": "Spoofed issue", + "state": str(state.id), + "created_by": str(admin_user.id), + "created_at": FORGED_CREATED_AT.isoformat(), + }, format="json", ) assert response.status_code == status.HTTP_201_CREATED, f"got {response.status_code}: {response.data!r}" @@ -136,6 +145,9 @@ class TestIssueCreateIgnoresBodyCreatedBy: "created_by must be the authenticated caller regardless of what the body requested" ) assert created.created_by_id != admin_user.id + assert created.created_at > FORGED_CREATED_AT + timedelta(days=1), ( + "created_at must not be backdated by a body-supplied value" + ) # No test for IssueDetailAPIEndpoint.put's external_id-upsert create branch: # confirmed against apps/api/plane/api/urls/work_item.py that @@ -211,7 +223,11 @@ class TestCommentCreateIgnoresBodyCreatedBy: url = _comments_url(workspace.slug, project.id, issue.id) response = member_api_client.post( url, - {"comment_html": "

please approve the payment

", "created_by": str(admin_user.id)}, + { + "comment_html": "

please approve the payment

", + "created_by": str(admin_user.id), + "created_at": FORGED_CREATED_AT.isoformat(), + }, format="json", ) assert response.status_code == status.HTTP_201_CREATED, f"got {response.status_code}: {response.data!r}" @@ -220,6 +236,9 @@ class TestCommentCreateIgnoresBodyCreatedBy: assert comment.created_by_id == member_user.id assert comment.created_by_id != admin_user.id assert comment.actor_id == member_user.id, "actor (the audit-log identity) must also be the real caller" + assert comment.created_at > FORGED_CREATED_AT + timedelta(days=1), ( + "created_at must not be backdated by a body-supplied value" + ) @pytest.mark.django_db