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

Develop #624

Merged
merged 14 commits into from
Nov 11, 2024
Merged

Develop #624

merged 14 commits into from
Nov 11, 2024

Conversation

iandjx
Copy link
Collaborator

@iandjx iandjx commented Oct 30, 2024

No description provided.

@@ -37,6 +40,12 @@
const [deletingMap, setDeletingMap] = useState({});
const [confirmClose, setConfirmClose] = useState(false);

const goToDatasourcePage = (id: string) => {
if (isAirbyteEnabled) {
router.push(`/${resourceSlug}/datasource/${id}`);

Check warning

Code scanning / CodeQL

Client-side URL redirect Medium

Untrusted URL redirection depends on a
user-provided value
.

Copilot Autofix AI 3 days ago

To fix the problem, we should avoid using user input directly in constructing the redirect URL. Instead, we can maintain a list of authorized redirects and choose from that list based on the user input. This ensures that only valid and safe URLs are used for redirection.

  1. Create a list of authorized resourceSlug values.
  2. Check if the resourceSlug from router.query is in the list of authorized values.
  3. Only perform the redirection if the resourceSlug is authorized.
Suggested changeset 1
webapp/src/components/DatasourceTable.tsx

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/webapp/src/components/DatasourceTable.tsx b/webapp/src/components/DatasourceTable.tsx
--- a/webapp/src/components/DatasourceTable.tsx
+++ b/webapp/src/components/DatasourceTable.tsx
@@ -42,5 +42,8 @@
 
+	const authorizedResourceSlugs = ['validSlug1', 'validSlug2']; // Add all authorized slugs here
 	const goToDatasourcePage = (id: string) => {
-		if (isAirbyteEnabled) {
+		if (isAirbyteEnabled && authorizedResourceSlugs.includes(resourceSlug)) {
 			router.push(`/${resourceSlug}/datasource/${id}`);
+		} else {
+			toast.error('Unauthorized resource slug');
 		}
EOF
@@ -42,5 +42,8 @@

const authorizedResourceSlugs = ['validSlug1', 'validSlug2']; // Add all authorized slugs here
const goToDatasourcePage = (id: string) => {
if (isAirbyteEnabled) {
if (isAirbyteEnabled && authorizedResourceSlugs.includes(resourceSlug)) {
router.push(`/${resourceSlug}/datasource/${id}`);
} else {
toast.error('Unauthorized resource slug');
}
Copilot is powered by AI and may make mistakes. Always verify output.
Positive Feedback
Negative Feedback

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Please select one or more of the options
@ragyabraham ragyabraham self-requested a review November 11, 2024 22:19
@ragyabraham ragyabraham marked this pull request as ready for review November 11, 2024 22:20
Copy link

File Coverage
All files 96%
src/lib/utils/validationutils.ts 95%

Minimum allowed coverage is 80%

Generated by 🐒 cobertura-action against 64d54c0

Copy link
Contributor

@ragyabraham ragyabraham left a comment

Choose a reason for hiding this comment

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

This is a big merge... each component has been individually tested. Code LGTM

@ragyabraham ragyabraham merged commit ca9e5de into master Nov 11, 2024
8 checks passed
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