Skip to content

refactor: move config to user dir and enforce owner-only perms#1078

Open
madhavilosetty-intel wants to merge 2 commits into
mainfrom
configUser
Open

refactor: move config to user dir and enforce owner-only perms#1078
madhavilosetty-intel wants to merge 2 commits into
mainfrom
configUser

Conversation

@madhavilosetty-intel

Copy link
Copy Markdown
Contributor

No description provided.

@madhavilosetty-intel madhavilosetty-intel requested a review from a team as a code owner June 15, 2026 23:29
@codecov

codecov Bot commented Jun 15, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 58.69565% with 19 lines in your changes missing coverage. Please review.
✅ Project coverage is 43.49%. Comparing base (e7b1869) to head (8d58e17).

Files with missing lines Patch % Lines
config/config.go 58.69% 13 Missing and 6 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1078      +/-   ##
==========================================
+ Coverage   43.44%   43.49%   +0.05%     
==========================================
  Files         143      143              
  Lines       13621    13663      +42     
==========================================
+ Hits         5917     5943      +26     
- Misses       7143     7155      +12     
- Partials      561      565       +4     

☔ View full report in Codecov by Harness.
📢 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.

@sudhir-intc

Copy link
Copy Markdown
Contributor

@madhavilosetty-intel : could you please add some context to this PR on why this PR is needed. Is it an enhancement or a fix.
Incase of feature/enhancement - could you please link the github issue.
Incase of an issue - what issue is this PR fixing ?

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Moves the tray-mode configuration file into the per-user config directory (aligned with where the embedded SQLite DB lives) and adds a one-time migration path so installer-provisioned credentials carry over on first tray launch.

Changes:

  • Update config-path resolution so tray mode prefers <UserConfigDir>/device-management-toolkit/config.yml, otherwise fall back beside the binary.
  • Add seedConfig to copy an installer-provisioned config into the per-user location on first tray run.
  • Add unit tests covering config path resolution and seeding behavior.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
config/config.go Adds per-user config path resolution for tray mode and seeds per-user config from beside-binary config.
config/config_test.go Adds tests for tray/non-tray config path resolution and for seedConfig behavior.

Comment thread config/config.go
Comment thread config/config_test.go
@madhavilosetty-intel

Copy link
Copy Markdown
Contributor Author

@madhavilosetty-intel : could you please add some context to this PR on why this PR is needed. Is it an enhancement or a fix. Incase of feature/enhancement - could you please link the github issue. Incase of an issue - what issue is this PR fixing ?

@sudhir-intc This is for the installers, at present config file will be in the app’s install folder, which anyone could read. Now it’s stored securely in each user’s own space. I link this PR to #870

madhavilosetty-intel pushed a commit that referenced this pull request Jun 19, 2026
Address PR #1078 review findings:
- writeConfig: generate path now writes 0o600/0o700 (was 0o666/0o777),
  so a freshly generated config's secrets are not world-readable.
- seedConfig: surface unexpected os.Stat errors instead of treating any
  stat error as "missing" and silently re-seeding.
- Share configFilePerm/configDirPerm constants across both paths.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@madhavilosetty-intel madhavilosetty-intel marked this pull request as draft June 19, 2026 09:51
@madhavilosetty-intel madhavilosetty-intel force-pushed the configUser branch 4 times, most recently from 131d8a8 to 48f48b2 Compare June 24, 2026 18:01
@madhavilosetty-intel madhavilosetty-intel changed the title refactor: move config to user dir refactor: move config to user dir and enforce owner-only perms Jun 24, 2026
@madhavilosetty-intel madhavilosetty-intel force-pushed the configUser branch 3 times, most recently from ca05b67 to 7e130bf Compare June 24, 2026 18:44

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.

Comment thread config/config.go
Comment thread config/config.go
Comment thread config/config.go
Comment thread config/config.go
Comment thread config/config.go
Relocate config to the user directory and harden seedConfig error
handling, enforcing owner-only file permissions.
@madhavilosetty-intel madhavilosetty-intel marked this pull request as ready for review June 25, 2026 21:27
@shaoboon

Copy link
Copy Markdown
Contributor

tested on linux, results look good

both user config path and machine config path are not exist

  • user config path generated with correct permission

user config path exist, but machine config path not exist

  • user config path reused without generate

user config path not exist, but machine config path exist

  • machine config path content is copied over to user config path with correct permission

both user config path and machine config path are exist

  • user config path reused without generate

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.

5 participants