Skip to content

fix(rocprofiler-sdk): detect and filter out keywords from ENV VARs before writing into DB - #10550

Merged
yhuiYH merged 5 commits into
developfrom
users/yhui/filter-out-env-vars
Sep 18, 2026
Merged

yhuiYH merged 5 commits into
developfrom
users/yhui/filter-out-env-vars

Conversation

@yhuiYH

@yhuiYH yhuiYH commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

Motivation

We don't want to save all env vars, in case there are sensitive keys set

Technical Details

Identify if env var contains a sensitive keyword. Exclude from saving it into the DB if included in the list.
This is not an exhaustive list. We can add more keywords to the list over time.

Issue Tracking

JIRA ID: AIPROFSDK-1050

Test Plan

N/A

Test Result

N/A

Submission Checklist

@yhuiYH
yhuiYH requested review from a team as code owners August 21, 2026 17:37
Copilot AI lite review requested due to automatic review settings August 21, 2026 17:37
@therock-pr-bot

Copy link
Copy Markdown

✅ All Policy Checks Passed

Check Status Details
📝 PR Description ✅ Pass —
⛔ Forbidden Files ✅ Pass —
🧪 Unit Test ⚠️ Warning Error: Source/code files changed without an accompanying unit test.
Expected: add at least one test file named like test_<name>.py / test_<name>.cpp (or <name>_test.*).
Current: code file(s) changed: projects/rocprofiler-sdk/source/lib/output/generateRocpd.cpp; no test file found
🚫 Draft PR 🔜 To Be Enabled —
🚩 Feature Flag 🔜 To Be Enabled —
📊 Code Coverage 🔜 To Be Enabled —

🎉 All policy checks passed!

📖 Need help? See the Policy FAQ for details on every check and how to fix failures.

🙋 Wish to Override Policy?

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

This PR updates rocprofiler-sdk’s rocpd output generation to avoid persisting potentially sensitive environment variables by filtering env var names against a keyword list before serializing them into the DB “environment” JSON field.

Changes:

  • Add a keyword-based filter to exclude env vars whose names appear potentially sensitive (case-insensitive substring match).
  • Log when an env var is excluded due to the sensitive-keyword filter (instead of storing it in the JSON persisted to DB).
Suppressed comments (1)

projects/rocprofiler-sdk/source/lib/output/generateRocpd.cpp:1323

  • This branch claims the env var was "sanitized" due to a semi-colon, but the value is not actually modified and the variable is not written to the JSON (and thus not stored). Update the message to reflect the actual behavior (excluded/skipped), or implement real sanitization.
                    else if(eval.find(';') != std::string::npos)
                    {
                        ROCP_INFO << fmt::format(
                            "Env variable {} was sanitized due to semi-colon in the value", evar);
                    }

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@github-actions

Copy link
Copy Markdown
Contributor

Code Coverage Report

Code Coverage Report

Tests Only

code coverage tests.png

Samples Only

code coverage samples.png

Tests + Samples

code coverage all.png

@yhuiYH
yhuiYH force-pushed the users/yhui/filter-out-env-vars branch 2 times, most recently from a22ea58 to a821399 Compare August 25, 2026 18:55
@yhuiYH
yhuiYH force-pushed the users/yhui/filter-out-env-vars branch from a821399 to 7d32d8a Compare September 1, 2026 14:45

@bgopesh bgopesh 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.

we can add few more maybe:

access_key
access_token
authorization
bearer
client_secret
encryption_key
session_key
signing_key
ssh_key

@yhuiYH

yhuiYH commented Sep 16, 2026

Copy link
Copy Markdown
Contributor Author

we can add few more maybe:

access_key access_token authorization bearer client_secret encryption_key session_key signing_key ssh_key

Since we already included key, token, auth and secret, all those should be flagged as sensitive already and excluded. The only new one is bearer and I can include that.

@MythreyaK

Copy link
Copy Markdown
Member

maybe HEADER(s) too? e.g., ANTHROPIC_CUSTOM_HEADERS

@yhuiYH
yhuiYH force-pushed the users/yhui/filter-out-env-vars branch 2 times, most recently from fc14453 to ae03006 Compare September 17, 2026 01:27
@yhuiYH
yhuiYH force-pushed the users/yhui/filter-out-env-vars branch from 3e1d87c to ce53f78 Compare September 18, 2026 16:15
@yhuiYH
yhuiYH merged commit eb0197b into develop Sep 18, 2026
54 of 59 checks passed
@yhuiYH
yhuiYH deleted the users/yhui/filter-out-env-vars branch September 18, 2026 18:30
@systems-assistant

Copy link
Copy Markdown
Contributor

TheRock Submodule Bump Activity

Newest first

cursor Bot pushed a commit that referenced this pull request Oct 6, 2026
…th-cursor-fixes-help

Brings the branch up to date with develop (1,887 commits). There were no
conflicts. The only rocpd change on develop since the branch point is
#10550's environment-variable filter in generateRocpd.cpp, which merges
alongside the replay-pass changes. The rocpd schema SQL is unchanged on
develop, so the frozen 3.0.4 copy still matches it.

Co-authored-by: Vivek Kale <vivek.kale@amd.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants