Skip to content

fix: privateer binary version compatibility with pvtr-github-repo-scanner v0.23.2 (IN-1170)#4214

Open
joanagmaia wants to merge 12 commits into
mainfrom
fix/privateer-sdk-version
Open

fix: privateer binary version compatibility with pvtr-github-repo-scanner v0.23.2 (IN-1170)#4214
joanagmaia wants to merge 12 commits into
mainfrom
fix/privateer-sdk-version

Conversation

@joanagmaia

@joanagmaia joanagmaia commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

Summary

Restores the upsertOSPSBaselineSecurityInsights workflow after the plugin upgrade in #4126. That PR bumped pvtr-github-repo-scanner to v0.23.2 (requires privateer-sdk v1.24.0) but left the rest of the stack on the v1.4.0 contract, causing the workflow to fail at every layer.

What changed

Dockerfile (scripts/services/docker/Dockerfile.security_best_practices_worker)

  • Bumps the privateer binary release from 0.7.00.21.2 to match the plugin's SDK
  • Copies the renamed pvtr binary as privateer
  • Writes /root/.privateer/bin/plugins.json so the SDK's new manifest-based plugin discovery finds the github-repo plugin

Runtime (services/apps/security_best_practices_worker/src/activities/index.ts)

  • runBinary treats exit code 1 (failed assessments — run completed) as success, since the result file is still written. Codes ≥2 still reject and attach truncated stdout/stderr as structured fields on the error
  • saveOSPSBaselineInsightsToDB updated for the new YAML shape (see below)
  • runDuration derived from start/end timestamps (the SDK no longer emits it as a discrete field)

Types (services/apps/security_best_practices_worker/src/types.ts)

  • Mapped to the v1.24.0 YAML schema: kebab-case top-level keys (evaluation-suites, catalog-id, start-time, end-time, corrupted-state), control-evaluations is now a nested EvaluationLog object with an evaluations array, assessment-logs replaces assessments, and control/requirement IDs are now nested EntryMapping objects with reference-id/entry-id. Verified field-for-field against gemaraproj/go-gemara v0.4.0
  • Adds the new optional plan and confidence-level fields shipped by the SDK so they survive the YAML → Redis hop (DB persistence deferred — see follow-ups)

Schema changes shipped by upstream

Five fields the old SDK emitted are no longer present in the v1.24.0 output. Tracked upstream:

Old field Status in v1.24.0
corrupted-state per control Removed; only at suite level now (lost in the ossf/gemara → go-gemara type-system migration)
remediation-guide per control Migrated to assessment-logs[].recommendation (ossf/gemara#109) — already captured per-assessment
run-duration per assessment Replaced by start + end timestamps (ossf/gemara#112) — derived locally in this PR
value per assessment Removed, no replacement
changes per assessment Removed, no replacement

The DB columns for the removed-without-replacement fields (corruptedState per-control, value, changes) will be populated with their default (false / '' / null) for new rows. ON CONFLICT DO UPDATE is left as plain EXCLUDED.*, so re-evaluations overwrite normally.

Verified no display impact: the Tinybird pipe that feeds the Insights security widget (security_deduplicated_merged_copy_pipe.pipe) does not project any of the 5 fields, and the frontend SecurityData / SecurityAssessmentData types in lfx-insights only consume controlId, result, message, category, assessments[].{requirementId, description, result, message, recommendation}. All of these are still populated.

Follow-ups (not in this PR)

  • Persist confidence-level and plan to new columns on securityInsightsEvaluationAssessments — net-new SDK data we currently drop after parsing
  • Drop the now-unsourced columns (corruptedState per-control, remediationGuide, runDuration, value, changes) and clean up the Tinybird datasource schemas — they're unused downstream

Test plan

  • Build the security_best_practices_worker image with the updated Dockerfile
  • Deploy to a non-prod environment and trigger upsertOSPSBaselineSecurityInsights against a test repo
  • Verify the workflow completes without the gob/plugin-installed/Temporal-size-limit errors
  • Check the securityInsightsEvaluations and securityInsightsEvaluationAssessments rows: recommendation, start, end, runDuration populated; missing fields default as documented
  • Confirm the OSPS Baseline widget in Insights still renders for a freshly-evaluated project

Note

Medium Risk
Touches the end-to-end OSPS evaluation → Redis → Postgres path and changes how several assessment fields are persisted, though downstream Insights display is documented as unchanged.

Overview
Restores the OSPS baseline security insights Temporal workflow after the pvtr-github-repo-scanner upgrade by aligning the container and worker with the newer Privateer SDK output contract.

The security_best_practices_worker image now ships privateer 0.21.2 (renamed pvtr binary), writes plugins.json for manifest-based github-repo plugin discovery, and keeps the plugin pinned to v0.23.2.

Runtime parsing and persistence were updated for the v1.24.0 YAML shape: kebab-case keys, nested control-evaluations.evaluations, assessment-logs instead of assessments, and control/requirement IDs from nested entry-id fields. runBinary treats exit code 1 (failed assessments but completed run) as success; saveOSPSBaselineInsightsToDB adds guards for missing Redis/suite rows, derives runDuration from start/end, and stores defaults where upstream removed per-control corrupted-state, remediation-guide, value, and changes.

Reviewed by Cursor Bugbot for commit 871c16d. Bugbot is set up for automated code reviews on this repo. Configure here.

Signed-off-by: Joana Maia <jmaia@contractor.linuxfoundation.org>
Copilot AI review requested due to automatic review settings June 15, 2026 15:03

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 the security_best_practices_worker Docker build to use a newer privateer binary that is compatible with the pvtr-github-repo-scanner plugin version already pinned in the image, addressing IPC (gob) decoding failures caused by SDK version mismatch.

Changes:

  • Bump privateer binary version from 0.7.0 to 0.21.2 in Dockerfile.security_best_practices_worker.

Note: The PR title does not include a JIRA key in parentheses (expected format is type: description (CM-XXX) per .claude/rules/commit-workflow.md:52-56), so CI/policy checks may fail until that’s corrected.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@joanagmaia joanagmaia changed the title fix: privateer binary version compatibility with pvtr-github-repo-scanner v0.23.2 fix: privateer binary version compatibility with pvtr-github-repo-scanner v0.23.2 (IN-1170) Jun 15, 2026
Signed-off-by: Joana Maia <jmaia@contractor.linuxfoundation.org>
@joanagmaia joanagmaia force-pushed the fix/privateer-sdk-version branch from 32134a4 to fb2616b Compare June 15, 2026 15:52
@joanagmaia joanagmaia requested a review from gaspergrom June 15, 2026 15:54
@joanagmaia joanagmaia self-assigned this Jun 15, 2026
Signed-off-by: Joana Maia <jmaia@contractor.linuxfoundation.org>
Copilot AI review requested due to automatic review settings June 15, 2026 16:40
Signed-off-by: Joana Maia <jmaia@contractor.linuxfoundation.org>

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

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Signed-off-by: Joana Maia <jmaia@contractor.linuxfoundation.org>
Copilot AI review requested due to automatic review settings June 15, 2026 16:53

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

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

Comment thread scripts/services/docker/Dockerfile.security_best_practices_worker Outdated
Signed-off-by: Joana Maia <jmaia@contractor.linuxfoundation.org>
Signed-off-by: Joana Maia <jmaia@contractor.linuxfoundation.org>
Copilot AI review requested due to automatic review settings June 15, 2026 17:06
@joanagmaia joanagmaia removed the request for review from gaspergrom June 15, 2026 17:09

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

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

Comment thread services/apps/security_best_practices_worker/src/activities/index.ts Outdated
joanagmaia and others added 2 commits June 15, 2026 20:23
…K fields

privateer-sdk v1.24.0 removed `run-duration` in favor of `start`/`end`
timestamps; compute the duration locally so the existing column stays
populated. Adds the new optional `plan` and `confidence-level` fields
to the YAML types so they survive parsing into Redis.

Signed-off-by: Joana Maia <jmaia@contractor.linuxfoundation.org>
…ps (IN-1170)

Addresses PR review: guard against null returns from redis cache and
post-insert DB lookups so workflow failures surface with context instead
of TypeError on undefined property access.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Signed-off-by: Joana Maia <jmaia@contractor.linuxfoundation.org>
Copilot AI review requested due to automatic review settings June 15, 2026 19:33

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

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

Comment thread services/apps/security_best_practices_worker/src/activities/index.ts Outdated
…or fields (IN-1170)

Addresses PR review: guard computeRunDuration against missing or
unparseable timestamps so NaN/negative durations don't reach the DB,
and attach truncated stdout/stderr on rejected binary errors so
Temporal payloads stay within serialization limits.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Signed-off-by: Joana Maia <jmaia@contractor.linuxfoundation.org>
@joanagmaia joanagmaia force-pushed the fix/privateer-sdk-version branch from 39ed854 to f394703 Compare June 15, 2026 20:01
Signed-off-by: Joana Maia <jmaia@contractor.linuxfoundation.org>

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

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

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.

2 participants