Fix Supabase startup configuration and harden cloud foundation - #18
Fix Supabase startup configuration and harden cloud foundation#18juliosuas wants to merge 3 commits into
Conversation
📝 WalkthroughWalkthroughThe 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. ChangesRocio Cloud hardening
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
Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
qa/ios-built-cloud-config-audit.mjs (1)
15-18: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePrevent TypeError if the command fails to spawn.
If
spawnSyncfails to launchplutil(for example, if run on a non-macOS environment or if the executable is missing),converted.statuswill benullandconverted.stderrwill beundefined. Calling.trim()onundefinedthrows aTypeError, which hides the underlying spawn error.Consider checking for
converted.errorfirst 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 winSeveral 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), andproject-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 referencingserviceRoleKey, 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
📒 Files selected for processing (17)
.github/workflows/ios-archive.yml.github/workflows/ios.yml.gitignoreios/Config/Local.xcconfig.exampleios/Config/Rocio.xcconfigios/README.mdios/Rocio.xcodeproj/project.pbxprojios/Rocio/Resources/Info.plistios/Rocio/Views/AuthView.swiftios/RocioTests/CloudFoundationTests.swiftios/Scripts/validate-supabase-client-key.shqa/cloud-ai-security-audit.mjsqa/ios-app-store-readiness-audit.mjsqa/ios-built-cloud-config-audit.mjssupabase/config.tomlsupabase/functions/identify-flower/index.tssupabase/migrations/20260709000100_rocio_cloud_foundation.sql
What changed
Info.plistand per-MacLocal.xcconfig.identify-flowerEdge Function.Root cause
Xcode exposed the Supabase build settings but silently omitted the custom keys from its generated
Info.plist.BackendConfiguration.bundledtherefore returnednil, 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
9053859identify-flowerdeployed as active v5; unauthenticated request returns 401git diff --check: cleanSummary by CodeRabbit
New Features
Bug Fixes
Documentation