Skip to content

fix(ci): compare pull request DOCX output securely - #233

Open
JasonColapietro wants to merge 3 commits into
TurboDocx:developfrom
JasonColapietro:fix/docx-diff-pr-checkout
Open

fix(ci): compare pull request DOCX output securely#233
JasonColapietro wants to merge 3 commits into
TurboDocx:developfrom
JasonColapietro:fix/docx-diff-pr-checkout

Conversation

@JasonColapietro

@JasonColapietro JasonColapietro commented Aug 29, 2026 •
edited
Loading

Copy link
Copy Markdown

Summary

  • run the DOCX regression workflow on pull_request, so GitHub executes the pull request merge commit with the restricted fork token
  • keep checkout credentials out of the pull request worktree and only attempt authenticated comments for same-repository branches
  • keep the diff report inside the current worktree and align the reader, comment, and artifact paths on current/diff-report.md
  • add a regression test for the event, credential, containment, consumer-path, and comment-write invariants

Problem

pull_request_target runs in the base branch context. The workflow therefore loaded the trusted develop definition and its unqualified actions/checkout also populated current/ from the base repository rather than the pull request merge commit. It could not exercise the proposed DOCX changes.

The diff step then ran from current/ while requesting --output ../diff-report.md. The script correctly rejected that destination as outside its current working directory, so the workflow reported a DOCX regression without comparing the documents.

Using an explicit pull request head checkout under pull_request_target is not a safe alternative because this workflow executes npm ci, build scripts, and tests. GitHub recommends pull_request for building untrusted pull request code.

Verification

  • npm run test:all: 22/22 suites, 639/639 tests, plus all build, example, heading, TypeScript, and RTL generation stages
  • focused workflow regression test: red before the event/credential/comment fix, 4/4 green afterward
  • reproduced the original ../diff-report.md containment rejection before comparison
  • identical generated DOCX files: report written successfully, 71 identical / 0 changed / 0 deleted, exit 0
  • DOCX with word/styles.xml removed: report written successfully with one deletion, exit 1
  • workflow YAML parse, Prettier, and git diff --check

This keeps the prerequisite workflow repair separate from #229.

CI note

Until this workflow change lands on develop, a pull_request_target run triggered by updates to this PR can still execute the old base-branch definition and reproduce the old guard failure. Forked pull_request workflows may also require maintainer approval before running.

This was referenced Aug 29, 2026
Open
Closed
@JasonColapietro
JasonColapietro marked this pull request as ready for review August 31, 2026 02:27
@JasonColapietro JasonColapietro mentioned this pull request Aug 31, 2026
Open
@JasonColapietro

JasonColapietro commented Aug 31, 2026

Copy link
Copy Markdown
Author

@nicolasiscoding This prerequisite repair is now ready for review, as requested on #229. Fresh validation is green: 22/22 suites, 639/639 tests, the complete build/example/TypeScript/RTL generation matrix, formatting, and git diff --check. Its existing red docx-diff run is the old pull_request_target definition from develop and fails on the same rejected ../diff-report.md path this PR fixes. I have rebased #229 onto this commit, so once #233 lands, synchronizing #229 will exercise the corrected workflow.

@JasonColapietro JasonColapietro changed the title fix(ci): compare pull request DOCX output securely fix(ci): keep DOCX diff report within workspace Sep 7, 2026
@JasonColapietro JasonColapietro changed the title fix(ci): keep DOCX diff report within workspace fix(ci): compare pull request DOCX output securely Sep 7, 2026
nicolasiscoding pushed a commit that referenced this pull request Sep 9, 2026
Nicolas Fry @claude
ci(docx-diff): actually test PR code + fix the report output path
7fc455a 
Two defects made the docx-diff check non-functional:

1. The "Checkout PR branch" step had no `ref`, so under pull_request_target it
   checked out the BASE branch — the diff compared base-vs-base and never tested
   PR changes. Now checks out `pull_request.head.sha`, symmetric with the
   baseline step's `base.sha`.

2. The diff step ran with `working-directory: current` and wrote `../diff-report.md`,
   which the output-path guard added in #233 rejects (must be within cwd) — so it
   exited 1 on every run since. Now runs from the workspace root with the report
   inside cwd; the downstream read/comment/upload steps already expect it there.

Verified locally: built the example DOCX from develop and from this branch
(DETERMINISTIC_IDS=true) and ran the diff — 71 identical files, 0 changed/new/deleted.
So this branch's dependency bump is not a DOCX regression.

Note: pull_request_target runs the BASE branch's workflow, so this fix only takes
effect for future PRs once merged to develop; #238's own docx-diff check keeps
running the old (broken) base-branch workflow and stays red until then.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016amyscuSg6tbhYVvuQJ2S3
nicolasiscoding added a commit that referenced this pull request Sep 9, 2026
@nicolasiscoding Nicolas Fry @claude
chore(deps): patch-duty security bumps — fast-uri, browserslist, base…
b010211 
…line-browser-mapping (#238)

* chore(deps): patch-duty security bumps — fast-uri, browserslist, baseline-browser-mapping

Weekly patch duty. Closes all open Dependabot alerts (all [development] scope —
this is a published library, so devDeps do not ship to consumers):
- fast-uri -> 3.1.6 (high, 3 advisory ranges)
- browserslist -> 4.28.7 (high)
- baseline-browser-mapping -> 2.11.0 (medium)

All three are transitive; pinned via overrides. Supersedes the standalone
Dependabot PR #236 (fast-uri).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016amyscuSg6tbhYVvuQJ2S3

* ci(docx-diff): actually test PR code + fix the report output path

Two defects made the docx-diff check non-functional:

1. The "Checkout PR branch" step had no `ref`, so under pull_request_target it
   checked out the BASE branch — the diff compared base-vs-base and never tested
   PR changes. Now checks out `pull_request.head.sha`, symmetric with the
   baseline step's `base.sha`.

2. The diff step ran with `working-directory: current` and wrote `../diff-report.md`,
   which the output-path guard added in #233 rejects (must be within cwd) — so it
   exited 1 on every run since. Now runs from the workspace root with the report
   inside cwd; the downstream read/comment/upload steps already expect it there.

Verified locally: built the example DOCX from develop and from this branch
(DETERMINISTIC_IDS=true) and ran the diff — 71 identical files, 0 changed/new/deleted.
So this branch's dependency bump is not a DOCX regression.

Note: pull_request_target runs the BASE branch's workflow, so this fix only takes
effect for future PRs once merged to develop; #238's own docx-diff check keeps
running the old (broken) base-branch workflow and stays red until then.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016amyscuSg6tbhYVvuQJ2S3

---------

Co-authored-by: Nicolas Fry <nicolas@turbodocx.com>
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
nicolasiscoding added a commit that referenced this pull request Sep 10, 2026
@nicolasiscoding Nicolas
@K-Kumar-01 @claude @amitsharma-turbodocx @dependabot @dt-eric-lefevreardant @anupamme
release: html-to-docx v1.23.0 (promote develop → main) (#240)
5712256 
* ci: bump JS actions to Node-24-bundled versions

* docs: refresh agent-skill callout to match TurboDocx SDK READMEs

Bring the README's quickstart-skill section in line with the
pattern now used across the TurboDocx SDK READMEs:

- Rename the 'Agent Skill' badge to 'Agent Skills' (matches the
  agentskills.io brand name) and bump its accent purple to 8A2BE2
- Rename the 'skills.sh' badge to 'Quickstart Skill' and point the
  click-through at the canonical source repo (github.com/TurboDocx/
  quickstart) instead of skills.sh's profile page
- Replace the 'Install via AI Agent Skill' heading with the
  'Skip the boilerplate' tagline used on the SDK READMEs
- Tighten the intro copy and keep the 4-bullet 'The skill will'
  explanation that's specific to this package

* feat: zero-config browser imports (browser ESM build + exports) + Next.js example (#205)

* feat(browser): add browser ESM build + exports map for zero-config bundler imports

Bundlers (Next.js/Turbopack, Vite, webpack) resolved the package browser field
to the IIFE build, which exposes no ES module default export, so
import HTMLtoDOCX from '@turbodocx/html-to-docx' failed with 'not a function'
(Next) / 'does not provide an export named default' (Vite, issues #203 #179).
Add dist/html-to-docx.browser.esm.js (node polyfills bundled, sharp stubbed,
real default export) and an exports map: browser condition points bundlers at
it, node condition pins server/edge to esm/umd, script/CDN keep the IIFE.
All 618 unit tests pass.

* docs(example): add zero-config Next.js browser example

Minimal Next.js 16 App Router (TypeScript) app that generates a .docx entirely
in the browser via a plain import of @turbodocx/html-to-docx. Browser-verified
end to end (produces a valid ~25KB .docx).

* fix(browser): exclude sharp from browser ESM build

The browser ESM build relied solely on a resolveId stub to drop the optional
native `sharp` dependency, but sharp is loaded via `require('sharp')` inside a
try/catch in an ES module. @rollup/plugin-commonjs leaves try/catch requires in
mixed ES modules untransformed, so the stub never fired and a bare
`require('sharp')` survived into dist/html-to-docx.browser.esm.js. Downstream
bundlers (Next.js/webpack) then statically resolved it and pulled in sharp's
Node-native deps (detect-libc -> child_process), breaking the browser build.

Add a transform hook that neutralizes the require in first-party source so no
sharp specifier remains for any consumer to follow.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* docs(example): add tabbed simple/complex examples

Add a "Complex" tab to the Next.js example alongside the existing simple demo.
The complex document showcases nested lists, a table, an inline base64 image,
a code block, the full inline-formatting pipeline (bold/italic/underline/
strikethrough/code/links/sup/sub), and color, font-size, and font-family
variations.

Example HTML moves into app/examples.ts to keep the component readable; the
component gains a tab bar that loads each example into the editable textarea.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

---------

Co-authored-by: Nicolas <nicolas@turbodocx.com>
Co-authored-by: Kushal <kushalkumargupta4@gmail.com>
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>

* fix(security): close runtime Dependabot advisories (form-data, postcss) (#216)

Same runtime-security set as the reverted main PR (#214), now targeting develop.

- form-data (high, GHSA-hmw2-7cc7-3qxx): 4.0.5 -> 4.0.6 via root overrides.
- postcss (medium, <8.5.10): 8.4.31 -> 8.5.19 in the nextjs-example lockfile
  via a scoped override.

Both transitive infra libs, not imported by src; semver-safe version-only bumps.
Dev-scope alerts (js-yaml x2, @babel/core) left for a follow-up tranche.

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* chore(deps): bump the npm_and_yarn group across 2 directories with 4 updates (#220)

Bumps the npm_and_yarn group with 3 updates in the / directory: [axios](https://github.com/axios/axios), [@babel/core](https://github.com/babel/babel/tree/HEAD/packages/babel-core) and [fast-uri](https://github.com/fastify/fast-uri).
Bumps the npm_and_yarn group with 1 update in the /example/nextjs-example directory: [next](https://github.com/vercel/next.js).


Updates `axios` from 1.16.0 to 1.18.0
- [Release notes](https://github.com/axios/axios/releases)
- [Changelog](https://github.com/axios/axios/blob/v1.x/CHANGELOG.md)
- [Commits](axios/axios@v1.16.0...v1.18.0)

Updates `@babel/core` from 7.28.4 to 7.29.6
- [Release notes](https://github.com/babel/babel/releases)
- [Changelog](https://github.com/babel/babel/blob/main/CHANGELOG.md)
- [Commits](https://github.com/babel/babel/commits/v7.29.6/packages/babel-core)

Updates `fast-uri` from 3.1.2 to 3.1.4
- [Release notes](https://github.com/fastify/fast-uri/releases)
- [Commits](fastify/fast-uri@v3.1.2...v3.1.4)

Updates `next` from 16.2.7 to 16.2.11
- [Release notes](https://github.com/vercel/next.js/releases)
- [Commits](vercel/next.js@v16.2.7...v16.2.11)

---
updated-dependencies:
- dependency-name: axios
  dependency-version: 1.18.0
  dependency-type: direct:production
  dependency-group: npm_and_yarn
- dependency-name: "@babel/core"
  dependency-version: 7.29.6
  dependency-type: direct:development
  dependency-group: npm_and_yarn
- dependency-name: fast-uri
  dependency-version: 3.1.4
  dependency-type: indirect
  dependency-group: npm_and_yarn
- dependency-name: next
  dependency-version: 16.2.11
  dependency-type: direct:production
  dependency-group: npm_and_yarn
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* chore(deps): bump brace-expansion (#222)

Bumps the npm_and_yarn group with 1 update in the / directory: [brace-expansion](https://github.com/juliangruber/brace-expansion).


Updates `brace-expansion` from 1.1.14 to 1.1.18
- [Release notes](https://github.com/juliangruber/brace-expansion/releases)
- [Commits](juliangruber/brace-expansion@v1.1.14...v1.1.18)

---
updated-dependencies:
- dependency-name: brace-expansion
  dependency-version: 1.1.18
  dependency-type: indirect
  dependency-group: npm_and_yarn
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* chore(deps): bump the npm_and_yarn group across 1 directory with 2 updates (#224)

Bumps the npm_and_yarn group with 2 updates in the / directory: [nanoid](https://github.com/ai/nanoid) and [fast-uri](https://github.com/fastify/fast-uri).


Updates `nanoid` from 3.3.11 to 3.3.17
- [Release notes](https://github.com/ai/nanoid/releases)
- [Changelog](https://github.com/ai/nanoid/blob/main/CHANGELOG.md)
- [Commits](ai/nanoid@3.3.11...3.3.17)

Updates `fast-uri` from 3.1.4 to 3.1.5
- [Release notes](https://github.com/fastify/fast-uri/releases)
- [Commits](fastify/fast-uri@v3.1.4...v3.1.5)

---
updated-dependencies:
- dependency-name: nanoid
  dependency-version: 3.3.17
  dependency-type: direct:production
  dependency-group: npm_and_yarn
- dependency-name: fast-uri
  dependency-version: 3.1.5
  dependency-type: indirect
  dependency-group: npm_and_yarn
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* ci: publish to npm with provenance attestation (#223)

Add id-token: write (plus explicit contents: read for actions/checkout) to
the publish-npm job and pass --provenance to npm publish, so releases carry
a signed SLSA build attestation linking the tarball to this workflow run.

Co-authored-by: Nicolas Fry <nicolas@turbodocx.com>

* fix(security): clear open Dependabot advisories (js-yaml, sharp, postcss, nanoid) (#226)

Root (dev-scope, via commitlint):
- js-yaml 3.14.2 -> 3.15.1, 4.1.1 -> 4.3.1

example/nextjs-example:
- sharp 0.34.5 -> 0.35.3 (GHSA-f88m-g3jw-g9cj, inherited libvips CVEs;
  pulls the @img/sharp-* platform binaries forward with it)
- postcss 8.5.19 -> 8.5.25, nanoid 3.3.12 -> 3.3.17

The example sits below the repo .npmrc, so it does not inherit the
min-release-age=7 hardening — postcss/nanoid were pinned by hand to versions
published before the cutoff rather than taking 8.5.26 / 3.3.18.

npm audit --package-lock-only: example 0 vulnerabilities; root down to
image-size only, which has no patched release (advisory covers <= 2.0.2 and
2.0.2 is latest). Build-time image probing on trusted input; tracked, not fixed.

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

* chore(deps): move from image-size to probe-image-size (#227)

* chore(deps): move from image-size to probe-image-size

`image-size` is archived and is flagged by vulnerability scanners for
infinite-loop DoS on ICNS/HEIF/JP2/JXL inputs (SNYK-JS-IMAGESIZE-17295814).
Replace it with `probe-image-size`, which covers every raster format we
actually embed in a DOCX (png, jpeg, gif, bmp, webp, tiff, psd, ico, avif,
svg) and is still maintained.

Import from `probe-image-size/sync` rather than the package root: the root
entry pulls in `needle` (and with it http/https/zlib/iconv-lite/sax), which
would land in the bundled browser builds. We only ever probe Buffers, so the
sync entry is all we need.

The call sites are unchanged — `sizeOf(buffer)` still yields `{ width,
height }` and every one of them is already wrapped in a try/catch with a
falsy-dimension guard, which also covers `probe.sync` returning `null`
instead of throwing on unrecognised input.

Co-authored-by: Eric Lefevre-Ardant <eric.lefevre-ardant@dilitrust.com>

* fix(image): normalise measured dimensions to pixels

`probe-image-size` reports the raw declared number alongside the unit it was
written in, where `image-size` converted to pixels internally. Every caller
treats the result as pixels and multiplies by 9525 to get EMUs, so an SVG
declared `width="72pt"` rendered at 72px instead of 96 — a 25% shrink.

Add `measureImage()`, which measures and converts in one place using the
existing `SVG_UNIT_TO_PIXEL_CONVERSIONS` table, and route all three call sites
through it. Raster formats always report `px`, so they pass through untouched.

The conversion table already existed and was used for reading SVG attributes
as text; it simply was not wired into the measurement path.

This also removes the last reason for `probe-image-size` to be imported
anywhere but `utils/image.js`, so there is now one place that can produce a
dimension and no way to get an unconverted one out of it.

Reachable in practice: the `isSVG` short-circuit only exists in `buildImage`,
while the two `sizeOf` call sites in `xml-builder` had no such guard, so an SVG
reached measurement whenever it stayed an SVG in the media part — `svgHandling:
"native"`, or `"convert"` falling back because sharp is unavailable.

Tests lock the conversion against measurements taken from `image-size@2.0.2`,
so this is a parity check against the previous rendering rather than a
restatement of the new implementation.

Co-authored-by: Eric Lefevre-Ardant <eric.lefevre-ardant@dilitrust.com>

---------

Co-authored-by: Eric Lefevre-Ardant <eric.lefevre-ardant@dilitrust.com>

* chore(deps): bump nanoid to 3.3.18 (GHSA high — infinite loop on size 0) (#231)

Co-authored-by: Nicolas Fry <nicolas@turbodocx.com>

* fix: V-001 security vulnerability (#225)

Automated security fix generated by OrbisAI Security

* fix(build): correct repository.url owner casing (TurboDocx) for npm provenance

npm provenance verification is case-sensitive and expects the package.json
repository.url to match the canonical GitHub slug TurboDocx/html-to-docx.
The lowercase turbodocx value caused publish to fail with E422
'Error verifying sigstore provenance bundle'.

* fix(deps): bump nanoid 3.3.17 -> 3.3.18 in nextjs example (#232)

Resolves Dependabot alert (GHSA, high) for nanoid <3.3.18 in
example/nextjs-example. postcss requests nanoid ^3.3.16, so a targeted
npm update --package-lock-only picks up the patch (no override needed).

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* chore(deps): patch-duty security bumps — fast-uri, browserslist, baseline-browser-mapping (#238)

* chore(deps): patch-duty security bumps — fast-uri, browserslist, baseline-browser-mapping

Weekly patch duty. Closes all open Dependabot alerts (all [development] scope —
this is a published library, so devDeps do not ship to consumers):
- fast-uri -> 3.1.6 (high, 3 advisory ranges)
- browserslist -> 4.28.7 (high)
- baseline-browser-mapping -> 2.11.0 (medium)

All three are transitive; pinned via overrides. Supersedes the standalone
Dependabot PR #236 (fast-uri).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016amyscuSg6tbhYVvuQJ2S3

* ci(docx-diff): actually test PR code + fix the report output path

Two defects made the docx-diff check non-functional:

1. The "Checkout PR branch" step had no `ref`, so under pull_request_target it
   checked out the BASE branch — the diff compared base-vs-base and never tested
   PR changes. Now checks out `pull_request.head.sha`, symmetric with the
   baseline step's `base.sha`.

2. The diff step ran with `working-directory: current` and wrote `../diff-report.md`,
   which the output-path guard added in #233 rejects (must be within cwd) — so it
   exited 1 on every run since. Now runs from the workspace root with the report
   inside cwd; the downstream read/comment/upload steps already expect it there.

Verified locally: built the example DOCX from develop and from this branch
(DETERMINISTIC_IDS=true) and ran the diff — 71 identical files, 0 changed/new/deleted.
So this branch's dependency bump is not a DOCX regression.

Note: pull_request_target runs the BASE branch's workflow, so this fix only takes
effect for future PRs once merged to develop; #238's own docx-diff check keeps
running the old (broken) base-branch workflow and stays red until then.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016amyscuSg6tbhYVvuQJ2S3

---------

Co-authored-by: Nicolas Fry <nicolas@turbodocx.com>
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>

* ci(docx-diff): split untrusted build from the write-token comment + harden the diff script

Addresses the code-review findings on the docx-diff harness.

Security (blocker): the previous fix ran `npm ci` + `npm test` on untrusted PR
head code under `pull_request_target` with a `pull-requests: write` token — a
token-exfiltration / RCE vector for fork PRs. Split into two workflows:
- docx-diff.yml now triggers on `pull_request` with `contents: read` only (no
  secrets, no write token), checks out the PR merge ref with
  `persist-credentials: false`, builds baseline + current DOCX, runs the diff,
  and uploads the report (+ PR number) as an artifact. It never comments.
- docx-diff-comment.yml (new) triggers on `workflow_run` in the trusted base
  context, holds `pull-requests: write`, downloads the artifact, and posts the
  comment. It never checks out or runs PR code.

diff-docx.js hardening:
- `--output` with no value now errors gracefully instead of throwing a TypeError
  on `path.resolve(undefined)`.
- the cwd-containment guard resolves symlinks on the target's parent dir
  (fs.realpathSync) so a symlinked --output can't escape the working directory.

Verified: node --check passes; standalone assertions cover the no-value,
in-cwd, `../` escape, and symlinked-parent cases; both workflow YAMLs parse.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016amyscuSg6tbhYVvuQJ2S3

* fix(image): convert SVG `ex` units to pixels instead of silently treating them as px (#241)

`measureImage` normalizes probe-image-size's declared value + unit to pixels via
SVG_UNIT_TO_PIXEL_CONVERSIONS, but the table had no `ex` entry. probe-image-size
can emit `ex` (its unit regex is `in|mm|cm|pt|pc|px|em|ex`), so an SVG sized in
`ex` fell through the `|| 1` fallback and was measured as if it were pixels —
rendering the image ~8x too small (a `width="72ex"` SVG came out 72px instead of
576px).

Add `ex: 8` (1ex = 0.5em, half the table's assumed 16px em), matching the CSS
default and the existing em/rem convention.

TDD: added a failing `ex` parity case to tests/image-measurement.test.js
(72ex -> 576px), watched it fail (received 72), then added the table entry.
Full jest suite green (636 tests).


Claude-Session: https://claude.ai/code/session_016amyscuSg6tbhYVvuQJ2S3

Co-authored-by: Nicolas Fry <nicolas@turbodocx.com>
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>

* docs(readme): add TurboQuote to the TurboDocx ecosystem table

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016amyscuSg6tbhYVvuQJ2S3

* ci(docx-diff): resolve comment target trusted-side + harden output guard

Follow-up hardening from the v1.23.0 security + code review of the docx-diff split.

- docx-diff-comment.yml: resolve the target PR from the build run's head SHA
  (listPullRequestsAssociatedWithCommit) instead of from the artifact. The build
  runs untrusted PR code and could tamper with an uploaded pr-number.txt to
  redirect the bot comment at an arbitrary PR; the head SHA comes from the trusted
  workflow_run event. Only the comment body (the PR's own diff) still comes from
  the artifact and lands solely on that PR — bounded, not a redirect. Dropped the
  now-unused pr-number.txt step/artifact from docx-diff.yml.
- diff-docx.js: the containment guard now also refuses a symlinked target file
  (lstat) in addition to resolving the parent dir, so a symlinked --output can't
  redirect the write outside cwd — matching what the comment claims.

Verified: node --check, both workflow YAMLs parse, guard assertions cover
in-cwd / ../ escape / symlinked-target / symlinked-parent.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016amyscuSg6tbhYVvuQJ2S3

---------

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: Nicolas <nicolas@turbodocx.com>
Co-authored-by: Kushal <kushalkumargupta4@gmail.com>
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Co-authored-by: Amit Sharma <amit.sharma@turbodocx.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Eric Lefevre-Ardant <eric.lefevre-ardant@dilitrust.com>
Co-authored-by: Anupam Mediratta <mediratta@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@JasonColapietro