Release 0.77.3 - #3748
Merged
Merged
Conversation
…pendency cool-down (#3712) * Harden GitHub Actions supply-chain security with zizmor and delayed dependency updates Recent supply-chain attacks have exploited both malicious/misconfigured GitHub Actions workflows and newly-published malicious package versions slipping into CI before the ecosystem catches them. This adds two independent mitigations: - Run zizmor (static analysis for GitHub Actions workflows) both in CI, on any push/PR that touches .github/workflows/**, and as a local pre-commit hook, so workflow misconfigurations (e.g. injectable expressions, overly broad permissions, unpinned actions) are caught before merge. - Set uv's exclude-newer to a rolling "7d" window in pyproject.toml, so `uv lock` won't resolve a dependency version until it has been published for at least 7 days. This gives the community time to flag newly-introduced malicious or broken releases before this repo picks them up. Confirmed via `uv lock` that this is stored as a self-updating exclude-newer-span (P7D) in uv.lock rather than a fixed cutoff date. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KDdYm3kAV35FottKdRFXo2 * Exempt in-house packages from exclude-newer cool-down The exclude-newer = "7d" setting added in a prior commit guards against newly-published third-party malicious/vulnerable packages by delaying resolution to versions at least 7 days old. That protection isn't needed for packages MIT ODL owns and iterates on rapidly (ol-concourse, django-aqueduct, open-edx-plugins, ol-django) — the 7-day delay only slows down consuming our own releases without adding any security benefit. Add [tool.uv.exclude-newer-package] with "0d" overrides for each in-house package so uv always resolves them to the latest available version while the global 7-day cool-down still applies to everything else. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KDdYm3kAV35FottKdRFXo2 * Fix zizmor high-severity findings and align pre-commit gate with CI The zizmor CI check was failing on 3 high-severity unpinned-uses findings (astral-sh/setup-uv@v7, openapi-generators/openapitools-generator-action@v1 x2). Ran `zizmor --fix=all` to pin those refs to SHAs with version comments, plus incidental artipacked fixes (persist-credentials: false) and a ref-version-mismatch correction the same pass caught. Also: - Add required-version = ">=0.9.17" to [tool.uv] in pyproject.toml. That's the uv release that introduced relative-duration exclude-newer support (used by this table's exclude-newer = "7d"); older uv now fails loudly with a version-mismatch error instead of silently mishandling the setting. `uv lock` re-run confirms this is a no-op for the lockfile. - Add args to the pre-commit zizmor hook to match the CI workflow's own --min-severity=high --min-confidence=medium gate, so pre-commit.ci and CI agree on what blocks a PR instead of pre-commit.ci enforcing a stricter, unfiltered threshold. --no-progress is kept since args: overrides all default args. Remaining lower-severity zizmor findings (10 medium excessive-permissions, below the CI gate) and an unrelated uv.lock jinxed/ansicon marker regression are tracked in #3715 rather than bundled here. * fix(ci): drop redundant pull_request trigger from zizmor workflow Both push and pull_request fired on every PR commit for the same path-scoped check, running zizmor twice per push. push alone still covers PR branch commits. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(ci): add back pull_request trigger for zizmor workflow, scope push to main Bot reviewers (Copilot, Sentry) correctly flagged that a push-only trigger misses fork-based PRs and can't act as a required merge-gate status check. Scoping push to the default branch avoids the original double-run problem (push firing on every commit to a same-repo PR branch, redundant with pull_request) while restoring PR-gate coverage. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * chore: trim exclude-newer-package allowlist to this repo's actual dependencies The allowlist exempting in-house MIT ODL packages from the 7-day uv dependency cool-down was copy-pasted org-wide, unpruned. Reviewers on two separate PRs independently flagged the same thing: most of the ~44 entries (mostly Open edX plugins) aren't dependencies of this repo at all. Trimmed to the intersection with this repo's own dependency closure (uv.lock's locked package set, or pyproject.toml's declared deps where no lockfile exists). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(deps): regenerate uv.lock to match the trimmed exclude-newer-package allowlist The previous commit edited pyproject.toml's allowlist without regenerating uv.lock, so 'uv sync --locked' correctly rejected the mismatch in CI. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(ci): lower zizmor gate to min-severity=medium, fix resulting findings - ci.yml: add workflow-level `permissions: contents: read`. All six jobs (python-tests, javascript-tests, build-nextjs-container, build-storybook, openapi-generated-client-check-v0/v1) only checkout, build, or test — none push, publish, or comment — so a single read-only default resolves both the workflow-level and per-job excessive-permissions findings. - openapi-diff.yml: add job-level `permissions: contents: read, pull-requests: write` to the openapi-diff job. It posts/updates a PR comment via peter-evans/find-comment and peter-evans/create-or-update-comment using secrets.GITHUB_TOKEN, which requires pull-requests: write. - publish-pages.yml: add workflow-level `permissions: contents: read`. The build job only checks out and builds Storybook, so it picks up the read-only default; the deploy job already declares its own narrower permissions (pages: write, id-token: write) which continue to override. - actions-static-analysis.yml: lower zizmor gate from min-severity=high to min-severity=medium (min-confidence unchanged at medium). - .pre-commit-config.yaml: lower the local zizmor hook's --min-severity from high to medium to match CI. Verified with `zizmor --min-severity=medium --min-confidence=medium .github/workflows/`: no findings to report. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
* feat: use CMS Certificate Title for program LinkedIn "Add to Profile" The LinkedIn "Add to Profile" link for program certificates now uses the CMS "Certificate Title" (product_name) as the credential name, matching the title shown on the certificate, and falls back to the program title when product_name is unset. Course certificates are unchanged.
* feat(content_feedback): allow anonymous submissions
* adding check for image reachability * use fallback image url * adding tests and celery task for cleaning resources with bad images * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Potential fix for pull request finding 'CodeQL / Full server-side request forgery' Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com> * Revert "Potential fix for pull request finding 'CodeQL / Full server-side request forgery'" This reverts commit ac761b9. * more efficient processing for image pruning task * follow through for 301 * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * removing all the overkill (pruning tasks etc) features. keeping simple check during percolate/email send process * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
OpenAPI ChangesNo changes to report, but the specs are different. Unexpected changes? Ensure your branch is up-to-date with |
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
Comment on lines
1434
to
+1436
| raise RetryError(str(ex)) from ex | ||
| msg = f"Errors occurred during recreate_index: {errors}" | ||
| TaskJob.objects.filter(id=job_id, status=TaskJob.Status.FINISHING).update( |
There was a problem hiding this comment.
Bug: The finish_reindex_job task only retries on RequestError, causing it to fail permanently on other transient OpenSearch exceptions instead of retrying.
Severity: MEDIUM
Suggested Fix
Broaden the exception handling in finish_reindex_job to catch other relevant opensearch-py exceptions, such as TransportError (the base class for RequestError, NotFoundError, etc.), and raise a RetryError for them. This will make the task more resilient to transient issues and align its behavior with other tasks.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: learning_resources_search/tasks.py#L1434-L1436
Potential issue: In the `finish_reindex_job` Celery task, the error handling for
`api.delete_orphaned_indexes` only catches and retries `RequestError`. Other common and
potentially transient exceptions from the OpenSearch client, such as `NotFoundError` or
`ConnectionError`, are not caught. When these unhandled exceptions occur, the task will
not be retried as intended. Instead, it will fail immediately, and the `on_failure`
handler will mark the entire reindexing job as `FAILED`. This is inconsistent with other
parts of the code, like `start_recreate_index`, which use a broader exception handling
strategy to prevent premature job failure.
Did we get this right? 👍 / 👎 to inform future reviews.
odlbot
added
waiting for checkboxes
all checkboxes checked
and removed
deploying to rc
waiting for checkboxes
all checkboxes checked
labels
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
7 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Shankar Ambady
Danielle Frappier
Zaman Afzal
Matt Bertrand
Anastasia Beglova
Tobias Macey