Skip to content

chore: update pvtr plugin to v0.24.0+2fixes#4314

Merged
joanagmaia merged 33 commits into
mainfrom
chore/update-pvtr-version
Jul 14, 2026
Merged

chore: update pvtr plugin to v0.24.0+2fixes#4314
joanagmaia merged 33 commits into
mainfrom
chore/update-pvtr-version

Conversation

@joanagmaia

@joanagmaia joanagmaia commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Summary

Bumps `pvtr-github-repo-scanner` and hardens the security-insights worker's token handling.

Plugin bump

  • `pvtr-github-repo-scanner` commit `c7bd9538` (v0.23.2) → `c1095c95` (post-v0.24.0 HEAD)
  • Two required upstream fixes:
    • `fix(OSPS-BR-07.01)` — consult Security Insights when `security_and_analysis` is null
    • `fix(OSPS-LE-02.01)` — pass deprecated but OSI/FSF-approved SPDX IDs
  • Plugin build stage `golang:1.26.3-alpine3.23` → `golang:1.26.4-alpine3.22` (required by new `go.mod`)
  • Privateer binary stays at `0.21.2` (confirmed compatible)
  • `example-config.yml`: quote `owner`/`repo`/`token` placeholders so repos with numeric names don't break sed substitution

Token error classification (`classifyTokenError`)

  • `401` (including `401 Bad credentials`) → `TokenAuthError` (globally invalid, non-retryable)
  • `403` with rate-limit body OR `429` → `Token403Error` (1h cooldown, non-retryable)
  • Permission `403` (no rate-limit markers) — logged, not thrown; child workflow retry policy exhausts naturally and the repo defers to next scheduled run
  • Failure timestamp passed via `ApplicationFailure.details[0]` so the workflow can persist an accurate `rateLimitedAt`
  • `runBinary` attaches full stdout/stderr to thrown errors (message is still truncated) so the classifier sees rate-limit markers past the 500-char cutoff

Redis-persisted token state (`ITokenInfo`)

  • Adds `rateLimitedAt` (ISO string, drives 1h cooldown auto-reset)
  • Adds `isInvalid` (401-only global exclusion; recover by rotating the PAT out of `CROWD_GITHUB_PERSONAL_ACCESS_TOKENS`)
  • `lastUsed` typed as `Date | string` (reflects JSON round-trip reality through Redis and Temporal payloads)
  • `initializeTokenInfos` reconciles env-var pool with Redis cache, resets stale `inUse` flags on load, drops legacy rate-limit entries without a timestamp
  • No schema break: additive fields; existing keys continue to load

Workflow (`triggerSecurityInsightsCheckForRepos`)

  • `unwrapApplicationFailure`: traverses `ChildWorkflowFailure → ActivityFailure → ApplicationFailure` cause chain — the original `instanceof ApplicationFailure` check was silently failing for child workflow errors
  • Wall-clock time via `getCurrentTimeMs` activity (workflow code can't call `Date.now()` — Temporal determinism rule); refreshed each outer-loop iteration so mid-run rate-limit cooldowns can expire without waiting for `continueAsNew`
  • `MAX_TOKEN_ATTEMPTS = max(5, poolSize)` — scales with pool size so no token goes untried
  • Rate-limited pool: repo requeued (not failed), deferred to next scheduled run if all tokens exhausted
  • `refreshExpiredRateLimits`: NaN-safe 1h expiry; clears malformed `rateLimitedAt` rather than wedging the token
  • `hasFreeToken` guard before starting a task — avoids oversubscribing when all tokens are held by concurrent children
  • All-tokens-invalid: remaining repos marked failed (not deferred) so ops sees the problem
  • `deferred` flag: returns instead of `continueAsNew` when all tokens rate-limited and repos still pending, preventing a hot loop; `rateLimitedAt` in Redis makes recovery resumable on the next daily run
  • Child `upsertOSPSBaselineSecurityInsights` timeout `5m` → `15m`
  • `nonRetryableErrorTypes` covers `Token403Error` and `TokenAuthError`

No breaking changes

  • Redis token cache schema is additive (new optional fields)
  • `CROWD_GITHUB_PERSONAL_ACCESS_TOKENS` env var remains authoritative for pool membership
  • Cron schedule `scheduleCheckReposWithObsoleteSecurityInsights` unchanged

Bumps pvtr-github-repo-scanner from v0.23.2 (c7bd9538) to commit
c1095c95, which is post-v0.24.0 and includes two required fixes:
- fix(OSPS-BR-07.01): consult Security Insights when security_and_analysis is null
- fix(OSPS-LE-02.01): pass deprecated but OSI/FSF-approved SPDX IDs

Also updates the plugin build stage to golang:1.26.4-alpine3.22, required
by go.mod at the new commit (go 1.26.4). Privateer binary stays at 0.21.2,
confirmed compatible by pvtr's own Dockerfile.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Signed-off-by: Joana Maia <jmaia@contractor.linuxfoundation.org>
Copilot AI review requested due to automatic review settings July 7, 2026 15:00
@github-actions

github-actions Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

⚠️ Jira Issue Key Missing

Your PR title doesn't contain a Jira issue key. Consider adding it for better traceability.

Example:

  • feat: add user authentication (CM-123)
  • feat: add user authentication (IN-123)

Projects:

  • CM: Community Data Platform
  • IN: Insights

Please add a Jira issue key to your PR title.

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 bumps the pvtr-github-repo-scanner plugin used by the security_best_practices_worker image from the v0.23.2 pin (commit c7bd9538) to a post-v0.24.0 commit (c1095c95, labeled "v0.24.0+2fixes"), which pulls in two required OSPS bug fixes. The change is isolated to a single build Dockerfile and involves no application code. I verified the change against the upstream scanner repo at the pinned commit: its go.mod declares go 1.26.4 and its own Dockerfile uses golang:1.26.4-alpine3.22 (digest-pinned), so the base-image change and version bump are correct and consistent. No other references to the old version or commit exist elsewhere in the repo.

Changes:

  • Bump PVTR_VERSION to v0.24.0 and PVTR_COMMIT to c1095c95…, updating the accompanying comment to match.
  • Update the plugin build stage base image from golang:1.26.3-alpine3.23 to golang:1.26.4-alpine3.22 (matches upstream go 1.26.4 requirement).

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

@joanagmaia joanagmaia requested review from Copilot and gaspergrom and removed request for Copilot July 8, 2026 10:50
gaspergrom
gaspergrom previously approved these changes Jul 9, 2026
Copilot AI review requested due to automatic review settings July 9, 2026 08:11

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.

Copilot AI review requested due to automatic review settings July 10, 2026 10:16

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 July 13, 2026 14:36
…sion

Signed-off-by: Joana Maia <jmaia@contractor.linuxfoundation.org>
Comment thread services/apps/security_best_practices_worker/src/activities/index.ts Outdated

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 1 comment.

Comment thread scripts/services/docker/Dockerfile.security_best_practices_worker
Signed-off-by: Joana Maia <jmaia@contractor.linuxfoundation.org>
Copilot AI review requested due to automatic review settings July 13, 2026 14:48
…check

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 7 out of 7 changed files in this pull request and generated 1 comment.

Copilot AI review requested due to automatic review settings July 14, 2026 13:22

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 7 out of 7 changed files in this pull request and generated 1 comment.

Comment thread scripts/services/docker/Dockerfile.security_best_practices_worker
Signed-off-by: Joana Maia <jmaia@contractor.linuxfoundation.org>
Copilot AI review requested due to automatic review settings July 14, 2026 13: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 7 out of 7 changed files in this pull request and generated 2 comments.

Copilot AI review requested due to automatic review settings July 14, 2026 13:37

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 7 out of 7 changed files in this pull request and generated 2 comments.

Copilot AI review requested due to automatic review settings July 14, 2026 13:44

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 7 out of 7 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

services/apps/security_best_practices_worker/src/workflows/triggerSecurityInsightsCheckForRepos.ts:213

  • This gate ignores repoAttemptedTokens. If token A already returned TokenRepoAccessError for this repo while token B is rate-limited, A keeps hasFreeToken true, but processRepo immediately excludes A, requeues the repo, and repeats. The workflow then emits getCurrentTimeMs activities in a hot loop and never reaches the defer branch. Select a queued repo only when a token is usable after that repo's exclusions.
   * Each task will acquire a token, process the repo, and then release the token.

Copilot AI review requested due to automatic review settings July 14, 2026 13:51

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 7 out of 7 changed files in this pull request and generated 1 comment.

Copilot AI review requested due to automatic review settings July 14, 2026 14:02
Signed-off-by: Joana Maia <jmaia@contractor.linuxfoundation.org>
Signed-off-by: Joana Maia <jmaia@contractor.linuxfoundation.org>
Signed-off-by: Joana Maia <jmaia@contractor.linuxfoundation.org>
@joanagmaia joanagmaia force-pushed the chore/update-pvtr-version branch from 37e0a2e to 1daefa3 Compare July 14, 2026 14:05

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 7 out of 7 changed files in this pull request and generated 3 comments.

Comment thread services/apps/security_best_practices_worker/src/activities/index.ts Outdated
Copilot AI review requested due to automatic review settings July 14, 2026 14:07

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 7 out of 7 changed files in this pull request and generated 2 comments.

…oken handling

Permission 403s don't need their own error type — the child
workflow's retry policy (5 attempts) exhausts naturally and the
repo defers to the next scheduled run. Introducing TokenRepoAccessError
required per-repo attempted-token tracking (repoAttemptedTokens map,
sawRateLimit flag, releaseCounter) that added ~60 lines of complexity
for a marginal benefit.

Core fixes from the AI-generated code are preserved: unwrapApplicationFailure
(ChildWorkflowFailure traversal was silently broken), TokenAuthError for 401s,
isInvalid token field, rateLimitedAt 1h expiry, hasFreeToken guard, and
deferred flag to prevent continueAsNew spin on full rate-limit.

Signed-off-by: Joana Maia <jmaia@contractor.linuxfoundation.org>

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 6a552da. Configure here.

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 7 out of 7 changed files in this pull request and generated 3 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.

3 participants