fix(cli): use HOME env instead of hardcoded "/home/" path prefix check#1719
Conversation
dengbo11
commented
Jul 2, 2026
dengbo11
commented
- Replace hardcoded "/home/" string prefix check with HOME environment variable to support non-standard home directories
- Add null and empty string checks for HOME env to handle edge cases
- Append trailing slash to HOME path before prefix matching to avoid false positive on paths like /home/bob-xxx
- Fix minor indentation on LogE call
- Replace hardcoded "/home/" string prefix check with HOME environment variable to support non-standard home directories - Add null and empty string checks for HOME env to handle edge cases - Append trailing slash to HOME path before prefix matching to avoid false positive on paths like /home/bob-xxx - Fix minor indentation on LogE call
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: dengbo11 The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request replaces the hardcoded /home/ path check with a dynamic check using the HOME environment variable to avoid mapping files under the user's home directory. The review feedback correctly identifies that returning early when HOME is not set or empty will break file mapping in environments where HOME is undefined; instead, the check should be skipped so mapping can proceed.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| auto *homePath = ::getenv("HOME"); | ||
| if (homePath == nullptr || homePath[0] == '\0') { | ||
| LogE("failed to get HOME env"); | ||
| return target; | ||
| } | ||
|
|
||
| // Don't map files under the user's home directory | ||
| auto homeStr = std::string(homePath); | ||
| if (homeStr.back() != '/') { | ||
| homeStr.push_back('/'); | ||
| } | ||
| if (auto tmp = target.string(); tmp.rfind(homeStr, 0) == 0) { | ||
| return target; | ||
| } |
There was a problem hiding this comment.
If the HOME environment variable is not set or is empty, returning target early from Cli::mappingFile will completely bypass the file mapping logic (which maps files to /run/host/rootfs/...). This means that in environments where HOME is not defined (such as some systemd services, cron jobs, or restricted sandboxes), absolute paths will not be mapped at all, breaking core functionality.
Instead of returning target early when HOME is unavailable, we should simply skip the home directory check and proceed with the file mapping.
auto *homePath = ::getenv("HOME");
if (homePath != nullptr && homePath[0] != '\0') {
// Don't map files under the user's home directory
auto homeStr = std::string(homePath);
if (homeStr.back() != '/') {
homeStr.push_back('/');
}
if (auto tmp = target.string(); tmp.rfind(homeStr, 0) == 0) {
return target;
}
}
dengbo11
merged commit dff5088
into
OpenAtom-Linyaps:release/1.13
Codecov Report❌ Patch coverage is
🚀 New features to boost your workflow:
|
2 participants
