Skip to content

fix(api): validate channel disableAutoUpdate and accept metadata alias - #3275

Merged
riderx merged 1 commit into
mainfrom
posthog-self-driving/fixapi-validate-channel-ebcf7b
Sep 12, 2026
Merged

fix(api): validate channel disableAutoUpdate and accept metadata alias#3275
riderx merged 1 commit into
mainfrom
posthog-self-driving/fixapi-validate-channel-ebcf7b

Conversation

@posthog-eu

@posthog-eu posthog-eu Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Summary

Why: An API user set a channel strategy to metadata — the exact word our CLI and docs use — and got a 500 with no message instead of a saved channel. Their channel silently stayed unconfigured, and each hit minted a new error tracking issue.

  • The public endpoint copied disableAutoUpdate straight into the channels insert with no validation and no alias.
  • metadata is not a member of the Postgres disable_update enum (major, minor, patch, version_number, none), so the insert threw and the raw PostgrestError escaped as a 500.
  • The CLI already translates metadataversion_number before it calls us; the public API did not. The sibling updatePackage field was validated a few lines above, so this one field was the odd one out.

Changes — in supabase/functions/_backend/public/channel/post.ts:

  • Map the metadata alias to version_number, matching the CLI.
  • Validate disableAutoUpdate against the five enum values and throw a simpleError, so bad input now returns a 400 with a readable message (invalid_disable_auto_update) instead of a 500.

Checked the sibling public/bundle/set_channel.ts: it only accepts app_id/version_id/channel_id and never touches disableAutoUpdate, so it has no equivalent gap.

Test plan

  • bunx vitest run tests/channel-post.unit.test.ts — added two cases:
    • metadata maps onto the version_number strategy.
    • an unknown strategy is rejected with the invalid_disable_auto_update error.
  • bun run typecheck:backend and bun run lint:ox:backend pass.

Screenshots

Checklist

  • My code follows the code style of this project and passes
    bun run lint:backend && bun run lint.
  • My change requires a change to the documentation.
  • I have updated the documentation
    accordingly.
  • My change has adequate E2E test coverage.
  • I have tested my code manually, and I have provided steps how to reproduce
    my tests

Created with PostHog Desktop from this inbox report.


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.

Review in cubic

Summary by CodeRabbit

  • Bug Fixes
    • Added validation for automatic update settings when posting to existing channels.
    • The metadata option is now interpreted as version_number.
    • Invalid update strategies are rejected with a clear error, preventing unintended channel updates.

The public channel endpoint copied disableAutoUpdate straight into the
channels insert with no validation. The word "metadata", which the CLI and
docs use, is not a member of the disable_update enum, so Postgres rejected it
and the raw error escaped as a 500.

Validate disableAutoUpdate against the five enum values, map the metadata
alias to version_number to match the CLI, and throw a simpleError so bad input
returns a 400 with a readable message. This mirrors the updatePackage check
next to it.

Generated-By: PostHog Desktop
Task-Id: ecd3237a-d82e-40c3-9222-64cfd641251a
@codspeed-hq

codspeed-hq Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Merging this PR will improve performance by 98.74%

⚡ 1 improved benchmark
✅ 42 untouched benchmarks
⏩ 2 skipped benchmarks1

Performance Changes

Benchmark BASE HEAD Efficiency
/updates manifest response with metadata 271.9 µs 136.8 µs +98.74%

Tip

Curious why performance improved? Comment @codspeedbot explain why performance improved on this PR, or directly use the CodSpeed MCP with your agent.


Comparing posthog-self-driving/fixapi-validate-channel-ebcf7b (2ded19a) with main (81f5763)

Open in CodSpeed

Footnotes

  1. 2 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@sonarqubecloud

sonarqubecloud Bot commented Sep 8, 2026

Copy link
Copy Markdown

@posthog-eu
posthog-eu Bot marked this pull request as ready for review September 8, 2026 05:21
@TorichanCapgo

Copy link
Copy Markdown
Contributor

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor
⚠️ Action not completed

Review rate limited.


Your included review limit is currently reached under our Fair Usage Limits Policy. This review may still proceed through usage-based billing if eligible. Your next included review will be available in 25 minutes.

@coderabbitai

coderabbitai Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Advanced

Run ID: fd6e381f-86b9-4f27-b6b2-33d05de936b9

📥 Commits

Reviewing files that changed from the base of the PR and between 81f5763 and 2ded19a.

📒 Files selected for processing (2)
  • supabase/functions/_backend/public/channel/post.ts
  • tests/channel-post.unit.test.ts
🔗 Linked repositories identified

CodeRabbit considers these linked repositories for cross-repo context during reviews:

  • Cap-go/capacitor-updater (manual)

Included review availability: Your plan provides up to 2 included reviews per hour; 1 remains after this review.


📝 Walkthrough

Walkthrough

The channel POST handler now validates disableAutoUpdate, maps metadata to version_number, and rejects unsupported strategies. Unit tests cover normalization and prevention of channel updates after invalid input.

Changes

Disable auto-update validation

Layer / File(s) Summary
Validation and test coverage
supabase/functions/_backend/public/channel/post.ts, tests/channel-post.unit.test.ts
The handler maps metadata to version_number and rejects unsupported strategies with invalid_disable_auto_update. Tests verify normalization and that invalid input prevents updateOrCreateChannel.

Priority: ⬇️ Low — Defer this narrow channel POST validation change because it only normalizes supported update strategies and rejects invalid values with a client error.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to 2ded1

Channel updates now consistently normalize supported disable-auto-update values and return a 400 error for invalid values without modifying the channel. No current merge-blocking risk remains.

Suggested reviewers: riderx

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main changes: validating channel disableAutoUpdate and accepting the metadata alias.
Description check ✅ Passed The description follows the repository template. It explains the problem, implementation, test plan, backend-only scope, and checklist status. It includes the added test cases and reported typecheck a…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch

Comment @coderabbitai help to get the list of available commands.

@TorichanCapgo

Copy link
Copy Markdown
Contributor

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Full review finished.

@MarkZ1966github MarkZ1966github left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Unique note, not in the bot reviews:

The metadata alias is compared with strict equality after a cast, then rewritten to version_number before the allow-list check. Two consequences:

  1. The error string lists metadata as an accepted value, but that string never survives into includes(). A client that sends Metadata or METADATA gets invalid_disable_auto_update even though the message told them metadata is valid. The CLI flag is documented as metadata in lowercase, so this only bites if another API client capitalizes it. Worth normalizing with .toLowerCase() before the alias map, or dropping metadata from the error text since it is not actually a stored enum member.

  2. body.disableAutoUpdate is mutated in place. Anything that logs the original request after this block will show version_number and hide that the caller sent metadata. A local let strategy = ... keeps the inbound payload intact for logs.

The tests cover the happy alias and a typo (meta). A case-fold case would lock the first point down.

Otherwise the mapping matches the CLI, and rejecting unknown strategies with simpleError is the right 400 instead of a raw Postgrest 500.

@riderx
riderx merged commit 7dc3d02 into main Sep 12, 2026
96 of 100 checks passed
@riderx
riderx deleted the posthog-self-driving/fixapi-validate-channel-ebcf7b branch September 12, 2026 14:10
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.

3 participants