feat: allow to specify obfuscation salt string by mkl262 · Pull Request #726 · librespeed/speedtest

mkl262 · 2025-10-16T17:02:29Z

PR Type

Enhancement

Description

Allow users to specify custom obfuscation salt for test ID obfuscation
Add environment variable OBFUSCATION_SALT to configure salt string
Generate PHP configuration file with custom salt when specified
Document new environment variable with format requirements

Diagram Walkthrough

flowchart LR
  ENV["OBFUSCATION_SALT env var"] -- "if specified" --> SCRIPT["entrypoint.sh"]
  SCRIPT -- "generates" --> PHP["idObfuscation_salt.php"]
  SCRIPT -- "updates" --> TELEMETRY["telemetry_settings.php"]
  DOC["doc_docker.md"] -- "documents" --> ENV

File Walkthrough

Relevant files

Enhancement

entrypoint.sh `Add custom obfuscation salt configuration generation` docker/entrypoint.sh Added conditional check for `OBFUSCATION_SALT` environment variable Generates PHP configuration file `idObfuscation_salt.php` with custom salt when provided Writes salt value to PHP file for use by obfuscation logic	+4/-0

Documentation

doc_docker.md `Document OBFUSCATION_SALT environment variable` doc_docker.md Documented new `OBFUSCATION_SALT` environment variable Specified format requirement as 2 byte hex string (e.g. `0x1234abcd`) Noted that random salt is generated if not specified	+1/-0

qodo-free-for-open-source-projects · 2025-10-16T17:02:49Z

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
⚪	Code injection risk Description: The salt value from environment variable is echoed into a PHP file without quoting or sanitization, allowing shell/word-splitting or PHP code injection if OBFUSCATION_SALT contains spaces, quotes, or PHP code (e.g., setting $OBFUSCATION_SALT = system('id'); ?>). entrypoint.sh [81-84] Referred Code if [ ! -z "$OBFUSCATION_SALT" ]; then echo "<?php" > /var/www/html/results/idObfuscation_salt.php echo "\$OBFUSCATION_SALT = $OBFUSCATION_SALT;" >> /var/www/html/results/idObfuscation_salt.php fi
Ticket Compliance
⚪	🎫 No ticket provided `- [ ] Create ticket/issue <!-- /create_ticket --create_ticket=true --> </details></td></tr>`
Codebase Duplication Compliance
⚪	Codebase context is not defined Follow the guide to enable codebase context checks.
Custom Compliance
⚪	No custom compliance provided Follow the guide to enable custom compliance check.
Update

Compliance status legend

🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

qodo-free-for-open-source-projects · 2025-10-16T17:03:41Z

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Security	✅ ~~Prevent remote code execution vulnerability~~ Suggestion Impact: The commit added a regex check for OBFUSCATION_SALT and conditional writing to the PHP file, with a warning on invalid format, matching the suggested mitigation. code diff: + if [[ "$OBFUSCATION_SALT" =~ ^0x[0-9a-fA-F]+$ ]]; then + echo "<?php" > /var/www/html/results/idObfuscation_salt.php + echo "\$OBFUSCATION_SALT = $OBFUSCATION_SALT;" >> /var/www/html/results/idObfuscation_salt.php + else + echo "WARNING: Invalid OBFUSCATION_SALT format. It must be a hex string (e.g., 0x1234abcd). Using random salt." >&2 + fi Validate the `OBFUSCATION_SALT` environment variable to prevent a remote code execution vulnerability. Ensure the variable conforms to the expected hexadecimal format before injecting it into the PHP configuration file. docker/entrypoint.sh [81-84] if [ ! -z "$OBFUSCATION_SALT" ]; then - echo "<?php" > /var/www/html/results/idObfuscation_salt.php - echo "\$OBFUSCATION_SALT = $OBFUSCATION_SALT;" >> /var/www/html/results/idObfuscation_salt.php + if [[ "$OBFUSCATION_SALT" =~ ^0x[0-9a-fA-F]+$ ]]; then + echo "<?php" > /var/www/html/results/idObfuscation_salt.php + echo "\$OBFUSCATION_SALT = $OBFUSCATION_SALT;" >> /var/www/html/results/idObfuscation_salt.php + else + echo "WARNING: Invalid OBFUSCATION_SALT format. It must be a hex string (e.g., 0x1234abcd). Using random salt." >&2 + fi fi `[Suggestion processed]` Suggestion importance[1-10]: 10 __ Why: The suggestion correctly identifies a critical remote code execution vulnerability where the unsanitized `OBFUSCATION_SALT` variable is written directly into a PHP file, and the proposed fix effectively mitigates it.	High
Security	Restrict salt file permissions Set restrictive file permissions (e.g., `chmod 600`) for the generated `idObfuscation_salt.php` file to prevent the salt from being leaked. docker/entrypoint.sh [79-85] if [ "$ENABLE_ID_OBFUSCATION" == "true" ]; then sed -i s/\$enable_id_obfuscation\ =\ .\;/\$enable_id_obfuscation\ =\ true\;/g /var/www/html/results/telemetry_settings.php if [ ! -z "$OBFUSCATION_SALT" ]; then - echo "<?php" > /var/www/html/results/idObfuscation_salt.php - echo "\$OBFUSCATION_SALT = $OBFUSCATION_SALT;" >> /var/www/html/results/idObfuscation_salt.php + escaped_salt=${OBFUSCATION_SALT//\'/\'\\\'\'} + { + echo "<?php" + echo "\$OBFUSCATION_SALT = '${escaped_salt}';" + } > /var/www/html/results/idObfuscation_salt.php + chmod 600 /var/www/html/results/idObfuscation_salt.php \|\| true fi fi Apply / Chat* Suggestion importance[1-10]: 9 __ Why: This is a critical security suggestion. The obfuscation salt is sensitive information, and leaving the file with default permissions could expose it, compromising the ID obfuscation feature.	High
Possible issue	Properly quote PHP assignment Quote and sanitize the `OBFUSCATION_SALT` value when writing it to the PHP file to prevent syntax errors. This involves wrapping the value in single quotes and escaping any existing single quotes. docker/entrypoint.sh [79-85] if [ "$ENABLE_ID_OBFUSCATION" == "true" ]; then sed -i s/\$enable_id_obfuscation\ =\ .\;/\$enable_id_obfuscation\ =\ true\;/g /var/www/html/results/telemetry_settings.php if [ ! -z "$OBFUSCATION_SALT" ]; then - echo "<?php" > /var/www/html/results/idObfuscation_salt.php - echo "\$OBFUSCATION_SALT = $OBFUSCATION_SALT;" >> /var/www/html/results/idObfuscation_salt.php + escaped_salt=${OBFUSCATION_SALT//\'/\'\\\'\'} + { + echo "<?php" + echo "\$OBFUSCATION_SALT = '${escaped_salt}';" + } > /var/www/html/results/idObfuscation_salt.php fi fi Apply / Chat* Suggestion importance[1-10]: 8 __ Why: This suggestion correctly identifies that writing `$OBFUSCATION_SALT` directly into a PHP file can cause syntax errors if the value is not a valid PHP literal, leading to a broken application. Quoting the value makes the feature robust.	Medium
Update

kontaxis · 2025-10-17T03:30:19Z

Which problem is this change trying to solve?

Right now the salt is generated internally using openssl_random_pseudo_bytes which is a good source of randomness.

Giving control to users over the source of randomness may result in weak salt.

mkl262 · 2025-10-17T12:31:57Z

Because if the container is restarted for any reason, a new salt is generated, making all previous obfuscated IDs unresolvable.

kontaxis · 2025-10-17T14:54:47Z

This is a good point, perhaps an existing salt can simply survive container restarts?

In entrypoint.sh:

# Cleanup
rm -rf /var/www/html/*

Which is why /var/www/html/results/idObfuscation_salt.php gets deleted.
Perhaps, if ENABLE_ID_OBFUSCATION, the cleanup logic preserves an existing salt file?
I.e., does something smarter than rm -rf *

Even better, an existing salt can survive as long as the database of results survives?
Perhaps ID_OBFUSCATION_SALT_FILE (in idObfuscation.php) is read from the same directory as $Sqlite_db_file?

mkl262 · 2025-10-17T22:37:04Z

The rest of the settings are configured as environment variables, so I implemented this change the same way.
I run it in kubernetes with a postgres backend, so there is no PVC mounted to the container for ay presistant storage.

Co-authored-by: qodo-merge-for-open-source[bot] <189517486+qodo-merge-for-open-source[bot]@users.noreply.github.com>

kontaxis · 2025-10-18T14:16:20Z

This change is misguided as explained above. Disappointed it has been merged.

feat: allow to specify obfuscation salt string

fd06e94

qodo-free-for-open-source-projects Bot added the Review effort 2/5 label Oct 16, 2025

qodo-free-for-open-source-projects Bot reviewed Oct 18, 2025

View reviewed changes

Comment thread docker/entrypoint.sh

Update docker/entrypoint.sh

64e4c3b

Co-authored-by: qodo-merge-for-open-source[bot] <189517486+qodo-merge-for-open-source[bot]@users.noreply.github.com>

sstidl merged commit 4458c69 into librespeed:master Oct 18, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat: allow to specify obfuscation salt string#726

feat: allow to specify obfuscation salt string#726
sstidl merged 2 commits into
librespeed:masterfrom
mkl262:master

mkl262 commented Oct 16, 2025 •

edited by qodo-free-for-open-source-projects Bot

Loading

Uh oh!

qodo-free-for-open-source-projects Bot commented Oct 16, 2025 •

edited

Loading

Uh oh!

qodo-free-for-open-source-projects Bot commented Oct 16, 2025 •

edited

Loading

Uh oh!

kontaxis commented Oct 17, 2025 •

edited

Loading

Uh oh!

mkl262 commented Oct 17, 2025

Uh oh!

kontaxis commented Oct 17, 2025 •

edited

Loading

Uh oh!

mkl262 commented Oct 17, 2025

Uh oh!

Uh oh!

kontaxis commented Oct 18, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

mkl262 commented Oct 16, 2025 • edited by qodo-free-for-open-source-projects Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Type

Description

Diagram Walkthrough

File Walkthrough

Uh oh!

qodo-free-for-open-source-projects Bot commented Oct 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Compliance Guide 🔍

Uh oh!

qodo-free-for-open-source-projects Bot commented Oct 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Code Suggestions ✨

Uh oh!

kontaxis commented Oct 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mkl262 commented Oct 17, 2025

Uh oh!

kontaxis commented Oct 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mkl262 commented Oct 17, 2025

Uh oh!

Uh oh!

kontaxis commented Oct 18, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

mkl262 commented Oct 16, 2025 •

edited by qodo-free-for-open-source-projects Bot

Loading

qodo-free-for-open-source-projects Bot commented Oct 16, 2025 •

edited

Loading

qodo-free-for-open-source-projects Bot commented Oct 16, 2025 •

edited

Loading

kontaxis commented Oct 17, 2025 •

edited

Loading

kontaxis commented Oct 17, 2025 •

edited

Loading