Protect run config uploads with typed summaries - #916
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 60c56be884
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 99a712fb43
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| projected = _numbers(sampling, SAMPLING_FIELDS) | ||
| effort = sampling.get("reasoning_effort") |
There was a problem hiding this comment.
Preserve the boolean thinking sampling control
When an eval or training config uses sampling.enable_thinking (or sampling_args.enable_thinking), this projection drops it because _numbers retains only numeric values and the subsequent logic copies only reasoning_effort. This is a supported control and the mutually exclusive peer of reasoning_effort in packages/prime/src/prime_cli/commands/rl.py:494-507; the dashboard also preserves the full sampling mapping in packages/prime/src/prime_lab_app/training_config.py:34-40. Consequently, runs using enable_thinking=True or False upload an incomplete experiment summary, so include this field only when its type is exactly bool.
Useful? React with 👍 / 👎.
Config paths and mappings currently reach the platform verbatim, so a launch file's comments, credentials, internal URLs, harness environment values, or private task data can be stored as run metadata. Send a bounded summary before the HTTP request instead, while retaining the original in
run.config/run.config_sourcelocally.Evaluation create, update, and failure fallback keep only supported typed controls, sampling settings, client type, serving controls, and bounded terminal metadata. External training registration applies the same source-file protection with additional numeric training controls. Unknown fields and producer error text are omitted; this deliberately changes the previous arbitrary-config upload contract. Files are not parsed or uploaded, so a file-only config supplies no remote controls; callers should pass a mapping for the summary.
Companion platform protection and display: https://github.com/PrimeIntellect-ai/platform/pull/5238 (ENG-6004). Matching synthetic fixtures cover the SDK, API, and UI. This does not sanitize run identity, metrics, sample/trace payloads, or historical records; prime-evals #882 is separate and does not wire these streaming paths.
Validation: all 283 prime-runs tests pass, including request-body assertions for create/update/failure and external training. Ruff and repository pre-commit checks pass.
Rollout: deploy platform #5238 for legacy-client and historical-response protection; merge this PR and publish the next prime-runs release through a separate release PR. No version bump or package publication is included here. The platform PR supplies a dry-run audit and bounded cleanup tool for existing evaluation metadata.
State-column names retain exact non-empty strings, including spaces, punctuation and Unicode, within the shared 128-entry / 128-code-point bounds. Invalid entries are omitted independently; shared SDK/API/frontend fixtures and boundary tests cover this behavior.
Terminal timestamps require an explicit timezone and normalize to UTC before storage/upload; regression coverage includes missing offsets, non-UTC offsets and out-of-range dates.
Note
High Risk
Changes what sensitive run metadata is sent to the platform and alters the public upload contract; misconfiguration could hide dashboard controls or leak data if summary allowlists are wrong.
Overview
Run config is no longer uploaded verbatim to the platform. Eval create/update/failure and external training registration now send typed summaries via new
config_privacyhelpers (metadata_summary,training_config_summary) instead of rawspec.config.Only whitelisted numeric/boolean controls, nested sampling/client/serve fields, bounded
state_columnsnames (not values), and terminalprime_runsstatus/timestamps are included. Launch file text, paths, URLs, headers, harness env, arbitrary strings,config_source, and failure error text are omitted; full config stays local onrun.config/run.config_source. File-onlyconfig=yields empty remote metadata unless callers pass a mapping with publishable controls.Docs and tests are updated to match this breaking wire contract, including fixtures that assert request bodies never contain
SECRET/PRIVATEmarkers.Reviewed by Cursor Bugbot for commit 99a712f. Bugbot is set up for automated code reviews on this repo. Configure here.