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.

Sign up for GitHub

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

Jump to bottom

Updated NeLS Integration #2167

Draft
wants to merge 170 commits into
base: main
Choose a base branch
from
Draft

Updated NeLS Integration #2167

wants to merge 170 commits into from

Conversation

stuzart
Copy link
Member

@stuzart stuzart commented Mar 12, 2025

No description provided.

aapaolaza and others added 30 commits December 13, 2021 10:47
@aapaolaza
Working prototype for NeLS integration
48e6d86 
Waiting on create dataset, upload metadata API support
@aapaolaza
Fixed create dataset route
21008da
@aapaolaza
Adds download metadata feature
3994fd7
@aapaolaza
Update docker image name
8e43433
@aapaolaza
Adds upload metadata
400481d
@aapaolaza
Removes development logs
def822f
@aapaolaza
Adds tree history and subtypes
9d3d414
@aapaolaza
Add subtype file explorer mockup, add subtype tree, add tree state
bb0260c
@aapaolaza
Tweaked subtype folder browser prototype
e6f2d3f
@aapaolaza
Adds js tree icons
a3069a3
@aapaolaza
Replace project handlebar with partial; adds new dataset to project
60132a6
@aapaolaza
Fix duplicate project -> create dataset button typo
e170018
@aapaolaza
Adds folder browsing prototype
3bc5551
@aapaolaza
Adds fully working file browser
2db836e
@aapaolaza
Merge branch 'master' into NeLS-integration
032f33a
@aapaolaza
Adds prototypes to upload data, and explore/register modes
62f2a87
@aapaolaza
Routes test to make nels browser independent
7e952c8
@aapaolaza
NeLS register/browse mode switches on use of assay_id param
00cfd5e
@aapaolaza
Added upload/download suggested code
9323b74
@stuzart
Merge branch 'master' into NeLS-integration-test
b105ba0
@stuzart
Merge branch 'master' into NeLS-integration-test
09ebb1c
@stuzart
Merge branch 'seek-1.13' into NeLS-integration-test
51f0dc1
@stuzart
fix to nels content blob test
f0469bc
@stuzart
fix to the assays/nels nested route, and fixes assay tests
7a08acf
@stuzart
updated check_metadata_exists and added test and vcr cassette
71f69e5
@stuzart
fix typo in integration test
d4244e0
@stuzart
fix nels controller tests
dc9ff05
@stuzart
partial work testing the download file api
4558d37 
waiting for info before finishing off
@stuzart
completed initial test case for download_file
46e2580
@stuzart
client helper and test case for uploading a file
85648a8
stuzart added 17 commits October 16, 2024 10:47
@stuzart
Merge branch 'seek-1.15' into NeLS-integration
e823755
@stuzart
Merge branch 'main' into NeLS-integration
9545b0c
@stuzart
Merge branch 'main' into NeLS-integration
673a5b7
@stuzart
update the endpoint to get the list of projects
875d300
@stuzart
updated upload metadata to take token
4e8c2a1
@stuzart
islocked in api response is now all lowercase
1cebe4c
@stuzart
fix changed endpoint in vcr cassette for get projects
98074b4
@stuzart
change to v2 endpoint to get project details, for access to membershi…
84b6bf8 
…p_type
@stuzart
update UI to include membership_type when determining if a dataset is…
bbdedf6 
… locked.
@stuzart
updated tests and cassettes for checking locked status
a507870
@stuzart
Merge branch 'main' into NeLS-integration
ec953e5
@stuzart
Merge branch 'seek-1.16' into NeLS-integration
689d5b9
@stuzart
Merge branch 'main' into NeLS-integration
f93777c 
# Conflicts:
#	app/controllers/nels_controller.rb
#	config/version.yml
#	docker-compose.yml
@stuzart
test needed assertion and also updated DummyClient
55d45f7
@stuzart
Merge branch 'main' into NeLS-integration
9db0cd7
@stuzart
fix the upload metadata dialog text and remove option to add by URL
ccb760a
@stuzart
Merge branch 'main' into NeLS-integration
fb19c5e 
# Conflicts:
#	docker-compose.yml
@stuzart stuzart added this to the 1.17.0 milestone Mar 12, 2025
@stuzart stuzart marked this pull request as draft March 12, 2025 11:30
github-advanced-security[bot]
github-advanced-security bot found potential problems Mar 12, 2025
View reviewed changes
github-advanced-security[bot]
github-advanced-security bot found potential problems Mar 18, 2025
View reviewed changes
app/controllers/nels_controller.rb
raise Nels::Rest::Client::FetchFileError, 'temp copy of file doesnt exist' unless File.exist?(path)

# send_data blocks and allows the file to be cleaned up afterwards
File.open(path, 'r') do |f|

Check failure

Code scanning / CodeQL

Uncontrolled data used in path expression High

This path depends on a
user-provided value
Loading
.

Copilot Autofix AI 11 days ago

To fix the problem, we need to ensure that the key parameter is properly validated and sanitized to prevent any form of path manipulation. We can achieve this by using a whitelist approach to only allow known good patterns for the key parameter. Additionally, we should ensure that the path is constructed in a way that prevents directory traversal attacks.

The best way to fix the problem is to:

  1. Validate the key parameter using a strict whitelist of allowed characters.
  2. Ensure that the path is constructed in a secure manner that prevents directory traversal.
Suggested changeset 1
app/controllers/nels_controller.rb

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/app/controllers/nels_controller.rb b/app/controllers/nels_controller.rb
--- a/app/controllers/nels_controller.rb
+++ b/app/controllers/nels_controller.rb
@@ -147,8 +147,7 @@
 
-    unless key =~ /^[0-9a-z-]+$/
+    unless key =~ /\A[0-9a-z-]+\z/
       raise Nels::Rest::Client::FetchFileError, 'Invalid file key'
     end
-    sanitized_key = key.gsub(/[^0-9a-z-]/, '')
-    path = File.join("/tmp", "nels-download-#{sanitized_key}")
-    raise Nels::Rest::Client::FetchFileError, 'temp copy of file doesnt exist' unless File.exist?(path)
+    path = File.join("/tmp", "nels-download-#{key}")
+    raise Nels::Rest::Client::FetchFileError, 'temp copy of file doesnt exist' unless File.exist?(path) && path.start_with?("/tmp/nels-download-")
 
EOF
@@ -147,8 +147,7 @@

unless key =~ /^[0-9a-z-]+$/
unless key =~ /\A[0-9a-z-]+\z/
raise Nels::Rest::Client::FetchFileError, 'Invalid file key'
end
sanitized_key = key.gsub(/[^0-9a-z-]/, '')
path = File.join("/tmp", "nels-download-#{sanitized_key}")
raise Nels::Rest::Client::FetchFileError, 'temp copy of file doesnt exist' unless File.exist?(path)
path = File.join("/tmp", "nels-download-#{key}")
raise Nels::Rest::Client::FetchFileError, 'temp copy of file doesnt exist' unless File.exist?(path) && path.start_with?("/tmp/nels-download-")

Copilot is powered by AI and may make mistakes. Always verify output.
@stuzart stuzart committed this autofix suggestion 11 days ago.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
1.17.0
Development

Successfully merging this pull request may close these issues.
2 participants
@stuzart @aapaolaza