Skip to content

fix(events): make Event Venue team optional - #448

Merged
harshtandiya merged 3 commits into
developfrom
fix/venue-team-optional
Sep 8, 2026
Merged

fix(events): make Event Venue team optional#448
harshtandiya merged 3 commits into
developfrom
fix/venue-team-optional

Conversation

@harshtandiya

@harshtandiya harshtandiya commented Sep 8, 2026

Copy link
Copy Markdown
Collaborator

What changed

Event Venue.team was reqd, which meant a venue with no owning team could not be
created or saved at all — shared venues and legacy rows both hit the mandatory error.
This drops reqd and widens the generated type to DF.Link | None.

That also takes out BuzzEvent.validate_venue_team, the check that refused a venue
whose team differed from the event's. With the field optional it no longer describes
an invariant the data holds, so the check and its three tests go with it.

Worth a reviewer's eye: the booking confirmation and the calendar invite still read
the linked venue's address with no permission check of their own. Role permissions on
Event Venue are now the only thing gating a cross-team link. If we want that guard
back, the shape is probably "the venue's team must match or be unset" rather than
the strict equality that just came out.

Third commit is a fallout fix. test_leaves_team_empty_when_the_user_has_two_teams
inserted an Event Venue as a two-team user and asserted MandatoryError, leaning on
the venue's reqd Team field to make the outcome visible. It now asserts the empty team
directly, which is what the test name claims and is narrower than MandatoryError.
The set_team_from_sole_membership docstring said reqd raises for ambiguous cases;
that is now only true where the field is still required, so it says so.

Second commit is unrelated housekeeping: BuzzEvent.ts never got regenerated after
the co-hosts table landed in #447, so the dashboard types were missing co_hosts and
still had host as required.

Demo

No UI change — types only on the dashboard side. skip-demo.

Testing

  • bench --site buzz.localhost run-tests --module buzz.api.events.test_events — 84 passed, 1 skipped.
  • bench --site buzz.localhost run-tests --module buzz.events.doctype.buzz_team.test_buzz_team — 16 passed.
  • Full CI green on 9ed6258.
  • bench --site buzz.localhost run-tests --module buzz.events.doctype.buzz_event.test_buzz_event — 24 errors, all MandatoryError: team on Buzz Event fixtures. Same 24 fail on a clean develop tree, so they are pre-existing and not from this change.

A venue is not always owned by a team: shared and legacy venues have no team
stamp, and a required Team field blocked creating or saving them at all.

Drop `reqd` on Event Venue.team and widen the generated type to
`DF.Link | None`.

This also removes `BuzzEvent.validate_venue_team`, which refused a venue whose
team differed from the event's. With the field optional that guard no longer
describes an invariant the data holds, so the cross-team check and its tests go
with it. Note that the booking confirmation and the calendar invite still read
the linked venue's address without a permission check; role permissions on
Event Venue are now the only thing gating a cross-team link.
The co-hosts table and the now-optional host field landed on the DocType but
the generated types were never regenerated alongside them.
@github-actions

github-actions Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

UI Demo Check — skipped via the skip-demo label.

@harshtandiya harshtandiya added the skip-demo Skip adding a video / screenshot in PR description. Passes the failing CI for demo addition label Sep 8, 2026
`test_leaves_team_empty_when_the_user_has_two_teams` inserted an Event Venue as
a two-team user and asserted `MandatoryError`, leaning on the venue's reqd Team
field to surface the outcome. With that field optional the insert now succeeds
and the assertion never fired.

Assert `doc.team` is empty instead, which is what the test name claims and is a
narrower check than `MandatoryError` — that could have come from any missing
field.
@greptile-apps

greptile-apps Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR allows Event Venue records without an owning team, removes strict event-to-venue team validation, and refreshes generated dashboard types for event co-hosts.

  • Makes Event Venue.team optional and permits ambiguous multi-team creation to leave it unset.
  • Removes the venue ownership validator and its tests, unintentionally reopening cross-team venue-address disclosure.
  • Updates BuzzEvent dashboard typing so host is optional and co_hosts is represented by the new EventCoHost type.

Confidence Score: 3/5

The PR is not safe to merge until cross-team venue links with a nonempty, mismatched owner are rejected or every downstream address read enforces equivalent authorization.

A manager can now save an event linked to another team’s venue, and attendee-facing booking and calendar paths dereference that venue’s address without permission checks, creating a concrete cross-team disclosure.

Files Needing Attention: buzz/events/doctype/buzz_event/buzz_event.py

Security Review

Removing the event-level venue ownership check permits a manager to link another team’s venue by identifier. Existing booking-confirmation and calendar-invite paths then read and disclose that venue’s address without applying Event Venue permissions.

Important Files Changed

Filename Overview
buzz/events/doctype/buzz_event/buzz_event.py Removes the only document-boundary check preventing cross-team venue links, exposing linked venue addresses through downstream attendee outputs.
buzz/events/doctype/event_venue/event_venue.json Makes venue ownership optional as intended, enabling shared and legacy teamless venues.
buzz/events/doctype/buzz_team/test_buzz_team.py Updates the sole-membership behavior test to establish that multi-team users may create teamless venues.
dashboard/src/types/Events/BuzzEvent.ts Correctly aligns optional host and co-host declarations with the current Buzz Event schema.
dashboard/src/types/Events/EventCoHost.ts Adds a generated child-table type consistent with the Event CoHost schema.

Fix all with Greploop Fix All in Claude Code Fix All in Codex

Prompt To Fix All With AI
### Issue 1
buzz/events/doctype/buzz_event/buzz_event.py:106-108
**Cross-team venue address disclosure**

Removing the venue-team validation lets a manager who can create events for one team submit another team’s guessable venue identifier. The event saves because creation checks only access to the event’s team. Booking confirmations and calendar invitations then read the linked venue’s address directly without enforcing Event Venue permissions, disclosing it to the event’s attendees. Teamless shared venues can remain supported while venues with a different nonempty team must be rejected.

**How this was verified:** The caller-controlled venue link reaches unchecked `get_cached_doc` and `db.get_value` address reads after the document-level team comparison was removed.

---

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

Reviews (1): Last reviewed commit: "fix(events): assert the empty team direc..." | Re-trigger Greptile

Comment on lines 106 to 108
self.validate_guest_verification_config()
self.validate_custom_forms()
self.clear_unused_location()

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.

P1 security Cross-team venue address disclosure

Removing the venue-team validation lets a manager who can create events for one team submit another team’s guessable venue identifier. The event saves because creation checks only access to the event’s team. Booking confirmations and calendar invitations then read the linked venue’s address directly without enforcing Event Venue permissions, disclosing it to the event’s attendees. Teamless shared venues can remain supported while venues with a different nonempty team must be rejected.

How this was verified: The caller-controlled venue link reaches unchecked get_cached_doc and db.get_value address reads after the document-level team comparison was removed.

Knowledge Base Used:

Prompt To Fix With AI
This is a comment left during a code review.
Path: buzz/events/doctype/buzz_event/buzz_event.py
Line: 106-108

Comment:
**Cross-team venue address disclosure**

Removing the venue-team validation lets a manager who can create events for one team submit another team’s guessable venue identifier. The event saves because creation checks only access to the event’s team. Booking confirmations and calendar invitations then read the linked venue’s address directly without enforcing Event Venue permissions, disclosing it to the event’s attendees. Teamless shared venues can remain supported while venues with a different nonempty team must be rejected.

**How this was verified:** The caller-controlled venue link reaches unchecked `get_cached_doc` and `db.get_value` address reads after the document-level team comparison was removed.

**Knowledge Base Used:**
- [Event management domain](https://app.greptile.com/bwh-tech/-/custom-context/knowledge-base/bwhtech/buzz/-/docs/event-management.md)
- [Add dashboard event creation with access and location safeguards](https://app.greptile.com/bwh-tech/-/custom-context/knowledge-base/bwhtech/buzz/-/reverts/incident-mitigation_354-20260827-dashboard-event-creation-3638a1d.md)

---

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

Fix in Claude Code Fix in Codex

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

to be handled in upcoming PRs

@harshtandiya
harshtandiya merged commit 10f8fa4 into develop Sep 8, 2026
11 checks passed
@harshtandiya
harshtandiya deleted the fix/venue-team-optional branch September 8, 2026 14:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

skip-demo Skip adding a video / screenshot in PR description. Passes the failing CI for demo addition

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant