Skip to content

fix: Replaced os.popen() with Python's inbuilt file handling#897

Open
S1DDHEY wants to merge 1 commit intoSeedSigner:devfrom
S1DDHEY:fix/replace-deprecated-os.popen
Open

fix: Replaced os.popen() with Python's inbuilt file handling#897
S1DDHEY wants to merge 1 commit intoSeedSigner:devfrom
S1DDHEY:fix/replace-deprecated-os.popen

Conversation

@S1DDHEY
Copy link
Copy Markdown

@S1DDHEY S1DDHEY commented Apr 4, 2026

Description

Problem or Issue being addressed

Resolves #856

In tools_views.py, the entropy generation logic uses os.popen("cat /proc/cpuinfo | grep Serial") to gather the CPU serial number for mixing into the seed entropy hash. os.popen() implicitly spawns a shell, which is unnecessary here since the data can be read directly via Python's built-in file I/O.

Solution

Replaced the os.popen() call with native Python file I/O using open("/proc/cpuinfo", "r"). This reads the file line by line and extracts the Serial field without spawning any external processes (cat, grep) or a shell.

Also removed the now-unused import os at the top of the file, as no other code in the file depends on it.

Additional Information

  • The new implementation selects the first Serial entry (via break), whereas the previous os.popen approach would effectively select the last one. In practice, cpuinfo contains only a single Serial field, so the behavior is equivalent on all current hardware.
  • The existing try/except block is preserved, so on non-Raspberry Pi hardware (or any system without cpuinfo), the code gracefully falls back to hash_bytes = b'0' as before.
  • os.popen() is not strictly deprecated but is discouraged in favor of subprocess.run() or native file I/O. Since we're simply reading a local file, native file I/O is the simplest and most direct approach — no shell or subprocess needed.

Screenshots

N/A


This pull request is categorized as a:

  • New feature
  • Bug fix
  • Code refactor
  • Documentation
  • Other

Checklist

I ran pytest locally

  • All tests passed before submitting the PR
  • I couldn't run the tests
  • N/A

I included screenshots of any new or modified screens

Should be part of the PR description above.

  • Yes
  • No
  • N/A

I added or updated tests

Any new or altered functionality should be covered in a unit test. Any new or updated sequences require FlowTests.

  • Yes
  • No, I’m a fool
  • N/A

I tested this PR hands-on on the following platform(s):


I have reviewed these notes:

  • Keep your changes limited in scope.
  • If you uncover other issues or improvements along the way, ideally submit those as a separate PR.
  • The more complicated the PR, the harder it is to review, test, and merge.
  • We appreciate your efforts, but we're a small team of volunteers so PR review can be a very slow process.
  • Please only "@" mention a contributor if their input is truly needed to enable further progress.
  • I understand

Thank you! Please join our Devs' Telegram group to get more involved.

Copy link
Copy Markdown
Contributor

@PROWLERx15 PROWLERx15 left a comment

Choose a reason for hiding this comment

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

This appears to be a straightforward cleanup to me.

Reading /proc/cpuinfo directly is simpler and more appropriate than shelling out to cat | grep for this.

I do not see any obvious issue with the change. Main thing worth confirming is the equivalence on Rasp Pi, but the approach itself looks good.

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.

Harden: Replace os.popen with Python file handling in tools_views.py

2 participants