Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

url to reset #10402

Closed
wants to merge 5 commits into from
Closed

url to reset #10402

wants to merge 5 commits into from

Conversation

Xyzrr
Copy link

@Xyzrr Xyzrr commented Feb 22, 2025

has a demo sql query

Copy link
Contributor

Warnings
⚠️

Changes were made to package.json, but not to yarn.lock - Perhaps you need to run yarn install?

Welcome!

Hello there, congrats on your first PR! We're excited to have you contributing to this project.
By submitting your Pull Request, you acknowledge that you agree with the terms of our Contributor License Agreement.

Generated by 🚫 dangerJS against 4f6c676

@Xyzrr Xyzrr closed this Feb 22, 2025
Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

This PR introduces a reset endpoint with significant security concerns and database configuration changes.

  • Critical: The new ResetController exposes an unprotected GET endpoint that executes raw SQL queries with a hardcoded workspace ID, creating severe security vulnerabilities
  • The PostgreSQL image change to twentycrm/twenty-postgres-spilo:latest has mismatched environment variable conventions between render.yaml and docker-compose.yml
  • The reset endpoint returns sensitive database query results without proper error handling or sanitization
  • The commented memory limits in docker-compose.yml (32mb) seem unrealistically low for production services
  • The hardcoded schema name workspace_1wgvd1injqtife6y4rvfbu3h5 in the SQL query suggests improper multi-workspace handling

💡 (1/5) You can manually trigger the bot by mentioning @greptileai in a comment!

5 file(s) reviewed, 2 comment(s)
Edit PR Review Bot Settings | Greptile

const dataSource = await this.twentyORMGlobalManager.getDataSourceForWorkspace('3b8e6458-5fc1-4e63-8563-008ccddaa6db');

const result = await dataSource.query('SELECT * FROM workspace_1wgvd1injqtife6y4rvfbu3h5.person');
console.log(result);
Copy link
Contributor

Choose a reason for hiding this comment

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

style: Remove console.log before deploying to production

Suggested change
console.log(result);
// Remove debug logging

@@ -28,6 +28,7 @@ import { TwentyORMModule } from 'src/engine/twenty-orm/twenty-orm.module';
import { WorkspaceCacheStorageModule } from 'src/engine/workspace-cache-storage/workspace-cache-storage.module';
import { ModulesModule } from 'src/modules/modules.module';

import { ResetController } from './core/reset/reset.controller';
Copy link
Contributor

Choose a reason for hiding this comment

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

style: Consider moving reset functionality into a dedicated module rather than directly in core

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.

2 participants