Work item create/update via the external API returned 200/201 even
when assignees or labels didn't belong to the project, quietly
filtering the invalid ids out with no error. Now raises the same kind
of ValidationError already used for state/parent, matching the pattern
of #9517.
* [WEB-8352] fix(security): scope SubIssuesEndpoint to the URL project (GHSA-gxhv-fw9x-2pg3)
SubIssuesEndpoint is guarded only by ProjectEntityPermission, which verifies
the caller belongs to the URL project_id but not that the path issue_id lives
in that project. Both handlers then resolved issues without a project scope:
- GET filtered sub-issues by parent_id + workspace__slug only, leaking the
names/priorities/assignees/dates of another project's sub-issues (read IDOR).
- POST loaded the parent by bare pk (no workspace/project scope) and filtered
the moved sub-issues by workspace__slug only, letting any project member
re-parent issues from other projects/workspaces (write IDOR).
Scope the parent lookup and both sub-issue querysets to the URL project_id
(and bind the parent to the workspace), returning 404 when the parent is not
in the caller's project. Adds 5 contract tests (3 security, 2 positive
controls); fail-before verified.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* [WEB-8352] fix: dispatch sub-issue activity only for project-scoped issues (CodeRabbit/Copilot #9466)
The DB update + response were scoped to the URL project, but the activity loop
still iterated the raw caller-supplied sub_issue_ids. A cross-project id
(excluded from the re-parent) would still fire issue_activity.delay, whose task
does an unscoped Issue.objects.get and bumps updated_at — touching a foreign
issue and creating a bogus activity row.
Dispatch from the project-scoped sub_issues (scoped_sub_issue_ids) instead.
Strengthened the test to assert the foreign issue is absent from the response
body (sub_issues / state_distribution) and that no activity is dispatched for it
(mock). Fail-before verified.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* chore(security): drop advisory identifiers from code comments
Explanations kept unchanged; only the IDs are removed.
Co-authored-by: Plane AI <noreply@plane.so>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-authored-by: Plane AI <noreply@plane.so>
* [WEB-8283] fix: bind Spaces board object IDs to the anchor's project
The public Spaces board endpoints resolved the DeployBoard from the URL
anchor but trusted the caller-supplied issue_id/comment_id/intake_id
verbatim, without verifying the object belonged to that board's
project/workspace. Any authenticated user could write comments,
reactions and votes onto arbitrary issues cross-tenant, and read EXTERNAL
comments from a different project in the same workspace.
Bind every caller-supplied object id to the board's project + workspace
before writing:
- comment / issue-reaction / vote create: require the issue to exist in
the board's project via Issue.issue_objects (excludes draft/archived/
triage), else 404.
- comment-reaction create: require the comment to exist in the board's
project with access="EXTERNAL", else 404.
- intake create: require the URL intake_id to match the board's intake,
else 400.
- comment list read: scope the queryset to the board's project_id.
Also add the missing is_votes_enabled gate on vote create for parity with
comment/reaction create (pre-existing gap in the same method).
Adds contract regression tests (fail-before verified): cross-tenant
writes and the cross-project comment read now rejected, with positive
controls confirming legitimate board writes/reads still succeed.
Co-authored-by: Plane AI <noreply@plane.so>
* [WEB-8283] test: address Copilot review — cross-workspace + votes-disabled coverage
- Add cross-workspace write test (issue in a different workspace) to
exercise the workspace_id binding, matching the advisory's cross-tenant
impact (previously only same-workspace/different-project was covered).
- Add a regression test for the new is_votes_enabled gate on vote create
(votes-disabled board → 400), preventing the pre-existing gap from
reappearing.
- Clarify the section header comment: cross-tenant writes return 404 for
issue/comment binding, 400 for the intake binding mismatch.
Co-authored-by: Plane AI <noreply@plane.so>
* [WEB-8283] refactor: extract board-scope guards into shared helpers
Address CodeRabbit review: the identical "object belongs to the board's
project+workspace" existence check was duplicated across four create()
methods (in four different ViewSets). Extract two module-level helpers —
_issue_in_board_scope and _comment_in_board_scope — so the check is a
single source of truth and cannot drift between endpoints or be forgotten
on a new one (the exact class of bug this PR fixes).
Behavior-preserving; 14 contract tests still green.
Co-authored-by: Plane AI <noreply@plane.so>
* chore(security): drop advisory identifiers from code comments
Explanations kept unchanged; only the IDs are removed.
Co-authored-by: Plane AI <noreply@plane.so>
* fix(security): use the deploy board's project_id in the reaction-create activity log
CommentReactionPublicViewSet.create() logged the activity with
str(self.kwargs.get("project_id", None)) — this route's URL only ever
supplies anchor and comment_id, never project_id, so every comment
reaction created on a public board logged project_id="None", silently
corrupting the activity/audit trail. destroy() on the same viewset
already resolves the correct project_id from the deploy board; create()
now does the same.
Co-authored-by: Plane AI <noreply@plane.so>
* [WEB-8283] fix: apply board-scope guards to comment/reaction read, update and delete paths
The board/issue/external-comment scoping added by this PR's create() methods
was never applied to the list, update and delete paths built on the same
models. A caller could read reactions on an INTERNAL (non-public) comment
through the public reaction list, or reach a comment or reaction they
authored through a board it doesn't actually belong to via partial_update()
or destroy(), since those methods looked up objects by pk/actor only.
Bind IssueCommentPublicViewSet.partial_update()/destroy() to the board's
project, workspace, issue_id and EXTERNAL access; bind
IssueReactionPublicViewSet.destroy() to the board's project (previously only
workspace-scoped); and bind CommentReactionPublicViewSet.get_queryset()/
destroy() to EXTERNAL comments only. Add regression coverage for each gap,
plus positive controls confirming legitimate reads/writes on the board's own
objects still work.
Co-authored-by: Plane AI <noreply@plane.so>
---------
Co-authored-by: Plane AI <noreply@plane.so>
* [WEB-8110] fix: sanitize page list order_by against an allowlist (GHSA-2v48)
PageViewSet.get_queryset passed the raw order_by query param into
.order_by(). In Django 4.2 .order_by() resolves field names at call time,
so an unknown field (e.g. order_by=password) raises FieldError → 500 DoS,
and a valid relation path (e.g. order_by=owned_by__password) enables ORM
relational traversal (GHSA-2v48-qcjw-74ch).
Add PAGE_ORDER_BY_ALLOWLIST to utils/order_queryset.py and wrap the param
with the existing sanitize_order_by() before it reaches .order_by(),
matching the issue/project/view/notification endpoints. Unknown or
malformed values fall back to the safe -created_at default.
Covers only the app project-pages residual; the 3 external-REST-API sites
in the advisory are handled by PR #9348. EE Wiki counterpart: WEB-8111.
Add contract regression tests (fail-before verified).
Co-authored-by: Plane AI <noreply@plane.so>
* [WEB-8110] fix: fold sanitized order_by into a single order_by() call (review)
Address CodeRabbit + Copilot on #9387: the sanitized .order_by(user) was
immediately overridden by a later .order_by("-is_favorite", "-created_at"),
so the order_by param had no effect on the result (dead code) and cost an
extra query-build step.
Merge them into one .order_by("-is_favorite", <sanitized>, "id") — matching
the EE project-pages viewset — so favourites stay pinned first, the
allowlisted user ordering actually applies as the secondary sort, and id is
a stable pagination tiebreak. The no-param default is unchanged
(-created_at). Add a test asserting order_by=name / -name actually reorders
the results.
Co-authored-by: Plane AI <noreply@plane.so>
* chore(security): drop advisory identifiers from code comments
Explanations kept unchanged; only the IDs are removed.
Co-authored-by: Plane AI <noreply@plane.so>
---------
Co-authored-by: Plane AI <noreply@plane.so>
* [WEB-8103] fix: stop leaking webhook HMAC secret_key on reads (GHSA-83rj)
WebhookEndpoint list/retrieve/patch pass a fields= allowlist that excludes
secret_key, but DynamicBaseSerializer.__init__ discards the caller
allowlist (fields = self.expand). With WebhookSerializer using
fields="__all__" and secret_key only in read_only_fields (read-only is
still serialized), the HMAC signing secret leaked on every webhook read
(GHSA-83rj-4282-x39v; admin-only).
Rather than secret_key = CharField(write_only=True) — which would let a
client inject their own secret on create/patch and break the intended
one-time reveal — hide it by default and reveal only where intended:
- WebhookSerializer.to_representation drops secret_key unless the
show_secret_key context flag is set (secure by default). secret_key
stays server-generated (default=generate_token) and non-writable.
- POST create and WebhookSecretRegenerateEndpoint pass show_secret_key so
the secret is still returned once for the caller to configure their
receiver; list/retrieve/patch no longer emit it.
Add contract regression tests (fail-before verified). Follow-up: the
DynamicBaseSerializer.__init__ allowlist bug affects other serializers —
tracked separately.
Co-authored-by: Plane AI <noreply@plane.so>
* [WEB-8103] test: address review — patch network boundary + pin to_representation
Per review (@sriramveeraghanta):
- Patch the network boundary (validate_url) instead of the whole private
_validate_webhook_url method, so the domain/schema checks still run and
the test survives a rename of the private method.
- Add an assertion that an explicit fields=("secret_key",) request still
hides the key, pinning to_representation as the enforcement point so a
future DynamicBaseSerializer._filter_fields fix can't silently re-open
the leak.
Co-authored-by: Plane AI <noreply@plane.so>
* [WEB-8103] docs: spell out both levels of the dead fields= allowlist
The previous comment named only DynamicBaseSerializer.__init__ discarding the
caller's fields=, which is half the root cause. _filter_fields never removes
anything either: it builds `allowed` purely to attach expansion serializers for
names not already on the serializer, then returns self.fields unfiltered
(serializers/base.py:45-119).
So the fields= kwargs in views/webhook/base.py are no-ops on two independent
levels. Documented so a future `fields = fields or self.expand` fix isn't
assumed to re-activate the allowlists for confidentiality — _filter_fields has
to be made restrictive first. The show_secret_key context flag remains the sole
enforcement point.
Addresses @sriramveeraghanta's review on #9382. 6/6 contract tests pass.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* chore(security): drop advisory identifiers from code comments
Explanations kept unchanged; only the IDs are removed.
Co-authored-by: Plane AI <noreply@plane.so>
---------
Co-authored-by: Plane AI <noreply@plane.so>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix: add rate limiting to email/password sign-in and sign-up endpoints
All four password authentication views (app sign-in, app sign-up, space
sign-in, space sign-up) extended django.views.View, so DRF's global
AnonRateThrottle never ran and the endpoints accepted unlimited credential
guesses with no friction (brute-force / credential stuffing, GHSA-349j).
Add authentication_throttle_allows(request) at the top of each post()
method — before any DB access — using the same AuthenticationThrottle
already guarding the magic-code views. On rejection the view redirects with
RATE_LIMIT_EXCEEDED, consistent with all other throttled auth endpoints.
Default limit remains 10/minute, overridable via AUTHENTICATION_RATE_LIMIT.
Co-authored-by: Plane AI <noreply@plane.so>
* refactor: consolidate auth throttle into a decorator + add tests
- Extract the repeated throttle-and-redirect block from the six redirect-flow
auth views (email + magic, app + space) into a single throttle_auth_redirect
decorator in rate_limit.py. Behaviour is unchanged: the throttle still runs
before any DB access; brute-force traffic is rejected without a DB hit.
- Add regression tests for the password sign-in/sign-up throttle on both app
and space endpoints, mirroring the existing magic-code throttle tests.
- Reset the shared AuthenticationThrottle bucket before every test in
test_authentication.py. All auth endpoints share one per-IP throttle scope,
so the newly-throttled password requests exhausted the budget mid-file and
caused unrelated tests to trip RATE_LIMIT_EXCEEDED.
---------
Co-authored-by: Plane AI <noreply@plane.so>
Co-authored-by: sriramveeraghanta <veeraghanta.sriram@gmail.com>
filter_updated_at() passed `created_at__date` as the date term in both its GET
and POST branches, so filtering by "Updated At" actually filtered on the
creation date. Work items updated today but created earlier never appeared under
"Updated At -> is -> today", and the two filters returned identical result sets.
Re-raise of community PR #9323 by @sanjibani, which is approved-ready but cannot
merge because the CLA is unsigned. Original patch and tests carried over
unchanged apart from the two fixes below; credit for the fix is theirs.
Adjustments made while porting:
- test_get_method_targets_updated_at_column asserted the exact key
"updated_at__date", but date_filter's single-value branch appends a lookup
suffix ("updated_at__date__contains"), so the assertion always failed. Match
on the key prefix instead.
- Added the missing trailing newline to the new test file.
Verified on a local canary build: with one work item created 2020-01-01 but
updated today, `?updated_at=2026-07-01;after` now returns 5 while
`?created_at=2026-07-01;after` returns 4 — previously both returned 4. Unit
tests pass (5); 4 of the 5 fail without the source change.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Bug 5: filtering the work item list by a creation/update date returned an empty
list. created_at and updated_at are DateTimeFields, but the UI sends a bare
calendar date, and the filterset only exposed `exact` and `range` lookups:
- {"created_at__exact": "2026-07-30"} was coerced to 2026-07-30 00:00:00, so it
matched only rows stamped exactly midnight — effectively never.
- {"created_at__range": "2026-07-28,2026-07-30"} capped the upper bound at
2026-07-30 00:00:00, silently dropping everything created during that final
day (the range only "worked" if you overshot the end date by one day).
Compare the date component instead (`date` / `date__range` via a CSV-parsing
DateCSVRangeFilter), so a calendar date means the whole day and both range
bounds are inclusive. The UI's existing query format is unchanged.
Verified against a local canary build: the exact requests from the bug report
now return 4 and 4 (previously 0 and 0). Adds unit coverage; 6 of the 7 new
tests fail without this change.
Note: `__date` is evaluated in the active timezone, which TimezoneMixin takes
from the user's profile (user_timezone) rather than the browser's timezone, so a
profile/browser timezone mismatch can still shift results by a day. Tracked
separately — not addressed here.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* chore(api): upgrade Django 4.2 → 5.2 and bump Django ecosystem deps
Upgrade Django 4.2.30 LTS → 5.2.15 LTS and bump all Django-coupled
dependencies to versions that officially support 5.2 (DRF 3.17.1,
channels 4.3.2, django-cors-headers 4.9.0, django-filter 25.2,
django-storages 1.14.6, django-redis 7.0.0, celery 5.5.3,
django-celery-beat 2.9.0, django-celery-results 2.6.0,
drf-spectacular 0.29.0, scout-apm 3.5.3, psycopg 3.3.4,
whitenoise 6.12.0, django-debug-toolbar 6.0.0, pytest-django 4.12.0).
OpenTelemetry set, django-crum and pytz held (already 5.2-compatible).
Code changes the upgrade required:
- urls.py: gate the debug-toolbar URL include on apps.is_installed(),
since django-debug-toolbar 6.0 ships a model that errors when the app
isn't in INSTALLED_APPS (test settings run DEBUG=True but don't install it).
- migration 0122: state-only AlterField for three M2M fields using
through_fields (Django 5.1 deconstruction normalization); sqlmigrate is a
no-op, zero DB impact.
- test_authentication.py: module-level autouse cache.clear() fixture to fix
8 pre-existing throttle test-isolation failures (identical on the 4.2
baseline) so the suite is green.
Verified on python:3.12-alpine + Postgres 15.7: check clean,
makemigrations --check clean, full migrate applies, pytest 393 passed.
Adds the migration plan/audit write-up under apps/api/docs/.
* fix(api): scope auth test cache reset to throttle keys only
The module-wide _reset_auth_throttle_cache fixture called cache.clear(),
wiping the entire shared Redis cache between tests. Replace it with
targeted deletion of throttle_authentication_* keys (DRF
SimpleRateThrottle history for AuthenticationThrottle) via
cache.delete_pattern, keeping the same before/after cleanup.
* feat: add lite list endpoints for projects, members, cycles, and modules
* refactor: enhance order_by sanitization for cycle and module endpoints, update error handling for non-existent projects and workspaces
* [WEB-8074] fix: scope IssueListEndpoint to guest created_by
IssueListEndpoint.get (/workspaces/<slug>/projects/<project_id>/issues/list/)
returned any issue whose id was passed in ?issues=, without the guest
created_by restriction its sibling IssueViewSet.list enforces. A project GUEST
(role=5) on a project with guest_view_all_features=False could read issues they
did not author by supplying their ids (GHSA-32c7-84jc-4w67).
Replicate the guest scope: when the requester is an active role=5 ProjectMember
and not project.guest_view_all_features, filter the queryset to
created_by=request.user. Applied to the base queryset so it flows through
filtering, annotation and grouping.
Contract regression tests cover the restricted guest (own-only), a full member
(sees all), and a guest with guest_view_all_features enabled (sees all);
fail-before verified.
Co-authored-by: Plane AI <noreply@plane.so>
* Potential fix for pull request finding
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
---------
Co-authored-by: Plane AI <noreply@plane.so>
Co-authored-by: Dheeraj Kumar Ketireddy <dheeraj.ketireddy@plane.so>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
* [WEB-8095] fix: scope page-version reads to the URL project (GHSA-g49r/ghcr)
ProjectPagePermission verified the caller was a member of the URL
project_id but then resolved the page by workspace + page_id only, and
PageVersionEndpoint filtered versions the same way. A member of one
project could read the page versions of a public page belonging to a
different project in the same workspace via that project's URL
(GHSA-g49r-p85q-qq2w / GHSA-ghcr-frqr-6pqr).
- Scope the page lookup in ProjectPagePermission to projects__id via the
ProjectPage M2M (both app/ and utils/ copies); deny when the page does
not belong to the URL project.
- Scope PageVersionEndpoint list/detail querysets to
page__projects__id=project_id (defense in depth); distinct() on the
list guards against active + soft-deleted ProjectPage duplicates.
- Add contract regression tests (fail-before verified).
Co-authored-by: Plane AI <noreply@plane.so>
* [WEB-8095] fix: require active ProjectPage link when scoping pages to a project
Address CodeRabbit + Copilot review on #9380: projects__id=project_id
matched even soft-deleted ProjectPage links, so a page removed from the
project (link revoked) would still pass, and the version detail get()
could raise MultipleObjectsReturned on active + soft-deleted rows.
Put both conditions on the same project_pages relation in one filter so
they match a single ProjectPage row that is active:
project_pages__project_id=project_id + project_pages__deleted_at__isnull
=True. The partial-unique constraint (project, page WHERE deleted_at IS
NULL) then guarantees at most one row, so get() stays unambiguous and the
list needs no distinct(). Add a revoked-link regression test.
Co-authored-by: Plane AI <noreply@plane.so>
* [WEB-8095] fix: distinct() on page-version detail lookup as a MultipleObjectsReturned guard
Address CodeRabbit review on #9380. The active-link filter already keeps
the page__project_pages join to a single row via the partial-unique
constraint, but add distinct() to the detail get() as defense in depth so
the join can never surface MultipleObjectsReturned (a 500) even if that
invariant were ever violated.
Co-authored-by: Plane AI <noreply@plane.so>
---------
Co-authored-by: Plane AI <noreply@plane.so>
The SAFE_METHODS branch of ProjectMemberPermission filtered ProjectMember by
workspace only (no project_id), so any workspace user who was a member of *some*
project could pass the check for a project they were not in. Consumers then
returned project-scoped data:
- v1 ProjectMemberListCreateAPIEndpoint.get -> full project roster
(GHSA-w2vf-m9x9-mvmc)
- app DeployBoardViewSet.list -> project publish configuration (identical
app-copy sibling)
Add project_id=view.project_id to the SAFE_METHODS filter in both copies
(utils + app), mirroring the non-safe branch and ProjectEntityPermission. A
non-member now receives 403.
Contract regression tests cover both endpoints: a workspace user who is a
member of a different project is denied (403) on a foreign project, while an
active member of the target project is allowed. Fail-before verified (both
denied cases leak 200 without the fix).
Co-authored-by: Plane AI <noreply@plane.so>
* [WEB-8068] fix: scope workspace cycles/modules listing to project membership
WorkspaceCyclesEndpoint and WorkspaceModulesEndpoint are guarded only by
WorkspaceViewerPermission (any active workspace member) and filtered by
workspace__slug alone, letting any workspace member enumerate cycle/module
metadata (names, dates, issue counts) of private projects they are not a
member of (GHSA-wcc5-qgfr-8g9c).
Restrict both querysets to projects the requesting user is an active member
of, mirroring WorkspaceStatesEndpoint / WorkspaceLabelsEndpoint:
project__project_projectmember__member=request.user
project__project_projectmember__is_active=True
project__archived_at__isnull=True
Add .distinct() to the Module query (the member join is to-many; Cycle already
had it).
Contract regression tests cover hidden cycles/modules for a non-project member,
the positive project-member path, and no row duplication; fail-before verified.
Co-authored-by: Plane AI <noreply@plane.so>
* [WEB-8068] refactor: drop unnecessary distinct() from module listing
Address Copilot review: the project-membership join is filtered to
request.user, and ProjectMember has a unique constraint on (project, member)
where deleted_at IS NULL, so the join yields at most one row per project and
cannot duplicate Module rows. distinct() was dead weight (and a planner cost
for large workspaces). Matches the reference WorkspaceStates/WorkspaceLabels
endpoints, which use no distinct().
Also drop the distinct-focused contract test: adding a *different* project
member never fans out the request.user-filtered join, so it would pass with or
without distinct() — misleading coverage.
Co-authored-by: Plane AI <noreply@plane.so>
---------
Co-authored-by: Plane AI <noreply@plane.so>
* [WEB-8066] fix: scope workspace asset get/patch/delete to project membership
WorkspaceFileAssetEndpoint is authorized at the WORKSPACE level, so any
workspace member/guest could reach get/patch/delete for a project-bound
asset (issue attachment/description, comment description, page description)
of a project they are not a member of — an incomplete fix of the GHSA-qw87
asset-IDOR cluster (GHSA-h7mc-p9mm-2r4w / GHSA-cjph-cgm5-8pw8).
Add project_membership_denied(): for project-bound assets (project_id set)
require an active ProjectMember of the asset's project, else 403. Workspace-
level entity types (WORKSPACE_LOGO, USER_AVATAR, USER_COVER) have project_id
NULL and remain accessible to any workspace member. Mirrors ProjectAssetEndpoint
(level=PROJECT). Guard runs before the is_uploaded check / mutation so a
non-member gets a uniform 403 and cannot probe upload state.
Contract regression tests cover denied get/patch/delete for a non-project
member, the positive project-member path, and the workspace-level exemption;
fail-before verified.
Co-authored-by: Plane AI <noreply@plane.so>
* [WEB-8066] harden: scope asset project-membership check to the asset's workspace
Address Copilot review: filter ProjectMember by workspace_id=asset.workspace_id
in addition to project_id, mirroring allow_permission's PROJECT branch. Prevents
a member of the same project in a different workspace from passing the check if
an asset row is ever inconsistent (asset.workspace_id != project.workspace_id).
Co-authored-by: Plane AI <noreply@plane.so>
* [WEB-8066] refactor: return bool from asset access helper, build Response in views
Address review (Saurabhkmr98): rename project_membership_denied ->
has_project_asset_access, returning a boolean (True = allowed) instead of a
Response. Each of get/patch/delete now builds the 403 Response based on the
returned value. Behaviour is unchanged (same 403 + message; workspace-level
assets with project_id=None still allowed).
Co-authored-by: Plane AI <noreply@plane.so>
---------
Co-authored-by: Plane AI <noreply@plane.so>
Add ISSUE_GROUP_BY_ALLOWLIST and validate group_by_field_name/
sub_group_by_field_name in BasePaginator.paginate() — the single chokepoint
all GroupedOffsetPaginator/SubGroupedOffsetPaginator callers funnel through
(the unauthenticated public deploy-board endpoint plus 5 GUEST-reachable
authenticated endpoints). Invalid fields now raise ParseError (HTTP 400)
instead of reaching F()/.values()/.order_by()/Window partition_by as a raw
ORM field name, which previously let an anonymous caller crash the endpoint
or force a blind relational-traversal oracle (GHSA-wwgj-929g-42cm).
Same field-name-injection class as the order_by fix (GHSA-2r95/GHSA-w45q,
WEB-7813), which never extended to group_by/sub_group_by.
Closes WEB-8012
Co-authored-by: Plane AI <noreply@plane.so>
* [WEB-8060] fix(security): enforce authz on is_active member (de)activation
ProjectMemberViewSet.partial_update nested every authorization guard inside
`if "role" in request.data:`. Because ProjectMemberSerializer exposes is_active
through fields="__all__" with no read-only, a project GUEST could PATCH
{"is_active": false} while omitting "role" to deactivate any member — including
admins — and take over the project (GHSA-hpgm-9r34-c4x5 / GHSA-25gg-cxm8-g7h9).
Add an independent is_active guard, mirroring the role block and destroy(): only
a project admin (or workspace admin) may (de)activate a member, and never one
whose role is equal to or higher than the requester's own.
Adds contract regression tests covering guest/member deactivation attempts and
the legitimate project-admin path.
Co-authored-by: Plane AI <noreply@plane.so>
* [WEB-8060] test(security): cover workspace-admin is_active bypass
Address Copilot review on #9367: add a positive-control test asserting a
workspace admin holding only a project GUEST role can still deactivate a project
admin (the intended is_workspace_admin bypass), so future changes cannot silently
remove it.
Co-authored-by: Plane AI <noreply@plane.so>
---------
Co-authored-by: Plane AI <noreply@plane.so>
* [WEB-8017] fix(security): sanitize order_by on external REST API list endpoints
Close a partial bypass of WEB-7813 (GHSA-2r95 / GHSA-w45q): the external
REST API project-list and work-item-list endpoints passed a raw order_by
query parameter to Django's .order_by(). Because Django resolves
__-separated relational paths, an attacker could order by sensitive
columns on related tables (created_by__password / token / email) to build
a blind ordering oracle, or crash the endpoint (HTTP 500) with an unknown
field.
Route both endpoints through the existing sanitize_order_by() helper with
the appropriate allowlist (PROJECT_ORDER_BY_ALLOWLIST, default sort_order;
ISSUE_ORDER_BY_ALLOWLIST, default -created_at), mirroring how
order_issue_queryset() already sanitizes. Non-allowlisted values collapse
to the safe default; legitimate orderings are unchanged.
Adds unit tests (allowlist neutralisation + passthrough) and contract
tests asserting both endpoints return 200 (not 500) for injected fields;
fail-before verified via git stash.
Advisory: GHSA-p885-6jpg-cr2p
Co-authored-by: Plane AI <noreply@plane.so>
* [WEB-8019] fix(security): scope CycleIssue reassignment lookup to workspace/project
CycleIssueViewSet.create looked up "issues already in another cycle" with
CycleIssue.objects.filter(~Q(cycle_id=cycle_id), issue_id__in=issues) —
without scoping to the caller's workspace/project. An ADMIN/MEMBER of their
own project could pass a work-item UUID from a different tenant and have that
foreign CycleIssue row reassigned to their cycle, silently evicting the
victim's work item from the victim's cycle (cross-tenant write / BOLA).
Scope the lookup to workspace__slug + project_id, mirroring the adjacent
create-path guard. Foreign-tenant rows are excluded from reassignment and
already dropped from the create path by the scoped new_issues query.
Adds a contract regression test proving a foreign-tenant CycleIssue row is
not reassigned (fail-before verified via git stash) plus a same-project
reassignment test to confirm the legitimate flow is unaffected.
Advisory: GHSA-4w5x-wc9w-f47x
Co-authored-by: Plane AI <noreply@plane.so>
---------
Co-authored-by: Plane AI <noreply@plane.so>
Close a partial bypass of WEB-7813 (GHSA-2r95 / GHSA-w45q): the external
REST API project-list and work-item-list endpoints passed a raw order_by
query parameter to Django's .order_by(). Because Django resolves
__-separated relational paths, an attacker could order by sensitive
columns on related tables (created_by__password / token / email) to build
a blind ordering oracle, or crash the endpoint (HTTP 500) with an unknown
field.
Route both endpoints through the existing sanitize_order_by() helper with
the appropriate allowlist (PROJECT_ORDER_BY_ALLOWLIST, default sort_order;
ISSUE_ORDER_BY_ALLOWLIST, default -created_at), mirroring how
order_issue_queryset() already sanitizes. Non-allowlisted values collapse
to the safe default; legitimate orderings are unchanged.
Adds unit tests (allowlist neutralisation + passthrough) and contract
tests asserting both endpoints return 200 (not 500) for injected fields;
fail-before verified via git stash.
Advisory: GHSA-p885-6jpg-cr2p
Co-authored-by: Plane AI <noreply@plane.so>
Bot service accounts (User.is_bot=True, e.g. the WORKSPACE_SEED bot) are
internal identities meant to act only through API tokens. Nothing stopped
one from being driven through the interactive login flow if its email was
known, letting a human assume a service identity.
Reject bot accounts at the shared login chokepoint,
Adapter.complete_login_or_signup(), right beside the existing
deactivated-account check. This covers every interactive provider in one
place: email/password, magic code, and all OAuth providers (Google, GitHub,
GitLab, Gitea) across both the app and space surfaces. Bot API-token access
is left untouched, since that is how bots are meant to operate.
Also add a defense-in-depth is_bot guard to InstanceAdminSignInEndpoint,
which mints its own admin session outside the chokepoint (a bot is never an
InstanceAdmin today, so this is not currently reachable, but it closes the
path regardless).
Surface the rejection with a new dedicated error code
BOT_USER_LOGIN_FORBIDDEN (5017), plumbed into the app and space frontend
error helpers as well as the shared @plane/constants and @plane/utils
packages (message map + banner-alert list) so any consumer of the shared
auth-error handler renders it correctly. The admin path reuses the existing
ADMIN_AUTHENTICATION_FAILED code so it discloses no bot-specific error.
Add contract regression tests: a bot blocked via password and via magic
code, a bot blocked at the admin sign-in endpoint, and a non-bot control
that still logs in.
* fix: scope workspace user preference filter to current user
Without user=request.user on the PATCH filter, the ORM could match
another user's preference record in the same workspace, causing
pin/unpin state to leak across users or silently fail to persist.
Fixes#9260
Signed-off-by: okxint <cashmein.eth@gmail.com>
* test: add regression coverage for workspace user preference scoping (#9260)
Adds contract tests for the sidebar preference PATCH endpoint:
- test_patch_only_updates_requesting_users_preference: in a multi-member
workspace, a member's PATCH must update only their own preference row,
never another member's. Fails against the pre-fix code (the unscoped
.first() mutates the most-recently-created row regardless of user).
- test_patch_updates_own_preference: baseline that a member's PATCH
persists to their own row.
Verified RED on the unpatched view and GREEN with the user=request.user
filter from #9261.
* fix(api): wrap long line to satisfy ruff E501 in user preference view
---------
Signed-off-by: okxint <cashmein.eth@gmail.com>
Co-authored-by: okxint <cashmein.eth@gmail.com>
* fix(api): require at least one alphanumeric char in workspace name
Workspace name validation was enforced only on the frontend
(validateWorkspaceName), which gates the UI submit but is bypassable
via a direct API call. The backend WorkSpaceSerializer.validate_name
only rejected URLs, so a symbol-only name like "-_________-" could
still be saved via create or the rename (partial_update) path.
Add a Unicode-aware has_alphanumeric() helper and enforce it in both
the app and instance/license workspace serializers, mirroring the
frontend HAS_ALPHANUMERIC_REGEX (/[\p{L}\p{N}]/u) added in #9263.
International names (日本語, José, محمد) still pass since str.isalnum()
covers all scripts.
Adds unit tests covering symbol-only rejection and international
acceptance on both serializers.
Refs #9255
Signed-off-by: sriramveeraghanta <veeraghanta.sriram@gmail.com>
* fix(api): reject URLs in instance workspace name for parity
Address CodeRabbit review on #9278: the instance/license
WorkspaceSerializer.validate_name rejected symbol-only names but, unlike
the app-level WorkSpaceSerializer, still accepted names containing URLs.
Add the same contains_url() guard (imported from plane.utils.url, not
content_validator) so both workspace-create paths validate identically.
Add unit tests asserting URL-containing names are rejected on both
serializers.
Signed-off-by: sriramveeraghanta <veeraghanta.sriram@gmail.com>
---------
Signed-off-by: sriramveeraghanta <veeraghanta.sriram@gmail.com>
The custom API key authentication only verified that the APIToken row was
active and unexpired; it never checked the owning user's is_active flag.
DRF's IsAuthenticated only checks user.is_authenticated (always True for a
real User), so a user whose account was deactivated could keep using a
previously issued API key indefinitely.
Add user__is_active=True to the validate_api_token() lookup so a token tied
to a disabled account is treated as invalid (a generic AuthenticationFailed,
avoiding account-state disclosure). Applied to both the external API
middleware (plane/api) and the identical, currently unused copy in
plane/app to prevent the gap from being reintroduced.
Adds unit coverage on validate_api_token and an end-to-end contract test
proving GET /api/v1/users/me/ is denied once the account is deactivated.
User-controlled values (work item titles, labels, etc.) were written
raw into openpyxl worksheet cells, so values beginning with = were
stored as live formula cells in exported XLSX files. Apply the same
formula-trigger sanitization already used for CSV exports to XLSX
cell values and header rows in both export formatters, and sanitize
CSV header rows in the porters formatter for parity.
The public REST API GenericAssetEndpoint (/api/v1/workspaces/<slug>/assets/)
declared no permission class, inheriting only IsAuthenticated. Since
APIKeyAuthentication does not bind a token to a workspace and the workspace is
read straight from the URL slug, any valid Personal Access Token could read
(GET), create (POST), and modify (PATCH) assets in a workspace the caller is
not a member of — a cross-workspace IDOR, the public-API sibling of the
CVE-2026-46558 dashboard asset fix.
Add permission_classes = [WorkspaceUserPermission] so every method requires
active workspace membership, matching the dashboard fix semantics. Also add
contract regression tests covering cross-workspace GET/POST/PATCH (now 403)
and a positive control confirming members retain access.
Also ignore the local /security/ advisory notes folder.
* fix(api): rate-limit magic-code verification and bound per-token attempts
The magic-link sign-in / sign-up endpoints accept a 6-digit numeric code
(900k-value space, 600s TTL) but never increment a failure counter on a
wrong-code verify and extend django.views.View rather than DRF APIView,
so DRF's AuthenticationThrottle never runs against them. The space-side
generate endpoint also lacked throttle_classes. Combined, this allowed
an unauthenticated attacker who knew a victim's email to brute-force
the code within the TTL window and log in as the victim.
- Add MAX_VERIFY_ATTEMPTS=5 in MagicCodeProvider.set_user_data: failed
comparisons now persist verify_attempts in Redis under the remaining
TTL and, on hitting the limit, delete the key and raise
EMAIL_CODE_ATTEMPT_EXHAUSTED. This is the load-bearing fix - it caps
total attempts per issued token regardless of request rate.
- Add authentication_throttle_allows() so plain Django Views can apply
AuthenticationThrottle without converting to APIView (would change
CSRF + request-parsing semantics for the redirect-flow endpoints).
- Apply the throttle to MagicSignIn/UpEndpoint and the space variants;
add throttle_classes to MagicGenerateSpaceEndpoint to match its app
sibling.
Refs GHSA-9pvm-fcf6-9234.
* fix(api): make verify-attempt increment atomic, expose throttle rate via env
Address PR review feedback:
- Replace the JSON read-modify-write of verify_attempts with a Lua
EVAL script that INCRs a dedicated counter key and EXPIREs it only
on the first increment. The previous round-trip was racy: parallel
wrong-code requests could read the same value and both write the
same incremented count, letting an attacker exceed MAX_VERIFY_ATTEMPTS
under concurrency. Counter is now reset on each new token issuance
and cleared on successful verify / exhaustion.
- Make AuthenticationThrottle.rate configurable via the
AUTHENTICATION_RATE_LIMIT env var (default 10/minute, down from 30
to tighten the budget on unauth auth-adjacent endpoints). Document
it in deployments/aio and deployments/cli variables.env.
* test(api): cover magic-code attempt cap, counter reset, and auth throttle
Add the contract tests called out in the PR test plan:
- TestMagicSignInVerifyAttempts:
- test_exhausted_after_max_wrong_attempts: after MAX_VERIFY_ATTEMPTS
wrong codes the next verify redirects with EMAIL_CODE_ATTEMPT_
EXHAUSTED_SIGN_IN and both Redis keys are deleted; a follow-up
verify reports EXPIRED.
- test_counter_increments_on_each_wrong_attempt: the dedicated
verify_attempts counter advances by exactly one per wrong POST,
matching the atomic Lua INCR.
- test_counter_resets_on_token_regeneration: regenerating the
magic-link clears the counter so the user isn't pre-locked-out by
a prior session's wrong attempts.
- TestMagicSignUpVerifyAttempts.test_signup_exhausted_after_max_wrong_attempts:
the sign-up endpoint returns EMAIL_CODE_ATTEMPT_EXHAUSTED_SIGN_UP on
the exhausting attempt.
- TestAuthenticationThrottle: exercises authentication_throttle_allows
on the plain-View redirect-flow endpoints by patching the rate down
and asserting RATE_LIMIT_EXCEEDED is appended to the redirect URL
once the per-IP budget is exceeded, for both magic-sign-in and
magic-sign-up.
Each new class clears Django cache (DRF throttle storage) and the
per-email Redis keys around every test so runs are independent.
* fix(api): clamp remaining_ttl to >=1 for verify-attempt counter EXPIRE
ri.ttl() returns 0 when the token has less than one second remaining
(Redis floors to whole seconds). The previous clamp only caught
None and < 0, so a sub-second TTL would pass through and the Lua
script's EXPIRE counter 0 would immediately delete the key — letting
an attacker bypass MAX_VERIFY_ATTEMPTS during the final second of the
token's life. Switch the comparison to <= 0.
Narrow real-world impact (sub-second window, throttle still bounds
the rate) but the cap should hold regardless of timing.
* fix(api): return HTTP response from dispatch() exception handler
BaseAPIView.dispatch() and BaseViewSet.dispatch() built the proper
error Response via handle_exception() but returned the raw exception
object instead, causing Django to raise
"TypeError: 'Exception' object is not a valid HTTP response".
Fix all six occurrences across the api, app, license and space view
bases, and add a regression test covering every affected base class.
Fixes#9157
* chore(api): add copyright header to tests/unit/views/__init__.py
The empty package init file was missing the AGPL copyright header,
failing the Copy Right Check CI (addlicense -check on all tracked
.py files).
* fix(api): harden webhook & link-unfurl SSRF (advisory clusters A/B/C)
Resolves three overlapping SSRF advisory clusters around webhook delivery
and work-item link unfurling:
- Cluster A (private-IP validation + PATCH bypass): the webhook PATCH
handler passed context={request: request} (the request object as the
dict key) so the loopback/disallowed-domain guard silently no-op'd —
now context={"request": request}. Hardened IP classification
(is_blocked_ip) to also block multicast, unspecified, CGNAT
(100.64.0.0/10), and IPv4 embedded in IPv6 transition addresses
(IPv4-mapped, NAT64, 6to4, Teredo), robust across Python versions.
- Cluster B (DNS-rebinding TOCTOU): validators resolved DNS, then
requests resolved it again at connect time. New pinned-IP client
(plane/utils/url_security.py) resolves+validates once and connects to
the validated IP literal so urllib3 performs no second lookup, while
preserving Host header, TLS SNI and certificate verification against
the real hostname.
- Cluster C (redirect SSRF): webhook delivery never follows redirects;
the link crawler follows them manually, re-resolving + re-validating +
re-pinning every hop.
Also: pin requests==2.33.0 in base.txt (imported directly; the pinning
adapter needs the >=2.32 get_connection_with_tls_context hook), and log
webhook URL-validation rejections to WebhookLog instead of swallowing
them.
Tests: new test_url_security.py (pinning, rebinding, redirect
re-validation, IP edge cases, TLS SNI) + updated link-task tests.
Full unit suite: 178 passed.
* fix(api): block OAuth avatar SSRF + add per-advisory SSRF regression tests
Verified every SSRF-class advisory against the current code. The webhook /
link / favicon reports — including the published CVE-2026-30242 and
CVE-2026-39843 and the newer "still bypassable" reports (DNS rebinding
GHSA-3856/-fgcv/-9292/-whh3/-4mjx/-6p39/-fv24/-8wvv, IP-classification gaps
GHSA-75fg, redirect GHSA-6v37/-jw6g/-mq87) — are resolved by the pinned-IP
client + hardened classifier in this branch.
The one SSRF family still unresolved was the OAuth avatar path:
download_and_upload_avatar() fetched the provider-supplied avatar_url with a
raw requests.get (no IP validation, default redirect following), so an
attacker-controlled avatar could reach internal addresses and be exfiltrated
via the static-asset endpoint (GHSA-cv9p-325g-wmv5, and the avatar hop of the
Gitea SSRF GHSA-hx79-5pj5-qh42). It now uses pinned_fetch_following_redirects,
which validates + pins every hop and blocks internal targets.
Adds test_ssrf_advisories.py: a per-advisory regression map covering webhook
IP validation, the PATCH context-key guard, webhook DNS rebinding, webhook
redirect, favicon redirect + rebinding, and OAuth avatar SSRF.
docker compose test: 199 unit tests pass.
* fix(api): address PR review feedback on the SSRF pinned client
- url_security: preserve URL-embedded credentials (user:pass@host) as Basic
Auth instead of silently dropping them when rewriting to the IP literal
(Copilot); bracket IPv6-literal hostnames in the Host header (Copilot);
add stream=True support that keeps the session open until the response is
closed, and release intermediate redirect hops.
- ip_address / work_item_link_task: treat UnicodeError (IDNA failures) from
getaddrinfo as a resolution failure, not an uncaught exception (CodeRabbit).
- authentication/adapter/base: stream the avatar download so the size cap
actually bounds memory, upload the size-bounded buffer (not response.content),
and always close the response (CodeRabbit, major).
- tests: cover auth preservation, IPv6 Host bracketing, IDNA handling, and
streamed session lifetime; drop an unused import.
docker compose test: 204 unit tests pass.
* fix: harden API token handling against rate-limit tampering and plaintext logging
- Make `allowed_rate_limit` read-only on APITokenSerializer so users can no
longer raise their own API token rate limit via PATCH (GHSA-xfgr-2x3f-g2cf).
- Stop persisting API keys in plaintext in APITokenLogMiddleware: store a
SHA-256 hash as the token identifier and redact sensitive request headers
(X-Api-Key, Authorization, Cookie) before logging (GHSA-r5p8-cj3q-38cc).
* refactor: remove MongoDB log sink and add per-log-type retention
Logs are now written to and cleared from PostgreSQL only; MongoDB is no
longer used as a log sink or archive.
- Drop the MongoDB write/archival paths from the API request logger, the
webhook log writer, and the cleanup tasks; Postgres is the sole sink.
- Cleanup tasks now hard-delete expired rows in batches via `all_objects`
(rows are removed immediately, not soft-deleted).
- Add env-backed, per-log-type retention settings: API activity logs
(API_ACTIVITY_LOG_RETENTION_DAYS, default 14), webhook logs
(WEBHOOK_LOG_RETENTION_DAYS, default 14), email logs
(EMAIL_LOG_RETENTION_DAYS, default 7). HARD_DELETE_AFTER_DAYS no longer
drives any log cleanup.
- Delete settings/mongo.py, remove MONGO_DB_* settings and the plane.mongo
loggers, and drop the pymongo dependency.
* chore: gitignore local advisories.md notes file
* fix: use keyed HMAC-SHA256 for API token log identifier
Address CodeQL "weak hashing of sensitive data" by hashing the API key with
a SECRET_KEY-keyed HMAC instead of a bare SHA-256. The identifier is a
non-reversible tokenization of a high-entropy key (not password storage);
keying it also prevents precomputing the digest from a known key value.
* chore: address review feedback on log cleanup and request logging
- process_logs accepts extra kwargs so jobs enqueued by an older release
(with a mongo_log arg) don't fail during a rolling deploy.
- Log-cleanup batch delete failures are logged and skipped rather than
aborting the run, so a single bad batch can't block the rest.
- Extend logger middleware test to assert Authorization and Cookie headers
are redacted; add a test that a failing cleanup batch is swallowed.
* fix: fall back to default when a log retention env value is invalid
Negative (or unparseable) retention values would compute a future cutoff and
delete every log row. The retention settings now fall back to their defaults
in that case via a shared `_retention_days` helper.
* chore(api): add docker compose test runner
Adds docker-compose-test.yml at the repo root that boots an isolated
postgres / valkey / rabbitmq / minio stack with health checks and tmpfs
data dirs, then runs pytest against it and exits. Includes a usage doc
under apps/api/tests/RUNNING_TESTS.md and a pointer in AGENTS.md.
Prereq: ./setup.sh (generates apps/api/.env).
Usage:
docker compose -f docker-compose-test.yml up --build \
--abort-on-container-exit --exit-code-from api-tests
docker compose -f docker-compose-test.yml down -v
* fix(api): correct bugs surfaced by the pytest suite
Five small bugs caught by enabling the pytest contract suite end-to-end.
Each is independently justifiable:
- api/serializers/cycle.py + api/views/cycle.py: CycleCreateSerializer.validate
required project_id in the request body, but the view only ever passes
it through the URL kwarg. Cycle create/update via the public API was
returning 400 "Project ID is required". Read project_id from
serializer context (passed by the view) in addition to body/instance.
- app/views/api.py: ApiTokenEndpoint.get(pk) and patch(pk) did not filter
out is_service=True tokens, so a user could read and modify service
tokens through the user token endpoint. The list mode and delete
already filter is_service=False; aligned the other two.
- bgtasks/work_item_link_task.py: validate_url_ip checked hostname before
scheme, so file:///etc/passwd raised "No hostname found" instead of
the documented "Only HTTP and HTTPS" error. Swapped the order so the
scheme guard matches the docstring intent.
- utils/path_validator.py: get_allowed_hosts used `WEB_URL or APP_BASE_URL`
so when both are configured to different hosts (the standard local
setup: WEB_URL=:8000, APP_BASE_URL=:3000), only one was added to the
allow-list. Redirects to APP_BASE_URL then had their next_path stripped
because the host wasn't allowed. Include every configured base URL.
* chore(api): align pytest tests with current behavior, clear warnings
Test-side fixes paired with the product fixes in the previous commit, plus
deprecation cleanup that drops the test run from 104 warnings to 0.
Tests:
- tests/contract/api/test_cycles.py: project fixture sets cycle_view=True;
the Project model defaults the flag to False, so cycle create/update
always tripped "Cycles are not enabled for this project".
- tests/contract/app/test_authentication.py: next_path uses "/workspaces"
(validate_next_path rejects values without a leading slash and returns
empty, which dropped the path from the redirect URL).
- tests/unit/bg_tasks/test_copy_s3_objects.py: mocked sync_with_external_service
now returns description_json; the task unconditionally writes the value
back to the Issue, and Issue.description_json is NOT NULL on UPDATE.
- tests/unit/utils/test_url.py: three length-limit tests placed the URL at
char 970+ on a single line, which contains_url truncates away as ReDoS
defense (500-char per-line cap). Restructured to keep test intent intact
while staying inside the per-line window.
Warning cleanup (104 → 0):
- settings/common.py: removed USE_L10N=True (deprecated in Django 4.0,
removed in 5.0; default is True).
- celery.py, settings/local.py, settings/production.py: pythonjsonlogger
moved jsonlogger → json; update the import / formatter path.
* test(api): add regression tests for create-project endpoint
Cover three scenarios:
- project_lead set to the creator's own user_id
- project_lead set to a different workspace member
- project_lead omitted (baseline)
The first two currently fail on preview because of a UUID coercion
bug in ProjectMember.objects.create — see follow-up commit.
* fix(api): pass project_lead_id (not User instance) when creating ProjectMember
The create-project endpoint built a ProjectMember row with
member_id=serializer.instance.project_lead, which resolves to a User
instance via Django's related descriptor instead of a UUID. Django's
UUIDField coercion then fails with AttributeError: 'User' object has
no attribute 'replace', which the generic exception handler converts
to a 400 "Please provide valid detail" — but only after the Project
row was already persisted, leaving an orphaned project without
default states.
Fix:
- Use project_lead_id (FK ID, no descriptor lookup) on both the guard
comparison and the ProjectMember creation.
- Wrap the post-save flow in transaction.atomic() so any future
exception triggers a clean rollback.
- Defer model_activity.delay() with transaction.on_commit() so the
activity log only fires after a successful commit.
- Capture the exception with log_exception() in the generic catch so
future regressions surface in api logs.
Note: a related data integrity issue exists where
ProjectCreateSerializer doesn't create a ProjectIdentifier row
(unlike its frontend counterpart). Out of scope here, will follow
up in a separate PR.
* fix(api): return 500 on unexpected errors and harden project create
Address review feedback from @sriramveeraghanta on PR #8966:
- The catch-all `except Exception` now returns 500 instead of 400.
Reusing the generic 400 response on a server-side crash was the
anti-pattern that hid the original ghost-create bug for nine months;
a 500 lets clients distinguish between "bad input" and "server fault".
- The `IntegrityError` branch no longer falls through silently when the
message is unrecognised. It re-raises so the catch-all `except` logs
the exception and returns a 500.
- `transaction.on_commit()` now schedules `model_activity.delay` via
`functools.partial` instead of a lambda, avoiding late-binding closure
semantics.
- `ProjectCreateSerializer.validate()` now rejects `project_lead`
values that are not active workspace members, surfacing the error
under the `project_lead` field key (rather than as `non_field_errors`)
so API clients can react programmatically.
* test(api): harden assertions and cover rollback / workspace-membership
Address review feedback from @sriramveeraghanta on PR #8966:
- The three existing tests now look up the created project via
`Project.objects.get(id=response.data["id"])` instead of
`.first()`. The assertion now fails for the right reason if the
wrong project is returned by the endpoint.
- New `test_create_project_with_lead_not_in_workspace_returns_400`
guards the workspace-membership validation added to
`ProjectCreateSerializer.validate()`. Expects a 400 with a
field-shaped error and zero rows persisted.
- New `test_model_activity_not_called_on_rollback` locks in the
`transaction.on_commit()` semantics: when an exception is raised
inside the atomic block (forced via mocking `State.objects.bulk_create`),
the response is 500, no Project / ProjectMember / State rows are
persisted, and the deferred `model_activity.delay` task is never
dispatched. This prevents a future refactor from silently
regressing the rollback contract.
* fix(api): mark on_commit dispatch as robust against broker failures
Address coderabbit re-review feedback on PR #8966.
Without robust=True, an exception raised by model_activity.delay
(e.g., a Celery broker outage) propagates out of the on_commit
callback and is caught by the outer `except Exception` handler,
which returns a 500 despite the project, ProjectMember rows and
default States having already been committed. The client sees a
500 and assumes the create failed — the same class of mismatch
between actual state and reported status that the original bug
exhibited, just at the post-commit phase.
Set robust=True so Django logs the dispatch failure internally
via the standard transaction logger and the response stays 201,
reflecting the persisted state.
Switch from `functools.partial` to a nested function
(`_dispatch_model_activity`) for the on_commit callable. Django's
robust on_commit logging path reads `func.__qualname__` to format
the error message; `partial` objects lack that dunder by default,
and the `functools.update_wrapper` workaround turns out to be
brittle when the wrapped callable is replaced by a Mock (which
the new regression test relies on). A nested function exposes
`__qualname__` natively, and the locals it closes over are
bound at definition time and never rebound before the callback
fires, so the late-binding-closure motivation for `partial` over
`lambda` does not apply here.
A new test, test_response_still_201_when_broker_dispatch_fails,
mirrors test_model_activity_not_called_on_rollback to lock in the
post-commit branch. It uses `@pytest.mark.django_db(transaction=True)`
so the surrounding test transaction is actually committed and the
`on_commit` callback fires (the default wrapper suppresses it via
rollback).
* fix(api): handle unrecognised IntegrityError consistently
Address coderabbit re-review feedback on PR #8966.
The previous fix used `raise` inside the IntegrityError handler with
the intent of "letting the catch-all `except Exception` below log it
and return 500". Coderabbit correctly flagged that `raise` exits the
try/except entirely — sibling except clauses don't fire — so
unrecognised integrity errors actually skipped `log_exception` and
the consistent 500 JSON shape, contradicting the stated intent.
Replicate the catch-all behaviour inline: log the exception via
`log_exception(e)` and return the same generic 500 response with
`{"error": "An unexpected error occurred"}`. The client now gets a
uniform error shape regardless of which `except` branch handled it.
---------
Co-authored-by: Jose Antonio Martinez <257598434+jamartineztelecoengineer84-dotcom@users.noreply.github.com>
* fix: add WEBHOOK_ALLOWED_HOSTS allowlist for internal webhook targets
The IP-based allowlist alone isn't practical for containerised deployments
where service IPs are dynamic. Adds a hostname-based bypass for trusted
internal services (e.g. Silo via docker-compose / k8s service DNS) and
makes the previously hardcoded ["plane.so"] domain blocklist configurable
via WEBHOOK_DISALLOWED_DOMAINS.
- validate_url accepts allowed_hosts (exact, case-insensitive match;
skips DNS lookup for trusted names)
- WebhookSerializer wires both settings through and lets allowlisted
hosts bypass the disallowed-domain check
- Exposes WEBHOOK_ALLOWED_HOSTS in aio/cli deployment env files
* fix: default WEBHOOK_DISALLOWED_DOMAINS to empty for self-hosted
* fix: pass WEBHOOK_ALLOWED_HOSTS to send-time webhook re-validation
* fix: replace IS_SELF_MANAGED toggle with explicit WEBHOOK_ALLOWED_IPS allowlist
Instead of blanket-allowing all private IPs on self-managed deployments,
webhook URL validation now blocks all private/internal IPs by default and
only permits specific networks listed in the WEBHOOK_ALLOWED_IPS env
variable (comma-separated IPs/CIDRs).
* fix: address PR review comments for webhook SSRF protection
- Sanitize error messages to avoid leaking internal details to clients
- Guard against TypeError with mixed IPv4/IPv6 allowlist networks
- Re-validate webhook URL at send time to prevent DNS-rebinding
- Add unit tests for mixed-version IP network allowlists
* fix: validate redirects in favicon fetching to prevent SSRF
The previous SSRF fix (GHSA-jcc6-f9v6-f7jw) only validated redirects for
the main page URL but not for the favicon fetch path. An attacker could
craft an HTML page with a favicon link that redirects to a private IP,
bypassing the IP validation and leaking internal network data as base64.
Extract a reusable `safe_get()` function that validates every redirect hop
against private/internal IPs and use it for both page and favicon fetches.
Resolves: GHSA-9fr2-pprw-pp9j
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* fix: address PR review feedback for SSRF favicon fix
- Fix off-by-one in redirect limit: only raise RuntimeError when the
response is still a redirect after MAX_REDIRECTS hops, not when the
final response is a successful 200
- Return final URL from safe_get() so favicon href resolution uses the
correct origin after redirects instead of the original URL
- Add unit tests for validate_url_ip and safe_get covering private IP
blocking, redirect-following, and redirect limit enforcement
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* refactor: rename IssueUserProperty to ProjectUserProperty and update related references across the codebase
* migrate: move issue user properties to project user properties and update related fields and constraints
* refactor: rename IssueUserPropertySerializer and IssueUserDisplayPropertyEndpoint to ProjectUserPropertySerializer and ProjectUserDisplayPropertyEndpoint, updating all related references
* fix: enhance ProjectUserDisplayPropertyEndpoint to handle missing properties by creating new entries and improve response handling
* fix: correct formatting in migration for ProjectUserProperty model options
* migrate: add migration to update existing non-service API tokens to remove workspace association
* migrate: refine migration to update existing non-service API tokens by excluding bot users from workspace removal
* chore: changed the project sort order in project user property
* chore: remove allowed_rate_limit from APIToken
* chore: updated user-properties endpoint for frontend
* chore: removed the extra projectuserproperty
* chore: updated the migration file
* chore: code refactor
* fix: type error
---------
Co-authored-by: NarayanBavisetti <narayan3119@gmail.com>
Co-authored-by: sangeethailango <sangeethailango21@gmail.com>
Co-authored-by: vamsikrishnamathala <matalav55@gmail.com>
Co-authored-by: Anmol Singh Bhatia <anmolsinghbhatia@plane.so>
* chore(deps): upgrade psycopg packages to version 3.3.0
* chore: update Python version to 3.12.x in CI workflow
* refactor: clean up imports and improve code formatting across multiple files
- Update S3Storage to use configurable expiration time
- Default remains 3600 seconds (1 hour) for backward compatibility
- Add comprehensive unit tests with mocked S3 client
- Update .env.example with documentation and examples
* feat: set default owner for cycle creation if not provided
* Updated CycleListCreateAPIEndpoint to assign the current user as the owner when the 'owned_by' field is not included in the request data.
* Enhanced the CycleCreateSerializer initialization to ensure proper ownership assignment during cycle creation.
* feat: add comprehensive tests for Cycle API endpoints
* Introduced a new test suite for Cycle API endpoints, covering creation, retrieval, updating, and deletion of cycles.
* Implemented tests for various scenarios including successful operations, invalid data handling, and conflict resolution with external IDs.
* Enhanced test coverage for listing cycles with different view filters and verifying cycle metrics annotations.
* feat: enhance CycleCreateSerializer to include ownership assignment
* Added 'owned_by' field to CycleCreateSerializer to specify the user who owns the cycle.
* Updated CycleListCreateAPIEndpoint to remove redundant ownership assignment logic, relying on the serializer to handle default ownership.
* Ensured that if 'owned_by' is not provided, it defaults to the current user during cycle creation.
* fix: correct assertion syntax in CycleListCreateAPIEndpoint tests
* Updated the assertion in the test for successful cycle creation to use the correct syntax for checking the response status code.
* Ensured that the test accurately verifies the expected behavior of the API endpoint.
* chore: update psycopg dependencies to version 3.2.9 in base requirements
* refactor: clean up unused imports across multiple files
* chore: update lxml dependency to version 6.0.0 in base requirements
* style: improve code readability by breaking long lines into multiple lines across several files
* style: enhance readability by breaking long lines in ModuleSerializer docstring
