Skip to content

pmcd: restrict control metric stores to root users#2658

Open
orasagar wants to merge 1 commit into
performancecopilot:mainfrom
orasagar:pmda_control_fix
Open

pmcd: restrict control metric stores to root users#2658
orasagar wants to merge 1 commit into
performancecopilot:mainfrom
orasagar:pmda_control_fix

Conversation

@orasagar

Copy link
Copy Markdown
Contributor

Require root credentials when writing privileged pmcd control metrics to prevent unauthorized runtime configuration changes.

Pull Request Description

Related Issues

Fixes # #2652

Require root credentials when writing privileged pmcd control metrics to prevent unauthorized runtime configuration changes.
@coderabbitai

coderabbitai Bot commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Summary by CodeRabbit

  • Bug Fixes
    • Restricted updates to PMCD control settings to authorized contexts.
    • Improved context cleanup by fully resetting context ownership information.
    • Added clearer identification for debug, timeout, registration, signal, and credential-timeout controls.

Walkthrough

PMCD control metrics now use named item constants, enforce nonzero-UID authorization during stores, and reset context UIDs during teardown.

Changes

PMCD control authorization

Layer / File(s) Summary
Control metric authorization and context cleanup
src/pmdas/pmcd/src/pmcd.c
Named constants identify control items, pmcd_store rejects unauthorized contexts with PM_ERR_PERMISSION, and end_context resets the context UID to -1.

Possibly related issues

  • performancecopilot/pcp#2652 — Directly covers permission checks for pmcd.control.* writes and unauthorized UID rejection.

Suggested reviewers: natoscott

Poem

I’m a rabbit guarding controls in the night,
With named little IDs lined up just right.
No UID, no write—permission denied,
Old context tracks are cleared aside.
Hop, hop, secure code!

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: restricting pmcd control metric writes to root users.
Description check ✅ Passed The description matches the changes by explaining root-only writes for privileged pmcd control metrics.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot 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.

🧹 Nitpick comments (1)
src/pmdas/pmcd/src/pmcd.c (1)

34-39: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Consider utilizing these macros to replace existing magic numbers.

Defining these named constants is a great improvement. To fully realize their benefit and establish a single source of truth, consider using them to replace the hardcoded item numbers (e.g., item == 0, item == 4, item == 8) in the corresponding if / else if chains further down in pmcd_store and within the switch statement in pmcd_fetch.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/pmdas/pmcd/src/pmcd.c` around lines 34 - 39, Replace the hardcoded
control item numbers in the conditional chains within pmcd_store and the switch
in pmcd_fetch with the corresponding PMCD_CONTROL_DEBUG_ITEM,
PMCD_CONTROL_TIMEOUT_ITEM, PMCD_CONTROL_REGISTER_ITEM, PMCD_CONTROL_SIGHUP_ITEM,
and PMCD_CONTROL_CREDS_TIMEOUT_ITEM macros. Preserve the existing branch and
case behavior while using these definitions as the single source of truth.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@src/pmdas/pmcd/src/pmcd.c`:
- Around line 34-39: Replace the hardcoded control item numbers in the
conditional chains within pmcd_store and the switch in pmcd_fetch with the
corresponding PMCD_CONTROL_DEBUG_ITEM, PMCD_CONTROL_TIMEOUT_ITEM,
PMCD_CONTROL_REGISTER_ITEM, PMCD_CONTROL_SIGHUP_ITEM, and
PMCD_CONTROL_CREDS_TIMEOUT_ITEM macros. Preserve the existing branch and case
behavior while using these definitions as the single source of truth.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository YAML (base), Repository UI (inherited), Organization UI (inherited)

Review profile: CHILL

Plan: Pro Plus

Run ID: e3be4a26-c02e-4126-9a95-40b6f0f179f4

📥 Commits

Reviewing files that changed from the base of the PR and between 4529a26 and 59969c4.

📒 Files selected for processing (1)
  • src/pmdas/pmcd/src/pmcd.c

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.

1 participant