* 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
* chore: update completed_at logic updation in Issue save method
* fix: update error handling
Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com>
* fix: use StateGroup
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
---------
Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
is_workspace_admin in ProjectMemberViewSet.partial_update was derived
from the target member's workspace role, not the requester's. When the
target happened to be a workspace admin, all three project-role guards
(L231/238/247) were bypassed regardless of who was making the request,
allowing a non-admin requester to re-role a workspace admin's project
membership. Compute is_workspace_admin from the requester instead and
keep the target's workspace role under a distinct name for the existing
new-role-vs-workspace-role cap.
`ProjectViewSet.partial_update`, `BulkEstimatePointEndpoint.partial_update`,
and `WorkspaceUserProfileEndpoint.get` previously fetched objects by primary
key alone after a workspace-scoped permission check, allowing an authenticated
caller to act on resources belonging to other workspaces by supplying a
foreign UUID with their own workspace slug in the URL.
- Project partial_update: scope `Project.objects.get` by `workspace__slug`,
matching the existing pattern in `destroy`.
- Bulk estimate partial_update: scope `Estimate.objects.get` by
`workspace__slug` and `project_id`, matching `retrieve` and `destroy`.
- Workspace user profile: require the target `user_id` to be an active
member of the requested workspace before returning email and other PII.
* fix: filter out soft-deleted states from API endpoints
- Add deleted_at__isnull=True filter to StateListCreateAPIEndpoint.get_queryset()
- Add deleted_at__isnull=True filter to StateDetailAPIEndpoint.get_queryset()
- Prevents soft-deleted states from reappearing in UI after navigation
- Fixes#8829
* Fix: exclude issues linked to soft-deleted states
* fix: sanitize filenames in upload paths to prevent path traversal (GHSA-v57h-5999-w7xp)
Add server-side filename sanitization across all file upload endpoints
to prevent path traversal sequences (../) in user-supplied filenames
from being incorporated into S3 object keys. While S3 keys are flat
strings and not vulnerable to filesystem traversal, this adds
defense-in-depth and prevents S3 key pollution.
Changes:
- Add sanitize_filename() utility in path_validator.py
- Sanitize filenames in get_upload_path() for FileAsset and IssueAttachment models
- Sanitize name parameter in all upload view endpoints
* fix: address PR review feedback on filename sanitization
- Remove unused `import re`
- Normalize backslashes to forward slashes before os.path.basename()
so Windows-style paths (e.g. ..\..\..\evil.txt) are handled on POSIX
- Strip whitespace before removing leading dots so " .env" is caught
- Return None instead of "unnamed" for empty input so existing
`if not name` validation guards remain effective
- Add `or "unnamed"` fallback at call sites that lack a name guard
* fix: use random hex name as fallback in get_upload_path instead of "unnamed"
* fix: resolve ruff E501 line too long in DuplicateAssetEndpoint
* 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
WorkspaceFileAssetEndpoint had no authorization checks beyond
authentication, allowing any logged-in user to create, read, patch,
and delete assets in any workspace by slug. DuplicateAssetEndpoint
only authorized the destination workspace, letting users copy assets
from workspaces they don't belong to.
Add @allow_permission decorators to all WorkspaceFileAssetEndpoint
methods and scope DuplicateAssetEndpoint's source asset lookup to
workspaces where the caller is an active member.
Ref: GHSA-qw87-v5w3-6vxx
When patching instance configuration values, the raw values from
request.data were used directly without sanitization. This adds:
- Whitespace stripping via str().strip() to prevent leading/trailing
spaces from being stored
- Explicit None handling so that null values become empty strings
instead of the literal string "None"
* fix: prevent ORM field injection via segment parameter in analytics (GHSA-93x3-ghh7-72j3)
Centralize analytics field allowlists into VALID_ANALYTICS_FIELDS and
VALID_YAXIS constants in analytics_plot.py. Add defense-in-depth
validation in build_graph_plot() and extract_axis() so no caller can
pass arbitrary field references to Django F() expressions. Add missing
segment validation to SavedAnalyticEndpoint. Also fixes ExportAnalytics
using "estimate_point" instead of "estimate_point__value".
* fix: address PR review - remove unused imports and validate stored query params
Remove unused VALID_ANALYTICS_FIELDS and VALID_YAXIS imports from
analytic_plot_export.py. Add x_axis/y_axis allowlist validation in
SavedAnalyticEndpoint for stored query_dict values to prevent 500
errors from malformed saved analytics.
* 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>
Restrict role modification in ProjectMemberViewSet.partial_update to
Admins only and enforce that requesters cannot modify or assign roles
equal to or higher than their own. Previously, Guests could demote
Admins by exploiting a missing lower-bound check on role changes.
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The bulk update date endpoint fetched issues by ID without filtering
by workspace or project, allowing any authenticated project member to
modify start_date and target_date of issues in any workspace/project
across the entire instance (IDOR - CWE-639).
Scoped the query to include workspace__slug and project_id filters,
consistent with other issue endpoints in the codebase.
Ref: GHSA-4q54-h4x9-m329
Fixes#6746
API-driven issue updates (PUT update, PUT create-via-upsert, PATCH) were
missing `model_activity.delay()` calls, so webhooks were never dispatched
for changes made through the API. The web UI paths already include these
calls (e.g. in `post()` at L475), but the `put()` and `partial_update()`
methods only called `issue_activity.delay()`.
This adds `model_activity.delay()` immediately after each existing
`issue_activity.delay()` in these three code paths, using the same
signature as the existing call in `post()`.
Tested on Plane CE v1.2.1 self-hosted: API PATCH triggers
`webhook_send_task` in the Celery worker, confirming webhook delivery.
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* chore: replace Discord references with Forum links
* chore: migrate help and community CTAs from Discord to Forum
* refactor: replace Discord icons with lucide MessageSquare
* chore: rename Discord labels and keys to Forum
* chore: remove obsolete Discord icon component
* chore: update Discord references to Forum in templates
* chore: code refactoring
* feat: enhance authentication logging with detailed error and info messages
- Added logging for various authentication events in the Adapter and its subclasses, including email validation, user existence checks, and password strength validation.
- Implemented error handling for GitHub OAuth email retrieval, ensuring proper logging of unexpected responses and missing primary emails.
- Updated logging configuration in local and production settings to include a dedicated logger for authentication events.
* chore: address copilot comments
* chore: addressed some additional comments
* chore: update log
* fix: lint
* chore: updated the logic for page version task
* chore: updated the html variable
* chore: handled the exception
* chore: changed the function name
* chore: added a custom variable
