Skip to content

Conversation

@pcrespov
Copy link
Member

@pcrespov pcrespov commented Oct 1, 2025

Potential fix for https://github.com/ITISFoundation/osparc-simcore/security/code-scanning/96

To fix this issue, use a password/secret hashing function for generating API keys based on user input if they are intended as secrets or authentication tokens (i.e., if knowledge of the key grants access). Strong choices are Argon2, bcrypt, or PBKDF2. Since deterministic generation is needed (so the same name generates the same key), PBKDF2 is the most practical among these, allowing a fixed secret salt (ideally not public) to be set. If stricter determinism without secret salt is required, we may use SHA-2 (SHA-256/512), but only if the API key isn't used as a credential (which is discouraged). For best security, switch to PBKDF2 using your own application-wide salt (hardcoded or configurable, but never exposed to users).

Therefore, replace the SHA256 hashing with PBKDF2 using a strong ("pepper") salt and an appropriately high number of iterations for reasonable computational expense. Make sure to import and use hashlib.pbkdf2_hmac.

Changes required:

  1. Add an application pepper/salt constant at the top of the file.
  2. Replace the hashlib.sha256(name.encode()).hexdigest() call with a PBKDF2-HMAC using SHA256 and the constant salt, with a high iterations parameter.
  3. Import os if needed for salt (or simply declare a constant).
  4. Add any necessary imports (hashlib.pbkdf2_hmac is available from Python standard library ≥3.4).
  5. Ensure the generated key part is hex-encoded and truncated same as before.

Suggested fixes powered by Copilot Autofix. Review carefully before merging.

… cryptographic hashing algorithm on sensitive data

Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
@pcrespov pcrespov self-assigned this Oct 1, 2025
@pcrespov pcrespov added t:maintenance Some planned maintenance work security Pull requests that address a security vulnerability labels Oct 1, 2025
@pcrespov pcrespov marked this pull request as ready for review October 1, 2025 14:44
@pcrespov pcrespov added this to the Cheops milestone Oct 1, 2025
@sonarqubecloud
Copy link

sonarqubecloud bot commented Oct 1, 2025

@pcrespov pcrespov marked this pull request as draft October 1, 2025 14:44
Copy link
Collaborator

@matusdrobuliak66 matusdrobuliak66 left a comment

Choose a reason for hiding this comment

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

👍

@codecov
Copy link

codecov bot commented Oct 1, 2025

Codecov Report

❌ Patch coverage is 66.66667% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 87.94%. Comparing base (699808f) to head (26410a1).
⚠️ Report is 3 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #8441      +/-   ##
==========================================
+ Coverage   87.91%   87.94%   +0.03%     
==========================================
  Files        1976     1976              
  Lines       77221    77223       +2     
  Branches     1342     1342              
==========================================
+ Hits        67889    67916      +27     
+ Misses       8928     8903      -25     
  Partials      404      404              
Flag Coverage Δ
integrationtests 64.28% <ø> (+0.11%) ⬆️
unittests 86.63% <66.66%> (+0.02%) ⬆️
Components Coverage Δ
pkg_aws_library 93.59% <ø> (ø)
pkg_celery_library 83.41% <ø> (ø)
pkg_dask_task_models_library 79.33% <ø> (ø)
pkg_models_library 93.08% <66.66%> (+<0.01%) ⬆️
pkg_notifications_library 85.20% <ø> (ø)
pkg_postgres_database 87.95% <ø> (ø)
pkg_service_integration 70.19% <ø> (ø)
pkg_service_library 72.54% <ø> (ø)
pkg_settings_library 90.19% <ø> (ø)
pkg_simcore_sdk 84.99% <ø> (+0.05%) ⬆️
agent 93.53% <ø> (ø)
api_server 91.94% <ø> (ø)
autoscaling 95.74% <ø> (ø)
catalog 92.36% <ø> (ø)
clusters_keeper 99.13% <ø> (ø)
dask_sidecar 92.38% <ø> (+0.78%) ⬆️
datcore_adapter 97.94% <ø> (ø)
director 75.81% <ø> (ø)
director_v2 90.92% <ø> (+0.02%) ⬆️
dynamic_scheduler 96.68% <ø> (ø)
dynamic_sidecar 90.43% <ø> (ø)
efs_guardian 89.62% <ø> (ø)
invitations 91.44% <ø> (ø)
payments 92.62% <ø> (ø)
resource_usage_tracker 92.34% <ø> (+0.21%) ⬆️
storage 86.53% <ø> (+0.08%) ⬆️
webclient ∅ <ø> (∅)
webserver 87.73% <ø> (+0.03%) ⬆️

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 699808f...26410a1. Read the comment docs.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@mergify
Copy link
Contributor

mergify bot commented Oct 1, 2025

🧪 CI Insights

Here's what we observed from your CI run for 26410a1.

❌ Job Failures

Pipeline Job Health on master Retries 🔍 CI Insights 📄 Logs
CI system-tests Broken 0 View View

@pcrespov
Copy link
Member Author

pcrespov commented Oct 3, 2025

false positive.

@pcrespov pcrespov closed this Oct 3, 2025
@pcrespov pcrespov deleted the alert-autofix-96 branch October 3, 2025 11:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

security Pull requests that address a security vulnerability t:maintenance Some planned maintenance work

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants