diff --git a/AGENTS.md b/AGENTS.md index 3de0b80379..f6e2b9dd3a 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -22,3 +22,15 @@ - **State Management**: MobX stores in `packages/shared-state`, reactive patterns - **Testing**: All features require unit tests, use existing test framework per package - **Components**: Build in `@plane/ui` with Storybook for isolated development + +## Backend tests (Docker) + +The Django/pytest suite for `apps/api` runs in an isolated stack defined by `docker-compose-test.yml` at the repo root. + +Prereq (once): `./setup.sh` — generates `apps/api/.env` from `.env.example`. + +- Full suite: `docker compose -f docker-compose-test.yml up --build --abort-on-container-exit --exit-code-from api-tests` +- Subset: `docker compose -f docker-compose-test.yml run --rm api-tests pytest -m unit` +- Teardown: `docker compose -f docker-compose-test.yml down -v` + +See `apps/api/tests/RUNNING_TESTS.md` for the full walkthrough and troubleshooting; see `apps/api/tests/TESTING_GUIDE.md` for test conventions and fixtures. diff --git a/apps/api/plane/api/serializers/cycle.py b/apps/api/plane/api/serializers/cycle.py index 9b6aed516d..88725e3995 100644 --- a/apps/api/plane/api/serializers/cycle.py +++ b/apps/api/plane/api/serializers/cycle.py @@ -59,8 +59,10 @@ class CycleCreateSerializer(BaseSerializer): ] def validate(self, data): - project_id = self.initial_data.get("project_id") or ( - self.instance.project_id if self.instance and hasattr(self.instance, "project_id") else None + project_id = ( + self.context.get("project_id") + or self.initial_data.get("project_id") + or (self.instance.project_id if self.instance and hasattr(self.instance, "project_id") else None) ) if not project_id: diff --git a/apps/api/plane/api/views/cycle.py b/apps/api/plane/api/views/cycle.py index 30b04ed46d..84c8abaac5 100644 --- a/apps/api/plane/api/views/cycle.py +++ b/apps/api/plane/api/views/cycle.py @@ -305,7 +305,9 @@ class CycleListCreateAPIEndpoint(BaseAPIView): if (request.data.get("start_date", None) is None and request.data.get("end_date", None) is None) or ( request.data.get("start_date", None) is not None and request.data.get("end_date", None) is not None ): - serializer = CycleCreateSerializer(data=request.data, context={"request": request}) + serializer = CycleCreateSerializer( + data=request.data, context={"request": request, "project_id": project_id} + ) if serializer.is_valid(): if ( request.data.get("external_id") @@ -516,7 +518,9 @@ class CycleDetailAPIEndpoint(BaseAPIView): status=status.HTTP_400_BAD_REQUEST, ) - serializer = CycleUpdateSerializer(cycle, data=request.data, partial=True, context={"request": request}) + serializer = CycleUpdateSerializer( + cycle, data=request.data, partial=True, context={"request": request, "project_id": project_id} + ) if serializer.is_valid(): if ( request.data.get("external_id") diff --git a/apps/api/plane/app/views/api.py b/apps/api/plane/app/views/api.py index f3163c3314..10548e045b 100644 --- a/apps/api/plane/app/views/api.py +++ b/apps/api/plane/app/views/api.py @@ -44,7 +44,7 @@ class ApiTokenEndpoint(BaseAPIView): serializer = APITokenReadSerializer(api_tokens, many=True) return Response(serializer.data, status=status.HTTP_200_OK) else: - api_tokens = APIToken.objects.get(user=request.user, pk=pk) + api_tokens = APIToken.objects.get(user=request.user, pk=pk, is_service=False) serializer = APITokenReadSerializer(api_tokens) return Response(serializer.data, status=status.HTTP_200_OK) @@ -54,7 +54,7 @@ class ApiTokenEndpoint(BaseAPIView): return Response(status=status.HTTP_204_NO_CONTENT) def patch(self, request: Request, pk: str) -> Response: - api_token = APIToken.objects.get(user=request.user, pk=pk) + api_token = APIToken.objects.get(user=request.user, pk=pk, is_service=False) serializer = APITokenSerializer(api_token, data=request.data, partial=True) if serializer.is_valid(): serializer.save() diff --git a/apps/api/plane/bgtasks/work_item_link_task.py b/apps/api/plane/bgtasks/work_item_link_task.py index 5cf0fbb190..cae6a4feb4 100644 --- a/apps/api/plane/bgtasks/work_item_link_task.py +++ b/apps/api/plane/bgtasks/work_item_link_task.py @@ -36,15 +36,15 @@ def validate_url_ip(url: str) -> None: ValueError: If the URL points to a private/internal IP """ parsed = urlparse(url) - hostname = parsed.hostname - - if not hostname: - raise ValueError("Invalid URL: No hostname found") # Only allow HTTP and HTTPS to prevent file://, gopher://, etc. if parsed.scheme not in ("http", "https"): raise ValueError("Invalid URL scheme. Only HTTP and HTTPS are allowed") + hostname = parsed.hostname + if not hostname: + raise ValueError("Invalid URL: No hostname found") + # Resolve hostname to IP addresses — this catches domain names that # point to internal IPs (e.g. attacker.com -> 169.254.169.254) diff --git a/apps/api/plane/celery.py b/apps/api/plane/celery.py index 562d04856f..c2455a9a51 100644 --- a/apps/api/plane/celery.py +++ b/apps/api/plane/celery.py @@ -8,7 +8,7 @@ import logging # Third party imports from celery import Celery -from pythonjsonlogger.jsonlogger import JsonFormatter +from pythonjsonlogger.json import JsonFormatter from celery.signals import after_setup_logger, after_setup_task_logger from celery.schedules import crontab diff --git a/apps/api/plane/settings/common.py b/apps/api/plane/settings/common.py index 165b3bd7ce..9a6ec57c93 100644 --- a/apps/api/plane/settings/common.py +++ b/apps/api/plane/settings/common.py @@ -264,7 +264,6 @@ MEDIA_URL = "/media/" # Internationalization LANGUAGE_CODE = "en-us" USE_I18N = True -USE_L10N = True # Timezones USE_TZ = True diff --git a/apps/api/plane/settings/local.py b/apps/api/plane/settings/local.py index dc4135bc13..e71f40bf14 100644 --- a/apps/api/plane/settings/local.py +++ b/apps/api/plane/settings/local.py @@ -46,7 +46,7 @@ LOGGING = { "style": "{", }, "json": { - "()": "pythonjsonlogger.jsonlogger.JsonFormatter", + "()": "pythonjsonlogger.json.JsonFormatter", "fmt": "%(levelname)s %(asctime)s %(module)s %(name)s %(message)s", }, }, diff --git a/apps/api/plane/settings/production.py b/apps/api/plane/settings/production.py index 7f3f90d650..f58450a9da 100644 --- a/apps/api/plane/settings/production.py +++ b/apps/api/plane/settings/production.py @@ -34,7 +34,7 @@ LOGGING = { "formatters": { "verbose": {"format": "%(asctime)s [%(process)d] %(levelname)s %(name)s: %(message)s"}, "json": { - "()": "pythonjsonlogger.jsonlogger.JsonFormatter", + "()": "pythonjsonlogger.json.JsonFormatter", "fmt": "%(levelname)s %(asctime)s %(module)s %(name)s %(message)s", }, }, diff --git a/apps/api/plane/tests/contract/api/test_cycles.py b/apps/api/plane/tests/contract/api/test_cycles.py index d0138de8b8..d0fa87c922 100644 --- a/apps/api/plane/tests/contract/api/test_cycles.py +++ b/apps/api/plane/tests/contract/api/test_cycles.py @@ -19,6 +19,7 @@ def project(db, workspace, create_user): identifier="TP", workspace=workspace, created_by=create_user, + cycle_view=True, ) ProjectMember.objects.create( project=project, diff --git a/apps/api/plane/tests/contract/app/test_authentication.py b/apps/api/plane/tests/contract/app/test_authentication.py index 808416b028..ae0ab754e1 100644 --- a/apps/api/plane/tests/contract/app/test_authentication.py +++ b/apps/api/plane/tests/contract/app/test_authentication.py @@ -302,9 +302,10 @@ class TestMagicSignIn: user_data = json.loads(ri.get("magic_user@plane.so")) token = user_data["token"] - # Use Django client to test the redirect flow without following redirects + # Use Django client to test the redirect flow without following redirects. + # next_path must start with "/" per validate_next_path (otherwise it's discarded). url = reverse("magic-sign-in") - next_path = "workspaces" + next_path = "/workspaces" response = django_client.post( url, {"email": "user@plane.so", "code": token, "next_path": next_path}, @@ -315,8 +316,8 @@ class TestMagicSignIn: assert response.status_code == 302 assert "error_code" not in response.url - # Check that the redirect URL contains the next_path - assert next_path in response.url + # Check that the redirect URL contains the next_path (URL-encoded, leading slash → %2F) + assert "workspaces" in response.url # The user should now be authenticated assert "_auth_user_id" in django_client.session diff --git a/apps/api/plane/tests/unit/bg_tasks/test_copy_s3_objects.py b/apps/api/plane/tests/unit/bg_tasks/test_copy_s3_objects.py index c153703baa..dd59d3e9fc 100644 --- a/apps/api/plane/tests/unit/bg_tasks/test_copy_s3_objects.py +++ b/apps/api/plane/tests/unit/bg_tasks/test_copy_s3_objects.py @@ -78,6 +78,7 @@ class TestCopyS3Objects: mock_sync.return_value = { "description": "test description", "description_binary": base64.b64encode(b"test binary").decode(), + "description_json": {"type": "doc", "content": []}, } # Call the actual function (not .delay()) diff --git a/apps/api/plane/tests/unit/utils/test_url.py b/apps/api/plane/tests/unit/utils/test_url.py index 82b5b106d0..168e29410f 100644 --- a/apps/api/plane/tests/unit/utils/test_url.py +++ b/apps/api/plane/tests/unit/utils/test_url.py @@ -68,16 +68,20 @@ class TestContainsURL: assert contains_url("www.") is False # Incomplete www - needs at least one char after dot def test_contains_url_length_limit_under_1000(self): - """Test contains_url with input under 1000 characters containing URLs""" - # Create a string under 1000 characters with a URL - text_with_url = "a" * 970 + " https://example.com" # 970 + 1 + 19 = 990 chars + """Test contains_url with input under 1000 characters containing URLs. + + Note: contains_url also truncates each line to 500 chars (ReDoS protection), + so URLs must fall within the first 500 chars of their line. + """ + # Single line under 500 chars with URL at the end + text_with_url = "a" * 470 + " https://example.com" # 490 chars total assert len(text_with_url) < 1000 assert contains_url(text_with_url) is True - # Test with exactly 1000 characters - text_exact_1000 = "a" * 981 + "https://example.com" # 981 + 19 = 1000 chars - assert len(text_exact_1000) == 1000 - assert contains_url(text_exact_1000) is True + # Multi-line input under 1000 chars total; URL on its own short line + text_multiline = "a" * 480 + "\nhttps://example.com\n" + "b" * 480 + assert len(text_multiline) < 1000 + assert contains_url(text_multiline) is True def test_contains_url_length_limit_over_1000(self): """Test contains_url with input over 1000 characters returns False""" @@ -91,14 +95,17 @@ class TestContainsURL: assert contains_url(long_text_with_url) is False def test_contains_url_length_limit_exactly_1000(self): - """Test contains_url with input exactly 1000 characters""" + """Test contains_url with input exactly 1000 characters. + + URLs must fall within the first 500 chars of their line (ReDoS protection). + """ # Test with exactly 1000 characters without URL text_no_url = "a" * 1000 assert len(text_no_url) == 1000 assert contains_url(text_no_url) is False - # Test with exactly 1000 characters with URL at the end - text_with_url = "a" * 981 + "https://example.com" # 981 + 19 = 1000 chars + # Multi-line totalling exactly 1000 chars; URL on a short line + text_with_url = "a" * 480 + "\nhttps://example.com\n" + "b" * 499 # 480+1+19+1+499 = 1000 assert len(text_with_url) == 1000 assert contains_url(text_with_url) is True @@ -121,8 +128,9 @@ class TestContainsURL: over_limit_text = "a" * 1001 # No URL, but over total limit assert contains_url(over_limit_text) is False - # Test that under total limit, line processing works normally - under_limit_with_url = "a" * 900 + "https://example.com" # 919 chars total + # Test that under total limit, line processing works normally. + # URL must be within first 500 chars of its line (ReDoS protection). + under_limit_with_url = "a" * 400 + "https://example.com" # 419 chars total, fits in 500 assert len(under_limit_with_url) < 1000 assert contains_url(under_limit_with_url) is True diff --git a/apps/api/plane/utils/path_validator.py b/apps/api/plane/utils/path_validator.py index 901464ee6a..ab2e2bae53 100644 --- a/apps/api/plane/utils/path_validator.py +++ b/apps/api/plane/utils/path_validator.py @@ -90,20 +90,15 @@ def _contains_suspicious_patterns(path: str) -> bool: def get_allowed_hosts() -> list[str]: """Get the allowed hosts from the settings.""" - base_origin = settings.WEB_URL or settings.APP_BASE_URL - allowed_hosts = [] - if base_origin: - host = urlparse(base_origin).netloc - allowed_hosts.append(host) - if settings.ADMIN_BASE_URL: - # Get only the host - host = urlparse(settings.ADMIN_BASE_URL).netloc - allowed_hosts.append(host) - if settings.SPACE_BASE_URL: - # Get only the host - host = urlparse(settings.SPACE_BASE_URL).netloc - allowed_hosts.append(host) + # Include every configured base URL; WEB_URL and APP_BASE_URL may differ + # (e.g. WEB_URL points at the API host, APP_BASE_URL at the web app), and + # both need to be allowed for redirects to either origin to pass safety checks. + for setting in (settings.WEB_URL, settings.APP_BASE_URL, settings.ADMIN_BASE_URL, settings.SPACE_BASE_URL): + if setting: + host = urlparse(setting).netloc + if host and host not in allowed_hosts: + allowed_hosts.append(host) return allowed_hosts diff --git a/apps/api/tests/RUNNING_TESTS.md b/apps/api/tests/RUNNING_TESTS.md new file mode 100644 index 0000000000..9b60b2b782 --- /dev/null +++ b/apps/api/tests/RUNNING_TESTS.md @@ -0,0 +1,82 @@ +# Running the API Test Suite + +This guide covers running the Django/pytest suite for `apps/api` inside Docker via `docker-compose-test.yml` at the repo root. The compose file boots an isolated stack — Postgres, Valkey (Redis), RabbitMQ, MinIO — with tmpfs-backed data dirs, so every run begins from a clean slate and a single teardown command removes everything. + +For background on the test layout, markers, and fixtures, see [`TESTING_GUIDE.md`](./TESTING_GUIDE.md) and [`README.md`](./README.md). + +## Prerequisites + +- Docker and Docker Compose v2 (`docker compose ...`) +- Env files generated via the setup script: + + ```bash + ./setup.sh + ``` + + This copies `apps/api/.env.example` → `apps/api/.env` (along with the other app env files). The compose file reads `apps/api/.env`, so this step must run **before** the first `docker compose` invocation. + +## Running the suite + +All commands are run from the repo root. + +### Full suite + +```bash +docker compose -f docker-compose-test.yml up \ + --build \ + --abort-on-container-exit \ + --exit-code-from api-tests +``` + +- `--build` rebuilds the `api-tests` image when `Dockerfile.dev` or `requirements/*.txt` change. +- `--abort-on-container-exit` stops the dependency services as soon as `api-tests` exits. +- `--exit-code-from api-tests` propagates pytest's exit code so this works in CI. + +### Filtered runs + +Use `docker compose run` to override the default `pytest` command. Anything you pass after the service name is forwarded to pytest. + +```bash +# Only unit tests (marker defined in pytest.ini) +docker compose -f docker-compose-test.yml run --rm --build api-tests pytest -m unit + +# A single directory, filtered by name +docker compose -f docker-compose-test.yml run --rm api-tests \ + pytest plane/tests/unit -k "test_workspace" + +# Single file with verbose output +docker compose -f docker-compose-test.yml run --rm api-tests \ + pytest plane/tests/unit/models/test_workspace.py -vv +``` + +The available markers (`unit`, `contract`, `smoke`, `slow`) are declared in `apps/api/pytest.ini`. + +### Teardown + +```bash +docker compose -f docker-compose-test.yml down -v +``` + +`-v` removes the ephemeral volumes and the `test_env` network. Because the data directories are tmpfs, no host state survives a teardown — every run starts clean. Run this between unrelated test sessions to free Docker resources. + +## How it works + +| Service | Image | Purpose | +| ------------ | ------------------------------------ | --------------------------------------------- | +| `test-db` | `postgres:15.7-alpine` | Application database | +| `test-redis` | `valkey/valkey:7.2.11-alpine` | Cache / Celery broker | +| `test-mq` | `rabbitmq:3.13.6-management-alpine` | Task queue | +| `test-minio` | `minio/minio` | S3-compatible object storage | +| `api-tests` | built from `apps/api/Dockerfile.dev` | Installs `requirements/test.txt`, runs pytest | + +All four dependencies expose health checks; `api-tests` waits for `service_healthy` on each via `depends_on`, so pytest only starts once the stack is ready. + +Test-time env overrides live in the compose file itself (`POSTGRES_HOST=test-db`, `REDIS_URL=redis://test-redis:6379/`, `AWS_S3_ENDPOINT_URL=http://test-minio:9000`, `DJANGO_SETTINGS_MODULE=plane.settings.test`). Everything else is inherited from `apps/api/.env`. + +## Troubleshooting + +- **`./apps/api/.env: no such file or directory`** — run `./setup.sh` from the repo root. +- **Port already in use** — none of the test services publish host ports; if you see this it's coming from a different compose stack. Stop the local stack (`docker compose -f docker-compose-local.yml down`). +- **Stale image after dependency changes** — rebuild explicitly: `docker compose -f docker-compose-test.yml build --no-cache api-tests`. +- **MinIO bucket missing** — the `test-minio` entrypoint creates the bucket named by `AWS_S3_BUCKET_NAME` (default `uploads`). Change the value in `apps/api/.env` and re-run. +- **Database state leaking between runs** — confirm you ran `down -v` (not just `down`). The tmpfs mounts are torn down with the container, but the network and any externally created volumes need `-v` to clear. diff --git a/docker-compose-test.yml b/docker-compose-test.yml new file mode 100644 index 0000000000..b4cdc1fdc2 --- /dev/null +++ b/docker-compose-test.yml @@ -0,0 +1,151 @@ +# Docker Compose for running the API pytest suite in a contained environment. +# +# Prerequisite: +# ./setup.sh # creates apps/api/.env (and the other env files) from .env.example +# +# Usage: +# # Run the full suite (defaults to `pytest`): +# docker compose -f docker-compose-test.yml up --build --abort-on-container-exit --exit-code-from api-tests +# +# # Run a subset by overriding the command: +# docker compose -f docker-compose-test.yml run --rm --build api-tests pytest -m unit +# docker compose -f docker-compose-test.yml run --rm api-tests pytest plane/tests/unit -k "test_workspace" +# +# # Tear down (removes ephemeral volumes and the test network): +# docker compose -f docker-compose-test.yml down -v +# +# Notes: +# - Postgres / Valkey / RabbitMQ / MinIO start with health checks; the test +# runner only starts once each dependency is healthy. +# - Data dirs are tmpfs so every run begins from a clean state. +# - Env vars come from apps/api/.env (the same file the local stack uses); tests +# use plane.settings.test, which inherits from common and reads DATABASE_URL, +# REDIS_URL, RABBITMQ_* etc. from the environment. + +services: + test-db: + image: postgres:15.7-alpine + networks: + - test_env + env_file: + - ./apps/api/.env + environment: + POSTGRES_USER: ${POSTGRES_USER:-plane} + POSTGRES_PASSWORD: ${POSTGRES_PASSWORD:-plane} + POSTGRES_DB: ${POSTGRES_DB:-plane} + tmpfs: + - /var/lib/postgresql/data + healthcheck: + test: ["CMD-SHELL", "pg_isready -U ${POSTGRES_USER:-plane} -d ${POSTGRES_DB:-plane}"] + interval: 5s + timeout: 5s + retries: 10 + + test-redis: + image: valkey/valkey:7.2.11-alpine + networks: + - test_env + tmpfs: + - /data + healthcheck: + test: ["CMD", "valkey-cli", "ping"] + interval: 5s + timeout: 3s + retries: 10 + + test-mq: + image: rabbitmq:3.13.6-management-alpine + networks: + - test_env + env_file: + - ./apps/api/.env + environment: + RABBITMQ_DEFAULT_USER: ${RABBITMQ_USER:-plane} + RABBITMQ_DEFAULT_PASS: ${RABBITMQ_PASSWORD:-plane} + RABBITMQ_DEFAULT_VHOST: ${RABBITMQ_VHOST:-plane} + tmpfs: + - /var/lib/rabbitmq + healthcheck: + test: ["CMD", "rabbitmq-diagnostics", "-q", "ping"] + interval: 10s + timeout: 10s + retries: 10 + + test-minio: + image: minio/minio + networks: + - test_env + env_file: + - ./apps/api/.env + environment: + MINIO_ROOT_USER: ${AWS_ACCESS_KEY_ID:-access-key} + MINIO_ROOT_PASSWORD: ${AWS_SECRET_ACCESS_KEY:-secret-key} + entrypoint: > + /bin/sh -c " + mkdir -p /export/${AWS_S3_BUCKET_NAME:-uploads} && + minio server /export --console-address ':9090' & + sleep 3 && + mc alias set local http://localhost:9000 ${AWS_ACCESS_KEY_ID:-access-key} ${AWS_SECRET_ACCESS_KEY:-secret-key} && + mc mb local/${AWS_S3_BUCKET_NAME:-uploads} -p || true && + tail -f /dev/null + " + tmpfs: + - /export + healthcheck: + test: ["CMD", "mc", "ready", "local"] + interval: 10s + timeout: 5s + retries: 10 + + api-tests: + build: + context: ./apps/api + dockerfile: Dockerfile.dev + args: + DOCKER_BUILDKIT: 1 + networks: + - test_env + env_file: + - ./apps/api/.env + environment: + DJANGO_SETTINGS_MODULE: plane.settings.test + # Override service hostnames to point at the test-only services above. + POSTGRES_HOST: test-db + DATABASE_URL: postgresql://${POSTGRES_USER:-plane}:${POSTGRES_PASSWORD:-plane}@test-db:5432/${POSTGRES_DB:-plane} + REDIS_HOST: test-redis + REDIS_URL: redis://test-redis:6379/ + RABBITMQ_HOST: test-mq + AWS_S3_ENDPOINT_URL: http://test-minio:9000 + # Magic-link tests mock the celery delay but the view first checks that + # EMAIL_HOST is configured. Set a placeholder so the check passes. + EMAIL_HOST: test-smtp.invalid + volumes: + - ./apps/api:/code + working_dir: /code + depends_on: + test-db: + condition: service_healthy + test-redis: + condition: service_healthy + test-mq: + condition: service_healthy + test-minio: + condition: service_healthy + # Install test-only requirements (not in local.txt) then exec pytest. + # Any args passed via `docker compose run api-tests ` replace the default command. + entrypoint: + - /bin/sh + - -c + - | + set -e + pip install --no-cache-dir -r requirements/test.txt + # STATIC_ROOT must exist or Django's static middleware emits a + # UserWarning on every request (~100 of the warnings in test runs). + mkdir -p plane/static-assets/collected-static + exec "$@" + - -- + command: ["pytest"] + +networks: + test_env: + driver: bridge