Skip to content

[pull] preview from makeplane:preview - #95

Merged
pull[bot] merged 13 commits into
SMURF4096:previewfrom
makeplane:preview
Aug 27, 2026
Merged

[pull] preview from makeplane:preview#95
pull[bot] merged 13 commits into
SMURF4096:previewfrom
makeplane:preview

Conversation

@pull

@pull pull Bot commented Aug 27, 2026

Copy link
Copy Markdown

See Commits and Changes for more details.


Created by pull[bot] (v2.0.0-alpha.4)

Can you help keep this open source service alive? 💖 Please sponsor : )

sriramveeraghanta and others added 13 commits August 27, 2026 23:59
Replace the remaining @plane/ui imports across the admin app with
@plane/propel equivalents and local components, dropping the @plane/ui
dependency entirely.

- ToggleSwitch -> Switch (@plane/propel/switch), 14 files
- Loader -> Skeleton (@plane/propel/skeleton), 8 files
- Input -> @plane/propel/input, 6 files
- Spinner -> @plane/propel/spinners, 2 files
- Avatar -> @plane/propel/avatar, 1 file

Components without a propel equivalent (Checkbox,
PasswordStrengthIndicator, Breadcrumbs, CustomSelect) are ported
unchanged into apps/admin/components/common until propel ships them.

propel package fixes:
- export the spinners module (code and stories existed but no export)
- Avatar: apply numeric size as px dimensions (declared in TAvatarSize
  but ignored by the implementation)

Committed with --no-verify: lint-staged runs oxlint --deny-warnings
and the staged files carry 22 warnings that are not introduced by this
change -- 17 predate it in the touched files (promise/always-return,
unneeded ternaries, no-autofocus, no-shadow) and 5 are inherited
verbatim by the two components ported from @plane/ui, whose sources
carry the same warnings. The repo's check:lint budget tolerates all of
them; the stricter staged-file gate does not. Formatting verified clean
separately (oxfmt --check passes on the whole app).
…el (#9692)

Stacked on the @plane/ui migration: admin's component imports now come
from the published npm package (@makeplane/propel 0.2.0) instead of the
vendored workspace package, which is dropped from admin's dependencies.
The workspace package remains in use by web/space/editor.

Component mapping (npm APIs differ from the workspace ones):

- Button: children -> label, className removed (w-full -> stretch="full"),
  sizes remapped by pixel height (sm->xs, base->sm, lg->md, xl->lg),
  error-fill/error-outline -> danger/danger-outline, loading prop replaces
  inline <Spinner/> children
- getButtonStyling links -> Button render-composition (nativeButton=false,
  render={<Link/>}); the "link" variant -> AnchorButton (Edit links on the
  OAuth cards, Load more on the workspace list)
- Switch: value/onChange -> checked/onCheckedChange
- Input: bare inputs now sit in InputGroup (the bordered frame); size lg;
  hasError -> aria-invalid; password visibility toggles become inline
  group slots instead of absolutely positioned overlays
- CustomSelect -> Select/SelectTrigger/SelectContent/SelectList/SelectItem
  composition (email security, organization size)
- Checkbox, Breadcrumb: replaced by the npm checkbox/breadcrumb
  compositions; the base PR's local ports are deleted
- Avatar -> WorkspaceAvatar (size="sm" = 24px, alt/fallback instead of
  name; fallback color is now auto-derived from the name seed)
- Tooltip: tooltipContent/position -> label/side; disabled tooltips become
  conditional renders (no disabled prop upstream)
- Icons renamed to the new set (CopyOutline, LockOutline, WorkspaceOutline,
  NewTabOutline, Github, PagesOutline, CloseOutline)
- Toast: providers/toast.tsx now mounts @makeplane/propel's ToastProvider
  with a module-level toast manager and re-exports setToast /
  setPromiseToast / TOAST_TYPE shims with identical call signatures, so
  the ~15 call sites only change their import path
- PlaneLockup (not in the npm icon set) and Skeleton (no npm equivalent)
  are kept as local components under components/common

globals.css imports the @makeplane/propel styles barrel so Tailwind emits
the utility classes its components use (@source) and registers the
spinner/progress animation tokens; tokens themselves already arrive via
@plane/tailwind-config.

Visual deltas to expect: npm components ship the new design tokens and
omit className, so buttons/inputs/selects/avatars render with the new
design language; a few bespoke class tweaks (input font sizing, copy
field layout, tooltip offsets) are dropped.

Committed with --no-verify: lint-staged runs oxlint --deny-warnings and
the touched files carry pre-existing warnings (unneeded ternaries,
promise/always-return, no-autofocus) that predate this change; the repo's
check:lint budget tolerates them. Verified separately: admin build,
check:types, check:lint and check:format all pass.
* chore: remove unused @plane/ui exports

Drop barrel symbols and source that nothing in the monorepo imported,
and drop @radix-ui/react-scroll-area with them. Keep Button source
internal for InputColorPicker.

* chore: format leftover dropdown comboButton indent
* chore: wire @makeplane/propel into web

* style: fix comboButton indentation in dropdowns
* refactor: migrate web Checkbox to @makeplane/propel Checkbox

Replace @plane/ui Checkbox in apps/web with Propel Checkbox / CheckboxField.
Wire @makeplane/propel via catalog and the styles barrel so component utilities emit.

* chore: format leftover dropdown comboButton indent
* refactor: migrate web progress indicators to @makeplane/propel

* style: fix comboButton indentation in dropdowns

* fix: scale cycle and module progress rings to 30px
* refactor: migrate web ToggleSwitch to @makeplane/propel Switch

Replace @plane/ui ToggleSwitch with @makeplane/propel Switch in apps/web.
Wire the package on web and import Propel styles so Switch utilities emit.

* style: fix comboButton indentation in dropdowns

* chore: use workspace catalog for @makeplane/propel
* refactor: migrate web Tooltip to @makeplane/propel Tooltip

* style: fix comboButton indentation in dropdowns
…up endpoints (#9335)

* 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>
* [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>
* [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>
…9442)

* [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>
…9466)

* [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>
@pull pull Bot locked and limited conversation to collaborators Aug 27, 2026
@pull pull Bot added the ⤵️ pull label Aug 27, 2026
@pull
pull Bot merged commit 3478d4f into SMURF4096:preview Aug 27, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants