Add the public channel behind flag_public_channel - #4284
Conversation
Re-check the consent form per request so a consent form published mid-session ends the live public session with 409 consent_unavailable, end live sessions when a public channel is disabled or deleted (only regenerate did), drop blank lines from the welcome and starter lists instead of failing validation (a trailing newline blocked regenerate), pass user-id and user-name to team members only, keep the widget live for team members on refused states, include public channels in the outdated widget report, and pin the API-level revocation behaviour with a test.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![codescene-delta-analysis[bot]](https://avatars.githubusercontent.com/in/53921?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
📝 WalkthroughWalkthroughAdds Estimated code review effort: 4 (Complex) | ~60 minutes Merge Risk: 🟠 High · up to The public-link flow can expose unpublished chatbot details, mishandle IPv6 hosts, and leave conversations active after link regeneration or channel shutdown if a database update fails; the legacy flag removal also needs explicit owner approval. Merge should be blocked until these issues are addressed or formally accepted. Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 22.83% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 127 functions across 21 files. (9 skipped: 9 unsupported.) Full details: Description checkExplanation The description is complete and follows the repository template. It covers the product and technical changes, migration compatibility, demo steps, documentation impact, operator impact, feature-flag behavior, deployment requirements, and known follow-ups.
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Pull the version_number handling in chat_send_message into _send_version, share the published-version lookup between the two public-link helpers, gate email and public in for_dropdown through one flag helper, and assert the public page CSP by value rather than by substring.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
The inline-imports check showed the local import in end_live_sessions hoists cleanly; Experiment was already imported from the same module at the top of the file.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
A public link serves the widget bundled with the platform, so its version moves with the deploy and a team has nothing to upgrade. Reporting it gives whoever runs the command a row they cannot act on. Claude-Session: https://claude.ai/code/session_014quZadopDrfAzty9X1Sm5D
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
widget_update_status, min_widget_version and pending_min_widget_version all describe a widget the host site loads and the team can upgrade. A public link serves the widget bundled with the platform, so its version follows the deploy and these three name something the team has no way to change. widget_auth_level stays on both widget platforms: the public channel authenticates with an embed key and session token, so apps.api.permissions and the chat views read it. Claude-Session: https://claude.ai/code/session_014quZadopDrfAzty9X1Sm5D
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
A public link served from a non-canonical host returned a bare 404, which reads the same as a mistyped token, so a misconfigured deployment gives whoever is testing it nothing to work from. Follows the TeamAccessDenied convention: a tagged Http404 subclass that 404.html recognises through the exception context variable. A signed-in viewer is named both hosts; an anonymous visitor still gets the bare 404, so nothing about the deployment reaches the public and the request is still refused before any lookup. Claude-Session: https://claude.ai/code/session_014quZadopDrfAzty9X1Sm5D
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
_page_state returned early on a disabled channel without resolving the published version, so the view fell back to the working version for the page title and description. A team that renamed its draft after publishing had that name shown to anonymous visitors whenever the channel was switched off, which contradicts the published-version-only rule the start API enforces. Resolving the published version before any refusal keeps every state naming the chatbot a visitor could have reached. The existing tests could not catch this: the helper renames before create_new_version, so draft and published names were always identical. Claude-Session: https://claude.ai/code/session_014quZadopDrfAzty9X1Sm5D
CodeQL flagged the bare "other.example.com in html" check as incomplete URL substring sanitization. It is a test assertion rather than sanitization, so nothing was exploitable, but a substring that loose also passes when the host appears anywhere on the page. Asserting the surrounding markup pins the host to the sentence meant to carry it, and deriving the canonical root from get_server_root() keeps the assertion correct whichever way USE_HTTPS_IN_ABSOLUTE_URLS is set. Claude-Session: https://claude.ai/code/session_014quZadopDrfAzty9X1Sm5D
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Code reviewNo issues found. Checked for bugs and CLAUDE.md compliance. Non-blocking: the PR description is missing the |
The start-session and send paths refused a public visitor with a 409 when the published version carried a consent form, and the page showed a matching banner. No other channel reads consent_form in the Chat API, so the public link was the only surface applying it, and a chatbot carrying its team's form would have had a link that never opened. The page banner goes with the API refusal. Keeping it would show a visitor a refusal on a page whose widget the API now serves. 409 on start-session is no_published_version only, and the schema description follows. Tests cover the new behaviour on both paths: a consent-form chatbot starts a session and receives a session token, a send continues after a consent form is published mid-session, and the page renders the kiosk widget with no banner. Claude-Session: https://claude.ai/code/session_01GGtbSvKvUsS1i8mvwiQBuK
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Two copies of the same fixture, one per test module, each creating the flag row and adding the team. They move to the package conftest alongside the other shared channel fixtures. The docstring records why waffle's override_flag does not replace this: the platform gate reads Flag.is_active_for_team, which matches on the team M2M and never on everyone, the only field override_flag sets. A test decorated with it would assert the opposite of what it means. Claude-Session: https://claude.ai/code/session_01GGtbSvKvUsS1i8mvwiQBuK
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
The docstring said most chatbots carry their team's consent form. Nothing establishes that: ChatbotForm never sets consent_form, so a chatbot created through the UI starts without one. The point the test makes does not need a frequency claim. Gating the public link on a consent form would refuse any chatbot that carries one, and no other channel collects consent through the Chat API. Claude-Session: https://claude.ai/code/session_01GGtbSvKvUsS1i8mvwiQBuK
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
The page fell back to the working version whenever no published version existed, so a builder who shared a link and then renamed the draft to something internal had that name rendered in the title and heading for anyone holding the token. Two paths reached it: a channel that has never been published, and one that is both disabled and never published. Anonymous visitors now get a placeholder name and no description. Team members keep the draft's name, since they use this page to preview before publishing and the placeholder would hide what they came to look at. The docstring on _page_state claimed every state names the chatbot the visitor could have reached. That was stronger than the code could deliver on a branch with no published row, so it now says what actually holds. Claude-Session: https://claude.ai/code/session_01GGtbSvKvUsS1i8mvwiQBuK
There was a problem hiding this comment.
(2 files improve in Code Health)
Gates Failed
(1 file with Low Cohesion)
Our agent can fix these. Install it.
Gates Passed
Reason for failure
| Enforce critical code health rules | Violations | Code Health Impact | |
|---|---|---|---|
| forms.py | 1 critical rule | 9.07 → 8.00 | Suppress |
View Improvements
| File | Code Health Impact | Categories Improved |
|---|---|---|
| models.py | 9.69 → 10.00 | Complex Method |
| chat.py | 8.24 → 8.60 | Complex Method, Bumpy Road Ahead, Overall Code Complexity |
Quality Gate Profile: Clean Code Collective
Install CodeScene MCP: safeguard and uplift AI-generated code. Catch issues early with our IDE extension and CLI tool.
|
Workflow Run: https://github.com/dimagi/open-chat-studio-docs/actions/runs/33382814830 Please check the logs or manually update the changelog in the docs repository. |
3 participants
Product Description
Step 2 of #3682 (design:
docs/design/public-channel.md, D1 to D6, D8, D9). Behindflag_public_channel, a chatbot can get a "Public link" channel: a/c/<token>/page on the OCS host with the chat widget in kiosk mode. Share the link and people can chat, no embedding needed.What a builder sees:
Visitors only ever reach the published version. Regenerating the link revokes the old one straight away and ends every live conversation on it. Disabling or deleting the channel ends live conversations too.
The old chat-widget prototype behind
flag_chat_widgetis gone; that flag row can be deleted from/admin/flags/after merge.Technical Description
ChannelPlatform.PUBLICmaps toApiChanneland reuses the embedded widget'swidget_tokenembed key and session-token auth.ChannelPlatform.widget_platforms()covers the places both widget platforms share (auth lookups,widget_auth_level). The auth-level ratchet, migration 0029,EmbeddedWidgetChannelForm,fetch_widget_embed_key, the widget update status and the outdated-widget report stayEMBEDDED_WIDGETonly, since the platform bundles the widget a public link serves.channel_origin_allowed(request, channel), replaces the twoallowed_domainsreads:allowed_domainsfor the embedded widget, theSitehostname (port ignored) for a public channel.409 no_published_version. Send and upload re-resolve the published version on every request and return the same 409 if it has gone. Team members are exempt at start and per request so they can preview an unpublished chatbot through its page.ChatAPIRateThrottlekeys a public start on the visitor IP (soRATE_LIMIT_TRUSTED_PROXY_COUNTmatters behind a proxy). The page itself is on the existingpublic_chatscope.regenerate_link=1to the edit dialog.PublicChannelForm.clean()mints the token;post_savecallsExperimentChannel.end_live_sessions(), which marks the channel's open sessionsCOMPLETE(what the widget and API key off).soft_delete()and disabling the channel do the same.X-Robots-Tag: noindexandReferrer-Policy: origin; no Sentry or analytics on it./c/is disallowed inrobots.txt.user-id/user-nameare passed to team members only.ChannelPlatform.for_dropdownnow tolerates a used platform whose flag is off (it used toKeyErroron the chatbot home).Things to know when running it:
persistent-session="tab", now that widget 0.12.0 is live on the site. The session lives insessionStorage, so it survives a reload and clears when the tab closes, and a shared browser does not hand the next visitor the previous conversation.Sitedomain. If that row does not name the deployed host, links 404 and starts 403. The hosting docs say so now./c/<token>/lookup onextra_datais unindexed until the GIN index follow-up; thepublic_chatper-IP throttle bounds it.chatbot_idand no embed key still reaches the team API channel and its published-or-working version (that is the keyless-start sunset's job); after start, chat endpoints throttle per session rather than per IP;RATE_LIMIT_ENFORCEdefaults to log-only; the start response includes version names and descriptions.Follow-ups from the design, not in this PR: GIN index on
extra_data, scrubbing/c/<token>/from logs and Sentry URLs before GA, step 3 (consent in the widget).Migrations
0034_alter_experimentchannel_platform: choices only, no data touched.Demo
Local dev server, flag on,
Sitedomainlocalhost:8000, published chatbot./c/<token>/as an anonymous visitorplatform=public, trace on the published version)409 no_published_versionThe public page renders light regardless of the app theme: it is a standalone template with no
data-themehandling. Worth a follow-up if we want it to follow the visitor's OS preference.Docs and Changelog
Channel docs gain the "Public link" channel (flag-gated); the automation opens the docs PR on merge. Hosting docs (rate-limit scopes,
RATE_LIMIT_TRUSTED_PROXY_COUNT,Sitedomain) are updated here.Operator Impact
Nothing breaks on upgrade. The channel sits behind
flag_public_channel, which is off by default, and no existing behaviour changes until a team is opted in at/admin/flags/. Before enabling it:Siterow must name the deployed host. Public links are built from it, and every request to/c/<token>/is checked against it. If it does not match, links 404 and their chat starts are refused with 403. The value is cached per process, so a change needs a restart. Written up indocs/hosting/configuration.md.RATE_LIMIT_TRUSTED_PROXY_COUNTbehind a proxy or tunnel. A public link start is keyed on the visitor IP, and the page itself is on thepublic_chatscope. Left at0behind a proxy, every visitor shares one bucket.unpkg.com. The page's CSP allowsunpkg.comfor scripts andcdnjs.cloudflare.comfor fonts and styles. A deployment that expects visitor traffic to reach only its own origin needs to know that.robots.txtnow disallows/c/.Turning the flag off later hides the channel type from the dropdown but does not revoke links already handed out. Regenerate or delete the channel for that.
After merge the
flag_chat_widgetrow can be deleted from/admin/flags/; the prototype it gated is gone.