fix: post-audit verification findings (timestamp month, install perms, OOM leaks, errno)#388
Conversation
…, OOM leaks, errno) - log.zig/cli/dump.zig: drop the erroneous +1 on the month field; std.time.epoch.Month is 1-based, so @intFromEnum already yields the calendar month (June was printing as 07, December as 13). Add a Layer-0 test asserting a known epoch formats June as 06. - cli/install/plan.zig: copyFile via copyFileAbsolute creates the dest with open(O_CREAT) subject to umask, dropping the deterministic source permissions; re-apply src mode & 0o777 with an explicit fchmodat so a 0644 device TOML stays readable under a restrictive umask. Add a test asserting the copied mode equals the source mode under umask 0077. - supervisor.zig: handleSwitch leaked new_mapper + parsed_ptr in the stem_copy dupe and txs.append catch blocks (not yet in txs, so cleanupSwitchTxs could not reach them); free both, mirroring cleanupSwitchTxs semantics. - io/hidraw.zig: discoverAllWithRoot leaked the duped path if append OOMed before it entered paths.items; add errdefer free(owned). - event_loop.zig: decode the raw timerfd_settime return with linux.E.init instead of std.posix.errno (which reads C errno under libc) for a correct diagnostic errno string. Test plan: - ./scripts/padctl-docker test -> EXIT=0 - New month test and copyFile-perms test both proven falsifiable (reintroduce the bug -> test fails -> restore). - Supervisor/hidraw OOM leaks: no targeted test added; handleSwitch uses self.allocator (not an injectable param) and the leak paths require either a heavy non-OOM-safe Supervisor harness or a real hidraw char device, so they are not practically reachable at Layer 0. - event_loop errno is diagnostic-only (debug log); no test. refs: codebase audit (verification pass)
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
|
Warning Review limit reached
More reviews will be available in 14 minutes and 18 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
1 participant
Five findings from a post-audit verification pass, each re-confirmed against the current worktree before fixing.
Changes
src/log.zig,src/cli/dump.zig): removed the erroneous+ 1on the month field.std.time.epoch.Monthis 1-based, so@intFromEnumalready yields the calendar month; the+1made June print as07and December as13. Sibling formatters incli/install/udev.zig/cli/install/mappings.zigwere already correct (no+1).src/cli/install/plan.zig):copyFileswitched tocopyFileAbsolutefor atomicity but dropped the explicitchmod.copyFileAbsolutecreates the dest viaopen(O_CREAT)subject to umask, so under a restrictive umask a0644/usr/share/padctl/devicesTOML could become0600(unreadable by a non-root daemon). Re-applysrc mode & 0o777viafchmodatafter the copy.src/supervisor.zig): inhandleSwitch, thestem_copydupe catch and thetxs.appendcatch returned without freeingnew_mapper+parsed_ptr(not yet intxs, socleanupSwitchTxscould not reach them). Both blocks now free them, mirroringcleanupSwitchTxssemantics.src/io/hidraw.zig): indiscoverAllWithRoot, the duped path leaked ifappendOOMed before it enteredpaths.items(theerrdeferonly iteratespaths.items). Addederrdefer allocator.free(owned).src/event_loop.zig): decode the rawtimerfd_settimereturn withlinux.E.init(rc)instead ofstd.posix.errno(rc)(which reads C errno under libc). Diagnostic-only (debug log@tagName), no functional change — same class as the merged systemic fix.Test plan
./scripts/padctl-docker test-> EXIT=0.dump: formatTimestamp month is 1-based— asserts epoch1717200000formats as2024-06-01...(fails as2024-07-01with the+1).install: copyFile preserves source mode regardless of umask— asserts copied mode is0644underumask 0077(fails as0600without the explicit chmod).handleSwitchusesself.allocator(not an injectable param) and its setup harness is not OOM-safe forcheckAllAllocationFailures; the hidraw leak path requires a real hidraw char device to populatepaths. Both fixes are standard cleanup idioms verified by inspection against the existing cleanup paths.refs: codebase audit (verification pass)