build(deps): bump js-yaml to 4.3.0 in frontend (GHSA-52cp-r559-cp3m)#102
Open
herikwebb wants to merge 15 commits into
Open
build(deps): bump js-yaml to 4.3.0 in frontend (GHSA-52cp-r559-cp3m)#102herikwebb wants to merge 15 commits into
herikwebb wants to merge 15 commits into
Conversation
Signed-off-by: Herik Webb <herik@shardplate.local>
Signed-off-by: Herik Webb <herik@shardplate.local>
Add a pull request watch policy so that any agent (including scheduled routines) subscribes to a PR immediately after opening it, handing it to the persistent watcher for review-feedback and CI follow-up until the PR is merged or closed. Signed-off-by: Herik Webb <herikwebb@users.noreply.github.com>
…tter Codify that every commit must be authored, committed, and signed off by the human submitter rather than an agent identity. Automated environments default the git identity to the agent (e.g. Claude <noreply@anthropic.com>), so a bare 'git commit -s' signs off as the agent and breaks the DCO requirement. Document setting the repo-local identity to the submitter, verifying author and sign-off before pushing, and reusing the submitter identity for follow-up commits on a PR they already authored. Signed-off-by: Herik Webb <herikwebb@users.noreply.github.com>
gpt-5.5 is a reasoning model whose reasoning tokens count against max_output_tokens, so the previous 3000 budget was intermittently exhausted before any review text was produced, failing the review job with 'OpenAI API response did not include review text'. Raise the default to 16000 and make it overridable via the OPENAI_MAX_OUTPUT_TOKENS repository variable. Also export the variable in review-pr.sh so the python child process actually inherits it — previously it was set without export, so the child always used its own hardcoded default regardless of the shell value. Signed-off-by: Herik Webb <herikwebb@users.noreply.github.com>
Raising the token budget means a review can exceed GitHub's 65536-character comment body limit, which would make 'gh pr comment' fail after a successful model call. Truncate the body (with margin) and append a notice when it is too large instead of failing to post the review. Signed-off-by: Herik Webb <herikwebb@users.noreply.github.com>
`resolveFilePathOnVolume` in `frontend/server/utils.ts` composed the final filesystem path with `path.join(volumeMountPath, filePathInVolume)`, which collapses `..` segments. `getArtifactsHandler` in `frontend/server/handlers/artifacts.ts` validates the `bucket` query param with `isAllowedResourceName` but only length-caps the `key` param and hands it to `getVolumeArtifactsHandler` → `findFileOnPodVolume` → `resolveFilePathOnVolume` unchanged. Combined with a well-known volume name mounted on the ml-pipeline-ui pod (for example `config-volume` from `manifests/kustomize/base/pipeline/ml-pipeline-ui-deployment.yaml`), an authenticated Kubeflow user could pass `?source=volume&bucket=config-volume&key=../../var/run/secrets/kubernetes.io/serviceaccount/token` through the artifacts auth middleware — which only checks that they can access their own namespace — and stream the ml-pipeline-ui pod's service account token (or any other file the pod's filesystem contains). Normalize the joined path with `path.resolve` and require that it stays under the resolved `volumeMountPath` (equal to it or under `volumeMountPath + '/'`). This blocks both `../` escapes when no `subPath` is set and `subPath + '/../../'` escapes when `subPath` is configured. Add two `resolveFilePathOnVolume` regression tests covering both variants. Signed-off-by: Herik Webb <herikwebb@users.noreply.github.com>
…eads The lexical path.resolve containment check in resolveFilePathOnVolume stops '..' traversal but not a symlink inside the mounted volume pointing outside it (e.g. 'link -> /etc/passwd'), which fs.createReadStream would follow at read time. Add resolveRealPathWithinMount, which resolves the real (symlink-free) path immediately before the read and re-verifies it stays within the volume mount, minimizing the TOCTOU window by returning the fully resolved path to open. findFileOnPodVolume now also returns the resolved volume mount path so the read site can perform this check; the extra tuple element is backward compatible with existing two-element destructures. getVolumeArtifactsHandler calls the resolver before stat/createReadStream and reads the returned safe path. When the path cannot be resolved (missing file) the original path is returned so existing not-found behavior is preserved. Extract a shared isPathInsideMount helper used by both the lexical and the realpath containment checks. Add resolveRealPathWithinMount tests covering an in-mount file, a symlink escaping the mount, and an unresolvable path. Signed-off-by: Herik Webb <herikwebb@users.noreply.github.com>
The previous fix resolved the real path and re-checked containment, but the handler then reopened that path with stat()/createReadStream(). An attacker able to write the volume could swap the file for a symlink between the check and the open, and both calls follow symlinks, so the read could still escape. Replace resolveRealPathWithinMount with openRealFileWithinMount, which opens the file once, verifies the opened descriptor's real target (via /proc/self/fd on Linux, falling back to path resolution elsewhere) is inside the mount, and returns the handle. getVolumeArtifactsHandler streams from that same handle instead of reopening by name, so validation and read act on one fd and cannot be raced. Open errors (missing file) still propagate to preserve the existing not-found behavior. Update tests to cover the open-and-validate handle, symlink rejection, and missing-file propagation. Signed-off-by: Herik Webb <herikwebb@users.noreply.github.com>
Match the notebook-intelligence PR reviewer: - Trigger on pull_request instead of pull_request_target and check out the PR head directly (fork PRs get a read-only token and no secrets, so running the diff-only reviewer on the checked-out head is safe and simpler). - Emit a machine-readable VERDICT (APPROVE / CHANGES_REQUESTED) and fail the check on a non-APPROVE verdict, so a green 'review' check means the reviewer signed off. This is what an automated promotion can key off. - Add an 'override-review-gate' label escape hatch that relaxes only the exit status; the review still runs and comments. - Use a three-dot (merge-base) diff range with a two-dot fallback on shallow clones, so the reviewer sees what the PR introduces on merge. Keeps the fork's OPENAI_MAX_OUTPUT_TOKENS=16000 override: gpt-5.5 is a reasoning model and 3000 was easily exhausted, producing empty responses that would now fail the review gate. Signed-off-by: Herik Webb <herikwebb@users.noreply.github.com>
Add a workflow_dispatch action + scripts/promote-upstream.sh that opens a cross-fork PR from a chosen fork branch into kubeflow/pipelines, using a human PAT (secrets.UPSTREAM_PAT) since the default GITHUB_TOKEN cannot write upstream. It resolves the upstream default branch, refuses empty promotions, guards against leaking fork-only automation files into the upstream diff, and by default requires the branch's fork PR review gate to be green before promoting (override with require_review_pass=false). Change detection uses GitHub's server-side compare API so it is independent of local clone depth. Signed-off-by: Herik Webb <herikwebb@users.noreply.github.com>
promote-upstream.sh used GitHub's compare API only and never rebased, so a fix branch cut from the fork's default branch dragged the fork-only automation (pr-review.yml, this script, AGENTS.md, ...) into the upstream compare and tripped the fork-only guard — the same branch could not serve both the fork review PR and the upstream promotion. Add an optional FORK_BASE input: when set (e.g. master), replay only the commits the branch adds over that base onto the current upstream tip, push the result as a script-managed derived branch, and promote that. The review gate still keys off the original fork PR branch. Blank FORK_BASE preserves the previous promote-as-is behavior.
The derive step now cherry-picks with --signoff so the promoted commits carry a Signed-off-by trailer for the submitter identity, satisfying kubeflow/pipelines' DCO requirement without a manual re-sign. Signed-off-by: Herik Webb <herikwebb@users.noreply.github.com>
With more than one fix commit GitHub falls back to the branch name for the PR title and an empty description. Default the promotion PR title/body from the primary (first) fix commit instead of the branch tip, list the included commits, and append the sign-off / title-convention checklist. Add a body input to the workflow so it can be overridden. No squashing: upstream squash-merges anyway, so the PR title/body is what becomes the merge commit. Signed-off-by: Herik Webb <herikwebb@users.noreply.github.com>
js-yaml 4.2.0 in frontend/ is vulnerable to GHSA-52cp-r559-cp3m (high), a quadratic-CPU DoS: YAML merge-key (<<) chains force O(n^2) work during parsing. The advisory affects >=4.0.0 <4.3.0, so the prior bump to 4.2.0 does not cover it; the fix is 4.3.0. Reachability: js-yaml is a direct production dependency and parses stored pipeline/workflow specs client-side (WorkflowUtils.ts, usePipelineVersionTemplate.ts, PipelineSpecTabContent.tsx, PodYaml.tsx). A pipeline author can store a crafted YAML whose merge-key chains hang the browser tab of any user who later views that pipeline/run, a cross-tenant client-side DoS in multi-user mode. Dependabot already applied the same 4.3.0 bump to frontend/server; this brings frontend/ in line. Signed-off-by: Herik Webb <herikwebb@users.noreply.github.com>
Automated PR ReviewModel: Changed files: Review result: No automated findings were found for this dependency bump. The lockfile and Residual risk: as with any dependency update, there may be runtime behavior changes in YAML parsing/emitting that are only caught by the project’s CI/test suite or manual validation of YAML-related workflows. |
herikwebb
added a commit
that referenced
this pull request
Jul 23, 2026
… fork PR #102) Signed-off-by: Herik Webb <herikwebb@users.noreply.github.com>
herikwebb
force-pushed
the
master
branch
2 times, most recently
from
July 25, 2026 13:06
675041e to
8e96fb2
Compare
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
1 participant
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.
What
frontend/pinsjs-yaml@4.2.0, vulnerable to GHSA-52cp-r559-cp3m (high) — a quadratic-CPU DoS where YAML merge-key (<<) chains force O(n²) parsing work. The advisory affects>=4.0.0 <4.3.0, so the earlier bump to4.2.0does not cover it. Fixed in 4.3.0.Why it matters
js-yamlis a direct production dependency that parses stored pipeline/workflow specs client-side (WorkflowUtils.ts,usePipelineVersionTemplate.ts,PipelineSpecTabContent.tsx,PodYaml.tsx). Any user who can upload a pipeline can store a crafted YAML whose merge-key chains hang the browser tab of any other user who later views that pipeline/run — a cross-tenant client-side DoS in multi-user mode.Change
frontend/:js-yaml^4.2.0→^4.3.0(lockfile →4.3.0).frontend/package.jsonandfrontend/package-lock.jsonchange; no other lockfile churn.4.3.0bump tofrontend/server(chore(deps): bump js-yaml from 4.2.0 to 4.3.0 in /frontend/server kubeflow/pipelines#13768); this bringsfrontend/in line.Testing
Version-only dependency bump; no source changes. Frontend build/tests were not run in this environment (frontend requires Node 24; no
node_modules). The lockfile diff is confined to thejs-yamlentry and remains valid JSON.Tracking issue: #101
Generated by Claude Code