Skip to content

Fix Supabase startup configuration and harden cloud foundation - #18

Open
juliosuas wants to merge 3 commits into
mainfrom
fsociaty/rocio-cloud-recovery
Open

Fix Supabase startup configuration and harden cloud foundation#18
juliosuas wants to merge 3 commits into
mainfrom
fsociaty/rocio-cloud-recovery

Conversation

@juliosuas

@juliosuas juliosuas commented Jul 21, 2026

Copy link
Copy Markdown
Owner

What changed

  • Bundle the Supabase URL and publishable key through an explicit iOS Info.plist and per-Mac Local.xcconfig.
  • Reject secret/service-role keys during builds and audit the produced app/archive configuration in CI.
  • Harden the cloud migration, RLS/ACL boundaries, scan ownership constraints, and the identify-flower Edge Function.
  • Replace the invalid SF Symbol used by the missing-cloud state.

Root cause

Xcode exposed the Supabase build settings but silently omitted the custom keys from its generated Info.plist. BackendConfiguration.bundled therefore returned nil, the session stayed in .unconfigured, and the app never made a Supabase request. An explicit plist fixes the runtime configuration path.

Impact

The iOS app now reaches the authentication screen and remains running. Only the public publishable key is bundled; service-role and Plant.id secrets remain server-side. Anonymous callers cannot read or write the protected tables.

Validation

  • XCTest: 34/34 passed on iPhone 17 / iOS 26.3.1 Simulator
  • Cloud/security audit: 33/33
  • App Store release gate: 11/11
  • GitHub Actions: QA, iOS, and iOS Archive all passed on head 9053859
  • PostgreSQL integration checks: passed
  • Supabase migration applied and aligned with remote
  • identify-flower deployed as active v5; unauthenticated request returns 401
  • Produced app plist validated without printing the key
  • Negative gates verified: a second migration and a signed Release without the publishable key both fail closed
  • CodeRabbit's three actionable review threads were addressed and auto-resolved
  • git diff --check: clean

Summary by CodeRabbit

  • New Features

    • Added per-device iOS cloud configuration using a public publishable key.
    • Added built-app checks to verify cloud settings and required iOS metadata.
    • Improved cloud scan handling with safer input normalization and protected result storage.
  • Bug Fixes

    • Improved authentication and authorization handling for flower identification.
    • Strengthened data validation, quota enforcement, analytics safeguards, and account operations.
    • Updated the unavailable-cloud icon for clearer iOS presentation.
  • Documentation

    • Added iOS setup guidance and warnings against including private service credentials in app builds.

@coderabbitai

coderabbitai Bot commented Jul 21, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The PR adds per-Mac iOS Supabase configuration, explicit plist validation in CI, broader repository security audits, hardened flower-identification processing, and stricter Supabase schema, RLS, function, and privilege enforcement.

Changes

Rocio Cloud hardening

Layer / File(s) Summary
iOS build configuration and validation
.gitignore, ios/Config/*, ios/Rocio.xcodeproj/project.pbxproj, ios/Rocio/Resources/Info.plist, ios/Scripts/*, ios/Rocio/Views/AuthView.swift, ios/RocioTests/*, ios/README.md
Adds shared and local xcconfig handling, explicit Info.plist metadata, publishable-key validation, project wiring, and the icloud.slash UI symbol with a corresponding test.
Built iOS audits and workflow wiring
qa/ios-built-cloud-config-audit.mjs, qa/ios-app-store-readiness-audit.mjs, .github/workflows/ios.yml, .github/workflows/ios-archive.yml
Validates built plist values and connects the audit to Simulator and archive workflows using shared derived-data paths.
Repository security audit
qa/cloud-ai-security-audit.mjs
Aggregates migration and client-source inputs, scans tracked text files for credentials, validates key formats, and reports expanded security checks.
identify-flower authentication and result handling
supabase/config.toml, supabase/functions/identify-flower/index.ts
Disables platform JWT verification for manual handler validation, separates user and admin clients, sanitizes provider output, consumes quota through the user client, and inserts scan results through the admin client.
Database constraints, functions, and ACLs
supabase/migrations/20260709000100_rocio_cloud_foundation.sql
Tightens data constraints and relationships, updates analytics and scan policies, hardens security-definer functions, normalizes locales, and restricts table and function privileges.

Estimated code review effort: 5 (Critical) | ~120 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Caller
  participant identifyFlower
  participant SupabaseAuth
  participant PlantID
  participant SupabaseDatabase
  Caller->>identifyFlower: Submit image with Bearer token
  identifyFlower->>SupabaseAuth: Validate caller session
  identifyFlower->>SupabaseDatabase: Consume scan quota
  identifyFlower->>PlantID: Request identification
  identifyFlower->>SupabaseDatabase: Insert sanitized scan result
  identifyFlower-->>Caller: Return normalized suggestions
Loading

Possibly related PRs

  • juliosuas/rocio#8: Modifies the same Supabase scanner, function, configuration, and migration foundations.
🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check ⚠️ Warning It covers the main changes and validation, but it misses the required App Store/privacy impact, notes, and review stance sections from the template. Reformat it to the repository template and add the App Store/privacy impact checklist, Notes fields, and Review stance requested sections.
✅ Passed checks (3 passed)
Check name Status Explanation
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.
Title check ✅ Passed The title accurately summarizes the two main changes: Supabase configuration fixes and cloud foundation hardening.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fsociaty/rocio-cloud-recovery

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.

❤️ Share

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

@coderabbitai coderabbitai Bot 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.

Actionable comments posted: 3

🧹 Nitpick comments (2)
qa/ios-built-cloud-config-audit.mjs (1)

15-18: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Prevent TypeError if the command fails to spawn.

If spawnSync fails to launch plutil (for example, if run on a non-macOS environment or if the executable is missing), converted.status will be null and converted.stderr will be undefined. Calling .trim() on undefined throws a TypeError, which hides the underlying spawn error.

Consider checking for converted.error first or using optional chaining.

♻️ Proposed refactor
-if (converted.status !== 0) {
-  console.error(converted.stderr.trim());
+if (converted.error) {
+  console.error(`Failed to spawn plutil: ${converted.error.message}`);
+  process.exit(2);
+} else if (converted.status !== 0) {
+  console.error(converted.stderr?.trim() ?? 'Unknown error');
   process.exit(2);
 }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@qa/ios-built-cloud-config-audit.mjs` around lines 15 - 18, Update the spawn
result handling around converted.status to check converted.error before
accessing converted.stderr, and safely handle missing stderr with optional
chaining or an equivalent fallback. Preserve the existing nonzero exit behavior
while reporting the underlying spawn failure instead of throwing a TypeError.
qa/cloud-ai-security-audit.mjs (1)

141-142: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Several checks assert exact occurrence counts, which will spuriously fail on unrelated refactors.

security-definer-search-path (Line 142: securityDefinerSettings.length === 4), service-role-key-not-leaked (Line 156: exact counts of 3/2), debug-release-consume-shared-config (Lines 175-176: exact count of 2), and project-url-retained (Line 184: exact count of 2) all hardcode the number of matches rather than verifying the property they care about (e.g. "every security-definer function has a safe search_path", "the edge function keeps user/admin clients separate", "both build configurations reference the shared xcconfig/plist"). A legitimate change (e.g. adding a comment referencing serviceRoleKey, adding a third build configuration, or adding a new hardened function) will fail these checks even though nothing is actually wrong, causing avoidable CI churn.

Prefer structural assertions (e.g. .every(...) over the matched set, or checking for the presence of the specific properties) instead of brittle exact counts.

Also applies to: 156-156, 174-177, 184-184

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@qa/cloud-ai-security-audit.mjs` around lines 141 - 142, Replace the exact
match-count assertions in the checks for security-definer-search-path,
service-role-key-not-leaked, debug-release-consume-shared-config, and
project-url-retained with structural assertions that validate the required
properties for every relevant match or required configuration. Preserve the
intended guarantees—safe search_path settings, separated user/admin clients,
shared configuration references in both builds, and no leaked service-role
key—without depending on fixed occurrence counts.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@qa/cloud-ai-security-audit.mjs`:
- Around line 21-47: Replace the allMigrations-based input used by policy, ACL,
and security-definer checks with the authoritative current schema state,
preferably from a live schema/grants dump or the latest authoritative migration.
Update allMigrations and its consumers, including
scan-results-no-client-insert-policy, explicit-table-acls, and
security-definer-search_path, so revoked historical statements no longer affect
results while current effective grants and policies remain validated.
- Around line 189-196: Update the audit script’s final result handling after the
existing failed, credentialFiles, and oversizedTextFiles calculations so any
failed check or detected credential/oversized file sets a nonzero process exit
status; preserve the current reporting output and allow a zero exit only when
all checks pass and both file lists are empty.

In `@supabase/migrations/20260709000100_rocio_cloud_foundation.sql`:
- Around line 18-28: Update GardenStore.update to normalize outbound values
before invoking cloudChangeHandler: trim nickname, reject empty results as
currently handled, truncate nickname to 80 characters, and truncate notes to
2000 characters. Ensure the clamped values are the ones sent to CloudKit while
preserving existing update behavior.

---

Nitpick comments:
In `@qa/cloud-ai-security-audit.mjs`:
- Around line 141-142: Replace the exact match-count assertions in the checks
for security-definer-search-path, service-role-key-not-leaked,
debug-release-consume-shared-config, and project-url-retained with structural
assertions that validate the required properties for every relevant match or
required configuration. Preserve the intended guarantees—safe search_path
settings, separated user/admin clients, shared configuration references in both
builds, and no leaked service-role key—without depending on fixed occurrence
counts.

In `@qa/ios-built-cloud-config-audit.mjs`:
- Around line 15-18: Update the spawn result handling around converted.status to
check converted.error before accessing converted.stderr, and safely handle
missing stderr with optional chaining or an equivalent fallback. Preserve the
existing nonzero exit behavior while reporting the underlying spawn failure
instead of throwing a TypeError.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 7dfb53ef-2575-4b2a-91f9-4ff2b7a32f25

📥 Commits

Reviewing files that changed from the base of the PR and between 4ad8163 and cdef08d.

📒 Files selected for processing (17)
  • .github/workflows/ios-archive.yml
  • .github/workflows/ios.yml
  • .gitignore
  • ios/Config/Local.xcconfig.example
  • ios/Config/Rocio.xcconfig
  • ios/README.md
  • ios/Rocio.xcodeproj/project.pbxproj
  • ios/Rocio/Resources/Info.plist
  • ios/Rocio/Views/AuthView.swift
  • ios/RocioTests/CloudFoundationTests.swift
  • ios/Scripts/validate-supabase-client-key.sh
  • qa/cloud-ai-security-audit.mjs
  • qa/ios-app-store-readiness-audit.mjs
  • qa/ios-built-cloud-config-audit.mjs
  • supabase/config.toml
  • supabase/functions/identify-flower/index.ts
  • supabase/migrations/20260709000100_rocio_cloud_foundation.sql

Comment thread qa/cloud-ai-security-audit.mjs Outdated
Comment thread qa/cloud-ai-security-audit.mjs
Comment thread supabase/migrations/20260709000100_rocio_cloud_foundation.sql
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.

1 participant