Repository navigation
Conversation
|
Warning Review the following alerts detected in dependencies. According to your organization's Security Policy, it is recommended to resolve "Warn" alerts. Learn more about Socket for GitHub.
|
[Medium impact] The PR changes how two spec-manifest workflows reach the existing SDK bot. The PR appears safe to merge, with a non-blocking fix recommended for an empty Findings
Fix with agent prompt### Issue 1
scripts/replay-spec-changes.mjs:111
`--prepare-delivery` accepts a missing or empty directory as `""`. With otherwise valid inputs, the script then treats the command as a dry run and exits successfully without writing delivery files. Reject an empty directory when this option is supplied, and add a test for that case.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.SummaryThe PR moves normal and historical spec-change delivery into separate Tailscale jobs.
|
| else if (arg === "--repository") args.repository = argv[++i] ?? ""; | ||
| else if (arg === "--bot-url") args.botUrl = argv[++i] ?? ""; | ||
| else if (arg === "--push") args.push = true; | ||
| else if (arg === "--prepare-delivery") args.prepareDelivery = argv[++i] ?? ""; |
There was a problem hiding this comment.
Missing directory silently skips preparation
--prepare-delivery accepts a missing or empty directory as "". With otherwise valid inputs, the script then treats the command as a dry run and exits successfully without writing delivery files. Reject an empty directory when this option is supplied, and add a test for that case.
Prompt To Fix With AI
This is a comment left during a code review.
Path: scripts/replay-spec-changes.mjs
Line: 111
Comment:
**Missing directory silently skips preparation**
`--prepare-delivery` accepts a missing or empty directory as `""`. With otherwise valid inputs, the script then treats the command as a dry run and exits successfully without writing delivery files. Reject an empty directory when this option is supplied, and add a test for that case.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.|
going to pull for this info rather than have GHA POST to the bot |
Both normal spec-change delivery and confirmed historical replay currently reach the SDK bot over the public runner network. This moves their signed POSTs onto ephemeral Tailscale runners through an explicitly selected internal exit, using the existing NAT WAF allowance.
Both data-building jobs have no OIDC or signing secret. Separate dependency-free delivery jobs use distinct exact workflow/event identities, validate their workflow/ref/event and selected exit, verify the same-host NAT source, and sign the exact artifact bytes. Replay keeps dry-run, default-branch checking and explicit push confirmation, preflights all targets, and sends changed manifests sequentially in input order. Its script has no remaining direct fetch implementation; legacy --push delegates to the same guarded transport.
All requests and retries use explicit IPv4 SOCKS5 with curlrc, NO_PROXY bypass and redirects disabled. Before HTTP, delivery pins the destination IPv4 and installs a disposable-runner OUTPUT reject for direct TCP/443 to that IP, including tailscaled's host-network fallback if its route disappears. Failure to install the guard stops delivery.
This is controlled exit egress to the existing public Cloudflare Worker, not a tailnet-only Worker endpoint. The dedicated policy grants general IPv4 Internet HTTPS via internal exits; the script fixes the target host/path. No bot deployment is needed.
Depends on infra #2157. Complete administrator bootstrap, the reviewed human apply, and seven public Actions variables before merging this one. After bootstrap the infra plan must not mutate an existing identity. Keep SPEC_CHANGES_SECRET unchanged. Verify both producer events and a reviewed, explicitly requested replay push, bot manifest rows and ephemeral cleanup. Rollout details.
The Socket warning review traces version to the vendor download and statedir to argument-array spawn. Our inputs are a fixed version and omitted memory-only state; neither warning traces an OIDC secret. The executed bundle masks the JWT before returning it, although it is briefly present in CLI argv on the isolated runner. The signing secret is scoped only to the later delivery step. No warnings were suppressed.
Validation: all 18 caller/replay tests passed under Node 24.22.1, including exact-byte signing, ordered replay/preflight/confirmation checks and real curl failure with a closed loopback proxy. Actionlint passed for all three changed workflows; diff whitespace checks passed. These are local/static checks. Live OIDC, policy enforcement, NAT/Worker access, Linux firewall enforcement and cleanup remain rollout checks. No production replay was run.