Skip to content

fix(security): salt device hash for telemetry#883

Merged
aeppling merged 5 commits intodevelopfrom
hotfix/telemetry-salt-hash
Mar 28, 2026
Merged

fix(security): salt device hash for telemetry#883
aeppling merged 5 commits intodevelopfrom
hotfix/telemetry-salt-hash

Conversation

@aeppling
Copy link
Copy Markdown
Contributor

Add per-user random salt to generate_device_hash() in telemetry.
Salt stored in user machine.

Copy link
Copy Markdown
Collaborator

@pszymkowiak pszymkowiak left a comment

Choose a reason for hiding this comment

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

Good security improvement @aeppling — salting the device hash is the right call. The OnceLock cache, getrandom choice, and fallback path are all solid.

P1 — Backward compatibility: all device hashes change

Every existing user gets a new hash after upgrade (salt didn't exist before). The backend telemetry will see all users as "new devices" on the day they upgrade. Is this expected? If RTK Cloud tracks device count trends, there will be a one-time spike. Not a blocker but the backend should be aware.

P1 — Salt file permissions

.device_salt contains a crypto secret. On shared machines, another user could read it and reconstruct the device hash. Set 0600 after creation:

#[cfg(unix)]
{
    use std::os::unix::fs::PermissionsExt;
    let _ = std::fs::set_permissions(&salt_path, std::fs::Permissions::from_mode(0o600));
}

P2 — Dotfile naming

.device_salt starts with . (hidden) but lives in ~/.local/share/rtk/ where files are normally not hidden. Minor — consider device_salt (no dot) or leave as-is.

P2 — Init telemetry message

The config::telemetry_enabled() check in init.rs is a nice touch but it's a functional change hidden in a security PR. Consider noting it in the PR description.

Overall solid — fix the file permissions and we're good.

@aeppling
Copy link
Copy Markdown
Contributor Author

@pszymkowiak resolved, except for hidden file which is fine

@aeppling aeppling merged commit 2d2f199 into develop Mar 28, 2026
9 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