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

Add database backup route #417

Merged
merged 7 commits into from
Nov 19, 2022
Merged

Add database backup route #417

merged 7 commits into from
Nov 19, 2022

Conversation

dentarg
Copy link
Member

@dentarg dentarg commented Nov 13, 2022

Close #413

The SQL dump can be downloaded with

curl --verbose --silent --location --request POST --user user:pass --output test.sql http://localhost:8080/.backup

@dentarg dentarg requested a review from jage November 13, 2022 18:48
@dentarg
Copy link
Member Author

dentarg commented Nov 13, 2022

Warnings seen in CI

/home/runner/work/wikimum/wikimum/test/integration/app_backup_test.rb:43: warning: ambiguous first argument; put parentheses or a space even after `/' operator
/home/runner/work/wikimum/wikimum/test/integration/app_backup_test.rb:67: warning: ambiguous first argument; put parentheses or a space even after `/' operator

@dentarg dentarg mentioned this pull request Nov 13, 2022
16 tasks
dentarg added a commit that referenced this pull request Nov 14, 2022
Limit file access to the temporary directory we create.

Hopefully addresses the code scanning alert about
https://brakemanscanner.org/docs/warning_types/file_access/

Suggested by @jage at #417 (comment)
rel_path = Base64.urlsafe_decode64(params.fetch("encoded_path"))
dump_path = File.join(backup_tmpdir, rel_path)

send_file dump_path, type: "text/plain"

Check notice

Code scanning / Brakeman

Parameter value used in file name.

Parameter value used in file name.
@dentarg
Copy link
Member Author

dentarg commented Nov 14, 2022

Looked at ignoring the warning from Brakeman now that we have adequately addressed it. Not pleased with what I found 😞

@dentarg
Copy link
Member Author

dentarg commented Nov 14, 2022

Looks like the ignore is kinda specific, adding

  get '/foo/:foo' do
    send_file params["foo"]
  end

to another file, and also the same file, generates a new warning (not ignored). Adding some lines of code to the post "/download/:encoded_path" do block does not generate a new warning (it stays ignored).

@dentarg
Copy link
Member Author

dentarg commented Nov 14, 2022

@dentarg
Copy link
Member Author

dentarg commented Nov 14, 2022

Another disappointment, GitHub doesn't support suppressions in the uploaded SARIF 😞 github/codeql-action#1230 (comment)

@dentarg
Copy link
Member Author

dentarg commented Nov 14, 2022

I dismissed the alert now, it had me give a reason (gave it The file access is limited to a temporary directory we create.), had hope it would communicate that clearly in the PR, but nope...

Close #413

The SQL dump can be downloaded with

    curl --verbose --silent --location --request POST --user user:pass --output test.sql http://localhost:8080/.backup
/home/runner/work/wikimum/wikimum/test/integration/app_backup_test.rb:43: warning: ambiguous first argument; put parentheses or a space even after `/' operator
/home/runner/work/wikimum/wikimum/test/integration/app_backup_test.rb:67: warning: ambiguous first argument; put parentheses or a space even after `/' operator
Now the temporary file will be automatically deleted when the Ruby
interpreter exits.

https://ruby-doc.org/stdlib-2.7.6/libdoc/tempfile/rdoc/Tempfile.html#new-method
Limit file access to the temporary directory we create.

Hopefully addresses the code scanning alert about
https://brakemanscanner.org/docs/warning_types/file_access/

Suggested by @jage at #417 (comment)
The file access is limited to a temporary directory we create.
This string ends up in the "justification" attribute in the SARIF
output, and apparently it is not valid SARIF if it isn't a string (it
was null).

From https://github.com/Starkast/wikimum/actions/runs/3465429770/jobs/5788144432#step:6:26

> Error: Unable to upload "output.sarif.json" as it is not valid SARIF:
@dentarg dentarg merged commit 49b0656 into master Nov 19, 2022
@dentarg dentarg deleted the database-backup branch November 19, 2022 12:20
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.

Offsite database backup
2 participants