Skip to content

fix(web): check private space access before sharing Caps - #2883

Merged
richiemcilroy merged 3 commits into
CapSoftware:mainfrom
yashranaway:fix/web-actions-auth
Oct 23, 2026
Merged

richiemcilroy merged 3 commits into
CapSoftware:mainfrom
yashranaway:fix/web-actions-auth

Conversation

@yashranaway

@yashranaway yashranaway commented Oct 22, 2026 •

Copy link
Copy Markdown
Contributor

sharing a cap checked that you're in the space's organisation, but not that you can actually get into the space. so any org member could put their own video into a private space they weren't part of.

now newly added private spaces go through the same access check folder moves already use, so space members, the creator and org admins can still share, and it runs before anything is written. spaces the cap is already in are left alone, so losing access later doesn't block saving the other settings.

RetriggerView in GreptileConfidence Score: 5/5

The PR appears safe to merge; no new actionable issues were found.

Summary

This PR checks access before adding a Cap to a private space.

  • Cap sharing checks access before adding a private space.

Reviews (2) · Last reviewed commit: "test(web): cover existing organization s..." · Reviewed by Greptile

@yashranaway

Copy link
Copy Markdown
Contributor Author

@greptileai review

@vercel

vercel Bot commented Oct 22, 2026 •

Copy link
Copy Markdown
Contributor

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated
cap-status Skipped Skipped Oct 23, 2026 9:40pm UTC

@vercel
vercel Bot temporarily deployed to Preview – cap-status October 22, 2026 07:20 Inactive
Comment on lines +102 to +106
it("rejects an organization member outside the space before changing any shares or visibility", async () => {
await expect(
shareCap({ capId, spaceIds: [spaceId], public: true }),
).resolves.toMatchObject({ success: false });
expect(writes()).toEqual([]);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Existing shares are untested

This denial test starts with no existing organization shares, and the mock always returns an empty result for sharedVideos. Moving the access check below the organization-share writes would therefore still pass this test. Add an existing organization share that the request would remove, then assert that denial leaves it untouched and makes no writes.

Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/web/__tests__/unit/share-cap-authorization.test.ts
Line: 102-106

Comment:
**Existing shares are untested**

This denial test starts with no existing organization shares, and the mock always returns an empty result for `sharedVideos`. Moving the access check below the organization-share writes would therefore still pass this test. Add an existing organization share that the request would remove, then assert that denial leaves it untouched and makes no writes.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair. The denial test now starts with an org share the request would remove and checks it's still there with no writes. Moving the access check below the writes makes it fail (b00b166).

@vercel
vercel Bot temporarily deployed to Preview – cap-status October 22, 2026 08:37 Inactive
@richiemcilroy

Copy link
Copy Markdown
Member

@greptileai review

@vercel
vercel Bot temporarily deployed to Preview – cap-status October 23, 2026 21:40 Inactive
@richiemcilroy
richiemcilroy merged commit 2090ec2 into CapSoftware:main Oct 23, 2026
30 of 31 checks passed

This branch was previously deployed

1 inactive deployment
Preview – cap-status — 707775ae Deployed Oct 23, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants