feat: db policies #529
feat: db policies #529
Conversation
github-actions
bot
added
App: Dashboard
Summary of ChangesHello @chrismclarke, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the application's database security posture by implementing comprehensive Row Level Security (RLS) policies and fine-grained permissions for various tables. It ensures that database interactions from serverless functions are performed with appropriate service role privileges. Additionally, the PR includes minor front-end refinements for date display and button styling, alongside a small TypeScript optimization for type imports. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
|
View your CI Pipeline Execution ↗ for commit 3c1078b
☁️ Nx Cloud last updated this comment at |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces significant updates to database policies, including enabling Row Level Security (RLS) and defining access policies for several tables. It also refactors client imports in TypeScript files to consistently use getServiceRoleClient and refines date formatting in an Angular component. The changes to use import type for type-only imports are a good practice for TypeScript projects. Overall, the changes improve security and maintainability, though there are a couple of areas for clarification and correction in the SQL migration files.
apps/picsa-server/supabase/migrations/20260129134800_rls_updates.sql
Outdated
Show resolved
Hide resolved
apps/picsa-server/supabase/migrations/20260128102700_deploment_admin.sql
Outdated
Show resolved
Hide resolved
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces database policies and makes related backend and frontend adjustments. The changes include adding Row Level Security (RLS) policies for several database tables and updating backend functions to use the service_role client for database interactions, which is appropriate for these administrative tasks. My review focuses on the security implications of the new RLS policies. Some policies are overly permissive, granting all authenticated users write access to critical data tables, which I've flagged as a high-severity issue. I have also provided suggestions to improve the clarity of another policy and pointed out a small redundancy in one of the backend functions.
| CREATE POLICY "climate_station_data:read_write:authenticated" ON public.climate_station_data | ||
| FOR ALL | ||
| TO authenticated USING (true) WITH CHECK (true); |
There was a problem hiding this comment.
This policy grants full SELECT, INSERT, and UPDATE access to all authenticated users on the climate_station_data table. This is a very permissive policy and effectively bypasses row-level security for any authenticated user. This pattern is repeated for other tables in this file (climate_stations, crop_data, etc.), posing a significant security risk as any authenticated user could modify critical data. It is strongly recommended to implement more restrictive policies based on user roles or data ownership. If data modification is a privileged operation, it should be handled exclusively by edge functions running with a service role, as hinted by the TODO comment at the end of the file.
| id: string, | ||
| ): Promise<{ data?: IDBClimateForecastRow; error?: any }> { | ||
| const supabaseClient = getClient(); | ||
| const supabaseClient = getServiceRoleClient(); |
There was a problem hiding this comment.
This local supabaseClient declaration is redundant. The class property this.supabaseClient is already initialized as a service role client on line 28. You can remove this line and use this.supabaseClient throughout this method to avoid unnecessary client creation and improve consistency.
| CREATE POLICY "forecasts:read:anon" ON public.forecasts | ||
| FOR ALL | ||
| TO anon USING (true) WITH CHECK (true); |
There was a problem hiding this comment.
The policy for anonymous users is defined as FOR ALL with a WITH CHECK clause. However, the anon role only has SELECT privileges on this table, which makes the policy confusing. To improve clarity and precision, it's better to define the policy as FOR SELECT and remove the WITH CHECK clause, which is only relevant for INSERT and UPDATE operations.
CREATE POLICY "forecasts:read:anon" ON public.forecasts
FOR SELECT
TO anon USING (true);
TODO
Description
Discussion
Feedback discussion points if relevant (should also tag as
Feedback Discussion)Preview
Link to app preview if relevant
Screenshots / Videos
Include at least 1-2 screenshots of videos if visual changes