fix(agentlog): use proper file permissions when appending logs

[shjala]

Description

Bug report

When we run “Reboot reason“ command from Edge View, the command fails due to permission error:

reboot-reason.log
open /persist/log/reboot-reason.log: permission denied

When i check the file on eve I see that the file does not have any permission s set :

b8519a4e-1ba1-48a2-ae74-0d7668fb4457:~# ls -l /persist/log/reboot-reason.log
----------    1 root     root           621 Dec 12 04:59 /persist/log/reboot-reason.log
b8519a4e-1ba1-48a2-ae74-0d7668fb4457:~#

Fix

RebootReason function in EdgeView creates /persist/log/reboot-reason.log by calling printToFile, printToFile creates the file if it doesn't exist, incorrectly passing os.ModeAppend as permission. This will lead to unix perm being 000, hence no permission.

This PR replace os.ModeAppend with 0644 in OpenFile to correctly set Unix rw-r--r-- permissions when creating or appending to log files.

PR dependencies

None

How to test and validate this PR

Apply the fixes, or use an image with the fix in place and run "Reboot reason“ command from Edge View, it should succeed.

Changelog notes

Fix “Reboot reason” command in EdgeView failing due to log file permission errors

PR Backports

To be determined.

Checklist

• I've provided a proper description
• I've added the proper documentation
• I've tested my PR on amd64 device
• I've tested my PR on arm64 device
• I've written the test verification instructions
• I've set the proper labels to this PR

For backport PRs (remove it if it's not a backport):

• I've added a reference link to the original PR
• PR's title follows the template

And the last but not least:

• I've checked the boxes above, or I've provided a good reason why I didn't
  check them.

Please, check the boxes above after submitting the PR in interactive mode.

[eriknordmark]

You need to fix the file in pkg/pillar/agentlog/agentlog.c (as one commit), and then
run make bump-eve-pillar
which will update edgeview etc vendoring of pillar and commit those.
[I think that will make the go.mod/go.sum shas be correct without using two separate PRs.]

Is there other code which has the same bug in the use of os.OpenFile?

[europaul]

You need to fix the file in pkg/pillar/agentlog/agentlog.c (as one commit), and then run make bump-eve-pillar which will update edgeview etc vendoring of pillar and commit those. [I think that will make the go.mod/go.sum shas be correct without using two separate PRs.]

Is there other code which has the same bug in the use of os.OpenFile?

as far as I know there is no way to do it in one PR: the go package needs to be published first for us to know which hash it's going to have. make bump-eve-pillar is getting that hash from the go pkg database online, not locally.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 28.08%. Comparing base (2281599) to head (02eaf7c).
⚠️ Report is 159 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #5487      +/-   ##
==========================================
+ Coverage   19.52%   28.08%   +8.55%     
==========================================
  Files          19       19              
  Lines        3021     2314     -707     
==========================================
+ Hits          590      650      +60     
+ Misses       2310     1520     -790     
- Partials      121      144      +23     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[shjala]

You need to fix the file in pkg/pillar/agentlog/agentlog.c

I can't find this file :/

[shjala]

You need to fix the file in pkg/pillar/agentlog/agentlog.c (as one commit), and then run make bump-eve-pillar which will update edgeview etc vendoring of pillar and commit those. [I think that will make the go.mod/go.sum shas be correct without using two separate PRs.]

Is there other code which has the same bug in the use of os.OpenFile?

Oh shoot, I see now! I fixed in the vendor directory! sorry I just search for the function and picked the wrong file!

[shjala]

Is there other code which has the same bug in the use of os.OpenFile?

at least Semgrep can't find anything obvious. It is not possible to do bitwise check on flag value in Semgrep so this rule is limited to string matching.

shah in 🌐 shah-MS-7E12 in eve on  fix.printToFile [$?] took 5s 
❯ semgrep --config=tests/semgrep-rules/os-openfile-non-perm-mode.yaml .

┌──── ○○○ ────┐
│ Semgrep CLI │
└─────────────┘

/home/shah/.local/share/pipx/venvs/semgrep/lib/python3.12/site-packages/opentelemetry/instrumentation/dependencies.py:4: UserWarning: pkg_resources is deprecated as an API. See https://setuptools.pypa.io/en/latest/pkg_resources.html. The pkg_resources package is slated for removal as early as 2025-11-30. Refrain from using this package or pin to Setuptools<81.
  from pkg_resources import (
                                                                                                                        
Scanning 1525 files (only git-tracked) with 1 Code rule:
            
  CODE RULES
  Scanning 567 files.
                    
  SUPPLY CHAIN RULES
                  
  No rules to run.
                  
          
  PROGRESS
   
  ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 100% 0:00:00                                                                                                                        
                
                
┌──────────────┐
│ Scan Summary │
└──────────────┘
✅ Scan completed successfully.
 • Findings: 0 (0 blocking)
 • Rules run: 1
 • Targets scanned: 567
 • Parsed lines: ~100.0%
 • Scan skipped: 
   ◦ Files larger than  files 1.0 MB: 7
   ◦ Files matching .semgrepignore patterns: 34102
 • Scan was limited to files tracked by git
 • For a detailed list of skipped files and lines, run semgrep with the --verbose flag
Ran 1 rule on 567 files: 0 findings.

⏫ A new version of Semgrep is available. See https://semgrep.dev/docs/upgrading

✨ If Semgrep missed a finding, please send us feedback to let us know!
   See https://semgrep.dev/docs/reporting-false-negatives/

shah in 🌐 shah-MS-7E12 in eve on  fix.printToFile [$+?] took 5s 
❯ 

[eriknordmark]

You need to fix the file in pkg/pillar/agentlog/agentlog.c

I can't find this file :/

Wrong programming language on my part ;-)

[eriknordmark]

LGTM

[rene]

@shjala tests/semgrep-rules/os-openfile-non-perm-mode.yaml is missing Copyright note.

Please, once we merge it, don't forget to open the PR updating vendor files.... in all dependencies (pkg/edgeview, etc).

[shjala]

@shjala tests/semgrep-rules/os-openfile-non-perm-mode.yaml is missing Copyright note.

@rene added the Copyright note.

Please, once we merge it, don't forget to open the PR updating vendor files.... in all dependencies (pkg/edgeview, etc).

thanks for the reminder, do we have a script for this?

[eriknordmark]

thanks for the reminder, do we have a script for this?

make bump-eve-pilllar (or something like that)