Skip to content

Don't follow symlinks when writing htop_history #1945

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

Open
Explorer09 wants to merge 1 commit into
base: main
Choose a base branch
from
+1 −1
Open

Don't follow symlinks when writing htop_history #1945

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion History.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ void History_save(const History* this) {
if (!this->filename)
return;
/* Settings_write writes things via a temp file & rename, we do it less robust but faster here: */
int fd = open(this->filename, O_WRONLY | O_CREAT | O_TRUNC, 0600);
int fd = open(this->filename, O_WRONLY | O_CREAT | O_TRUNC | O_NOFOLLOW, 0600);

@coderabbitai coderabbitai Bot May 17, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Search for Settings_resolveSymlink function definition
ast-grep --pattern $'Settings_resolveSymlink($$$) {
  $$$
}'

# Also search for its usage and declaration
rg -n 'Settings_resolveSymlink' --type=c -C3

Repository: htop-dev/htop

Length of output: 39

🏁 Script executed:

#!/bin/bash
# Find O_NOFOLLOW usage to check for platform guards or fallbacks
rg -n 'O_NOFOLLOW' --type=c -C2

# Check for any HAVE_O_NOFOLLOW or similar feature macros
rg -n 'HAVE.*NOFOLLOW|O_NOFOLLOW.*ifdef' --type=c

Repository: htop-dev/htop

Length of output: 2132

🏁 Script executed:

cat -n History.c | head -100

Repository: htop-dev/htop

Length of output: 3290

History_load() also needs O_NOFOLLOW for consistency.

History_load() uses fopen() (line 25), which follows symlinks, while History_save() now uses open() with O_NOFOLLOW (line 72), preventing symlink following. A user with an intentional symlink will see history loaded successfully but updates silently fail to persist, causing unexpected data loss.

Apply O_NOFOLLOW to History_load() as well by switching from fopen() to open() with appropriate flags. This matches the existing pattern in Settings.c (line 326), which already uses O_NOFOLLOW for file access.

if (fd == -1)
return;
FILE* fp = fdopen(fd, "w");
Expand Down
Loading