Conversation
Bumps the go-tooling group with 1 update in the / directory: [fgrosse/go-coverage-report](https://github.com/fgrosse/go-coverage-report). Updates `fgrosse/go-coverage-report` from 1.3.0 to 1.3.1 - [Release notes](https://github.com/fgrosse/go-coverage-report/releases) - [Changelog](https://github.com/fgrosse/go-coverage-report/blob/main/CHANGELOG.md) - [Commits](fgrosse/go-coverage-report@v1.3.0...v1.3.1) --- updated-dependencies: - dependency-name: fgrosse/go-coverage-report dependency-version: 1.3.1 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: go-tooling ... Signed-off-by: dependabot[bot] <support@github.com>
Add a Socket supply-chain layer to the JS/TS PR umbrella, covering the gap npm audit, Trivy and CodeQL leave open: malicious install scripts, typosquats and hijacked patch releases. Two independent composites, toggled from the workflow: - src/security/socket-firewall — free tier, no token. Installs Socket Firewall, shims the package manager and runs the dependency install through it. Enabled by default and blocking, since a blocked package is malware, not a finding to triage. Skips with a warning when no lockfile is found in working-dir so monorepos do not go red on a configuration gap. - src/security/socket-scan — paid tier, socketcli. Posts the alert report on the PR and enforces the org policy. Disabled by default and advisory when enabled; skips with a notice when SOCKET_SECURITY_API_KEY is absent, so enabling it early breaks nothing. The socket job sits behind the existing change gate and exposes a stable Socket status check via result-gate, matching Frontend Analysis and Security. All inputs are additive with defaults and the new secret is optional — no caller migration needed.
Address the CodeRabbit review. Two of the findings were real bypasses that made the free tier a no-op reporting success: - The pinned SocketDev/action v1.3.2 exposes no `shims` input — verified against action.yml at ba6de6cc, where the input is absent and dist/main.js contains no shim logic; the README at that SHA documents `sfw npm install`. `shims: 'true'` was therefore silently ignored and bare `npm ci` installed completely uninspected. Every command is now prefixed with `sfw`. - actions/setup-node restored the package-manager cache, and Socket Firewall free can only inspect what crosses the network — per its docs, "if there are no network requests, as is the case when artifacts are cached locally, there is nothing for sfw to block". Dropped `cache:` and purge the cache before the guarded install. Dropping `cache:` also stops this job from writing a post-run cache entry other jobs would restore. Also from the review: - socketcli has no native --dry-run: any invocation authenticates, creates a real scan and comments on the PR. dry-run now skips the CLI entirely. - Stopped passing --disable-blocking in advisory mode. It forces exit 0 over everything, including API failures that surface as exit 3, so advisory mode was indistinguishable from a clean scan. The native exit code now reaches the evaluate step, which was already the only gate. - sarif-file is exported only once the report exists on disk. - Added socket_use_cache so the composite's use-cache is reachable, per the repo guideline that every optional composite feature gets a workflow input. - Documented the rationale for each third-party action in both composite READMEs, and corrected the docs wording that called a default-true run_socket "opt-in".
`pip install socketsecurity --upgrade` resolved whatever PyPI had published most recently and the next step ran it with SOCKET_SECURITY_API_KEY and a GitHub token in its environment — an unreviewed package executing with credentials, which is the exact risk class this composite exists to catch. Pin via a new cli-version input (default 2.5.8, the current release). PyPI forbids re-uploading an existing version, so an exact pin already resolves to a fixed artifact; --require-hashes is not used because it would also demand hashes for every transitive dependency, which is unmaintainable in a shared composite. `cli-version: latest` restores the old behavior and warns. Exposed as socket_cli_version on the umbrella and documented in both places. Bumping is manual, like the action SHAs — Dependabot does not scan src/**.
TEMPORARY, revert before merge. The socket-firewall and socket-scan composites do not exist at the v1 tag yet, so the socket job cannot resolve them and the end-to-end validation on a real caller is impossible without this. src/lint/pinned-actions reports internal refs outside @vN/@develop/@main as a warning, not an error, so the repo's own CI stays green.
Reshape the Socket layer around what the organization already runs. The socket-security GitHub App is installed and posts Socket Security: Project Report and Pull Request Alerts, both observed passing on product-console#682. Remove socket-scan. Running socketcli would re-scan the same dependency graph the App already analyses, post a second competing PR report and consume quota, for no added coverage. Drops the SOCKET_SECURITY_API_KEY secret and its inputs. Add src/setup/setup-node-guarded and use it for all twelve install jobs in frontend-pr-analysis.yml. This closes the real gap: the firewall shim only protects installs in its own job, so guarding one job left eleven running bare npm ci, where a malicious postinstall executes with the runner's tokens. The guarded path drops the package-manager cache by necessity — sfw only inspects what crosses the network — at a measured cost of ~20s per cold install. Add src/security/socket-app-gate. The App analyses but does not enforce: its checks land as success, neutral or skipped, and neither neutral nor skipped blocks a merge. The gate waits for those checks on the PR head SHA and converts them into a verdict we own, with no token and no duplicate scan. inconclusive (neutral/skipped/timeout) blocks by default, because the un-mergeable-PR skip means no diff was analysed at all and must not read as clean; missing (no App installed) only warns, so those repositories stay green on layer 1 alone. Add src/security/socket-reporter, posting one upserted PR comment in the same Stage/Status/Blocking layout as the security scan comment, under its own marker. A separate comment is forced by topology: pr-security-reporter runs inside the security_scan job of pr-security-scan.yml and step outputs do not cross jobs.
Without result-encoding: string, actions/github-script JSON-encodes the
returned string, so the step output arrived as "{\"has_findings\":false}" — a
quoted string — and the jq validation rejected it as missing has_findings.
pr-security-reporter already sets it; the new reporter did not.
Caught by the end-to-end run on product-console#682, where Socket Firewall and
the App gate both passed and only the report step failed.
Two defects found while auditing what tokens the Socket layer actually needs. The workflow-level permissions block omitted checks: read, which socket-app-gate needs to read the App's check runs. A reusable workflow's permissions are intersected with the caller's, never expanded, so any caller pinning its own permissions block would starve the gate. Added at workflow level and to the docs usage example, which every consuming repo copies verbatim. That failure was also silent: gh errors were folded into an empty result via `|| echo '[]'`, and no checks is classified as `missing`, which only warns. A missing scope therefore looked like "App not installed" and passed. A non-zero gh exit is now a hard error naming the likely cause. The run on product-console#682 passed only because that caller declares no permissions block and inherits the repository default.
No job consumes it: the firewall inspects traffic locally and the App gate reads the GitHub checks API, both token-free. It is declared because a reusable workflow cannot receive a secret it does not declare — not even via `secrets: inherit` — so declaring it now lets an organization secret become usable without cutting a release. Marked in-line and in the docs as intentionally unconsumed, with Socket Firewall enterprise named as the intended consumer, so it is not later removed as dead. Documented the least-privilege scope guidance alongside it.
TEMPORARY, remove before merge. Confirms an organization secret actually reaches this reusable workflow via secrets: inherit — the sole reason SOCKET_SECURITY_API_KEY is declared, since no job consumes it yet. Reports length and a 4-char prefix only, never the value.
It answered its question on product-console#682: the secret arrives (55 chars, skts prefix), so org secret -> selected repository -> secrets: inherit -> reusable workflow works end to end. Removed rather than left to ship.
Replace log grepping with the Socket Firewall JSON report, whose schema was
extracted from the v1.15.0 binary — the same version CI resolves:
{ blocked: [{name, namespace, version, purlString, registryFqdn,
alerts:[{alert, action, actionSource, inputPurl}]}] | null,
parseFail: [{registryFqdn, urlPath}] | null }
Detection was matching prose in the install output, written without ever having
seen a real block. Reading blocked[] is deterministic and removes the failure
mode where a genuine block was reported as blocked=false.
The report now renders a per-package table — package, version, registry and each
alert type — plus the policy source, and surfaces parseFail entries, which are a
coverage gap the previous version discarded silently: an unparsed request means
that package was never inspected.
socket-app-gate now captures each check's output.summary, so the comment shows
what the App actually said ("no net changes to dependencies" vs "Skipped
un-mergeable pull request") rather than a bare conclusion that hides which.
The five branches of the evaluate logic were exercised locally against synthetic
reports in the real schema — blocked/blocking, blocked/advisory, clean, failed
without a block, and report missing — since no safe canary package exists to
trigger a live block. safe-chain-test does not work: npm replaced it with a
benign security holding package and sfw installs it without complaint.
…cket API
Neither existing layer can say which package has which finding. Socket Firewall
free reports only what it refused — its JSON is {blocked, parseFail}, silent on
everything that passed. The App's checks carry a status and a link; Project
Report's output.text is null and the detail lives on the dashboard.
Add src/security/socket-api-report. It parses the org slug and full-scan id from
the dashboard URL the App publishes, then reads that scan back through
GET /v0/orgs/{org}/full-scans/{id} (NDJSON). Nothing is re-analysed and no scan
is created — one quota unit. Creating a scan with socketcli would have duplicated
work the App had already done and produced a competing report.
The comment gains scan scores, an alerts-by-type breakdown, and a per-package
table with severity, alert type, and whether the dependency is direct or
transitive, prod or dev.
Advisory by construction: every failure path exits 0, and HTTP statuses are
classified separately (401 invalid, 403 missing full-scans:list, 404 wrong scan,
429 quota) because they need different fixes. A Socket outage or an exhausted
quota must not read as a security finding — enforcement stays with the App gate
and with the firewall refusing an install.
The token goes through a curl --config file rather than argv, so it never lands
in the process table of a shared runner. Alert objects are read defensively, with
key/alert as fallbacks for type, so an upstream rename degrades to a label rather
than an empty report; debug-sample logs the real shape when adapting.
The jq extraction was validated against synthetic NDJSON in the documented shape.
The non-200 handler printed a hint derived from the status code, which is only a guess, and discarded the response body — the one place Socket names the missing permission. That turned scope debugging into trial and error: a 403 reported 'missing full-scans:list' even when the real cause was something else. The body is an error document, not scan data, so printing it leaks nothing.
… denial api.socket.dev sits behind a Cloudflare managed challenge that answers non-browser clients with 403, an HTML interstitial and cf-mitigated: challenge. By status code alone that is indistinguishable from a missing token scope, and the previous message asserted the scope reading — sending a real investigation after a token permission that was never the cause. Reproduced from a developer machine and from the Blacksmith CI runner, with Authorization: Bearer, HTTP Basic with the token as username, a custom User-Agent, and the exact header shape socket-sdk-python sends, against both /v0/quota and the full-scan endpoint. Three spaced retries, same result — not transient. Now classified on evidence: the cf-mitigated header and the interstitial are checked before the status code, and the message states that the request never reached Socket, so no scope audit is triggered. Also sends Accept and a User-Agent, which is correct practice even though neither lifts the challenge. Layer 3 stays wired: it exits 0 on every failure path, so the job stays green and the comment still renders, as observed on product-console#682.
Cloudflare fingerprints the HTTP client in front of api.socket.dev. curl is answered with a managed challenge — 403, an HTML interstitial, cf-mitigated — regardless of credentials, headers, auth form, endpoint or egress: reproduced with Bearer, with Basic, with a custom User-Agent, with the exact header shape socket-sdk-python sends, on /v0/quota and on the full-scan endpoint, from a developer machine and from a Blacksmith runner, three spaced retries each. Python's stdlib client is not challenged and returns real JSON from the API. It is the same stack the official SDK uses, and it explains why sfw reaches Socket from the very runner where curl is blocked — sfw is not curl. So this was never a Socket support matter, and never a token scope matter. The earlier diagnosis pointed at both and was wrong on both counts. The cf-mitigated check is kept as a regression guard should the fingerprint policy widen. The token is read from the environment inside the Python process rather than passed as an argument, so it stays out of the process table. Validated locally end to end: an invalid token now yields a clean HTTP 401 with Socket's own JSON error body, no challenge, step exit 0.
The first rendering was unusable: 4636 raw alerts, the same envVars repeated hundreds of times per package, ranked by raw count so the noisiest package led. The dashboard's 36 comes from filtering on alert action — a field this code has never inspected. TEMPORARY. Dumps the alert object keys and the distinct action/severity/category values so the filtering is built against the real shape rather than guessed.
Placed in the composite rather than behind the workflow-level debug input, because gh run rerun reloads composites from the branch but replays the original reusable-workflow graph — so a workflow input change needs a fresh push while a composite change does not. The action axis is what the Socket dashboard filters on, and the difference between 36 actionable alerts and 4636 raw ones.
…decision
The first real rendering was unusable: 4636 alerts, the same envVars repeated
hundreds of times per package, ranked by raw count so the noisiest package led.
The dashboard's much smaller number comes from filtering on alert action — a
field the extraction had never inspected.
Measured on a 1897-artifact scan: action is "ignore" for 4594 of 4636 alerts,
"monitor" for 24 and "warn" for 18. So 42 alerts carry a policy decision and the
rest are capability observations. Severity alone is not a usable filter — 118
alerts are "high" yet still ignored by policy.
The extraction now keeps only alerts whose action is in include-actions
(error,warn,monitor by default), collapses repeats per package+type into a count
with the first file, and ranks by action then severity rather than volume. Alert
objects also carry file, start, end and fix, so the comment can point at the
location and surface remediation.
Adds fail-on-actions, wired to the job gate: findings can now block the pull
request under a policy defined here rather than deferring to the App's verdict.
Default empty preserves report-only. The API-failure paths report zero blocking
findings by construction, so an unreachable Socket still cannot fail a PR.
Also stops the extraction from failing silently: a jq error previously produced
{} and a comment that read as a clean scan.
…ella Wires include-actions and fail-on-actions through as socket_api_include_actions and socket_api_fail_on_actions, and makes the job gate honour the blocking count. Default fail-on-actions is empty, so behaviour is report-only until a repository opts in. The API-failure paths report zero blocking findings by construction, so an unreachable Socket still cannot fail a pull request — only real findings can. Also drops the temporary debug-sample hardcode; the action distribution it was added to discover is now logged compactly by the composite on every run.
…t Object] The alert fix field is an object, not a string, so the suggested-fixes list printed [object Object] for every entry — the most actionable field Socket returns, lost in the render. Its shape is not documented, so the reporter prefers the human-readable fields and falls back to compact JSON rather than dropping the content, and the composite now logs the first non-null fix so the preference list can be narrowed against the real shape.
…g debt The findings reported so far were the whole dependency tree, not the pull request. On product-console that is 42 actioned findings on packages like react-color and brace-expansion, none of which any given PR touches — they would appear on every pull request forever. Worse, fail-on-actions applied to them: setting it to "warn" would have blocked every PR in the repository for 18 pre-existing obfuscatedFile alerts nobody introduced, which is how a gate gets switched off in its first week. socket-api-report now reads the most recent full scan of the target branch as a baseline and classifies each finding. A finding is introduced when this package@version did not already carry that alert type on the base — so a version bump counts as new, the conservative reading. The baseline is resolved through full-scans?branch=, which needs only the full-scans:list scope already granted; the diff-scans API would have required two more. Only introduced findings can block. Pre-existing ones are collapsed into a <details> and never gate. The extraction moved from jq to python: a two-scan diff in jq would have been unreadable, and python was already required for the HTTP call. The comment now carries findings only. Whether the scan ran, which App checks passed and how many alerts were filtered out are operational facts that belong in the job log and the Socket status check — not in a comment people are asked to read. With comment-when: findings (the default) nothing is posted when there is nothing to act on, and an existing comment collapses to a resolved note so a stale one never reads as current.
The split reported 38 findings as introduced by a pull request that changed only @types/lodash, which cannot be right. The baseline is whatever full scan of the target branch is most recent, and nothing so far reveals which scan that is or how complete it looks. Logs the baseline scan id, timestamp, commit, state and artifact count, and warns when the baseline is less than half the size of the head scan — an unrelated or incomplete baseline makes everything look introduced.
The first attribution was wrong: 38 findings reported as introduced by a pull
request that changed only @types/lodash. The baseline was the most recent scan of
the target branch regardless of state, and that scan was in state "resolve" —
still resolving dependencies. It carried 1556 artifacts and 1412 alert keys
against 1897 and 4636 in the head scan, so everything absent from it looked new.
Two defences now, because a wrong attribution is worse than none:
1. Candidates in pending/precrawl/resolve are skipped.
2. The candidate must be at least 80% the size of the head scan, which tests
what actually matters instead of trusting a state label.
Up to ten candidates are considered, newest first. If none qualify, everything is
reported as pre-existing and the comment says no baseline was available — no
attribution is honest, a confident wrong one is not.
Verified against a stub serving a resolve-state candidate, an 8%-size candidate
and a 92% one: the first two are rejected with the reason logged and the third is
used, splitting a genuinely new package from inherited debt.
All ten baseline candidates on develop were rejected as pending/resolve, so no baseline was available and everything fell back to pre-existing. Ten out of ten in an in-progress state is suspicious: the assumption that pending/precrawl/ resolve mean incomplete is an inference from the documented enum, not something the docs state. Logs the state reported for the head commit's own scan. That scan carries 1897 artifacts and 4636 alerts, so its data is demonstrably complete — if it also reports 'resolve', the state filter is wrong and the size check is the only trustworthy signal.
scan_state is not a completion signal. Every scan in this organization persists as 'pending' or 'resolve' — including the head scan itself, which carries 1897 artifacts and 4636 alerts and is demonstrably complete. Filtering on it rejected all ten candidates, so no baseline was ever found and every finding fell back to pre-existing. Replaced with the measurement that matters: how much of this tree the candidate actually shares. A candidate is accepted when at least 90% of the head scan's package@version set is present in it. That tests comparability directly, where the previous artifact-count ratio only approximated it and would have accepted the 1556-artifact scan that caused the 38 false attributions. scan_state is still logged, since it is useful context even though it cannot be trusted as a gate.
Two home-grown baselines failed, and the second failure explained the first. Comparing against the newest scan of the target branch produced 38 false attributions on a change to a single types package. Filtering candidates by scan_state then rejected all ten, because every scan in this organization persists as 'pending' or 'resolve' — the head scan included, whose data is demonstrably complete, so scan_state carries no completion meaning here. Measuring package overlap showed why no baseline was usable at all: the newest target-branch scan shared only 31.8% of this tree's package@version set despite holding 82% as many artifacts. An artifact-count threshold would have accepted it, which is precisely how the 38 false attributions happened. The Socket App already computes the correct diff per pull request, so it is read rather than approximated. The diff scan is looked up by after_full_scan_id, which makes it exact rather than heuristic, and its added/updated/replaced buckets define what this pull request is responsible for. Needs diff-scans:list. Without a diff scan nothing is attributed and everything is reported as pre-existing, so a lookup failure cannot manufacture findings.
…ard links The header stated⚠️ whenever any finding existed, including a pull request that introduced none and only inherited tree debt. That is a pass, and marking it as a warning trains people to ignore the warning that matters. Now: ❌ for blocking introduced findings,⚠️ for introduced but non-blocking, ✅ for none introduced. Adds the dashboard links the App's comment has and ours lacked. Package rows deep-link into the scan when Socket gave the artifact an id, and a footer links the diff, the full project report and the run logs. Only URL shapes observed coming from the App itself are used — socket.dev answers curl with a Cloudflare challenge, so these cannot be verified from CI and a guessed shape would be a dead link. Resolves the diff scan id from the App's own comment. Looking it up by after_full_scan_id returned nothing: the App diffs against a different full scan than the one its Project Report check links to. The comment only carries an id when dependencies actually changed, which is also exactly when attribution is possible — with no id nothing is attributed and everything reads as pre-existing.
The previous form collapsed into a single 253-character line, the only long-line warning this branch adds over the baseline.
Collapsed behind a bare summary line, the pre-existing findings read as an unexplained ▶ with no sense of scale — a reader cannot tell whether expanding it shows two rows or two hundred. Now it gets a heading, a sentence stating what it is and that this change did not touch it, and an action breakdown (🟠 18 warn · 🟡 24 monitor) before the collapsed table. The same breakdown precedes the introduced section, and horizontal rules separate the verdict, the debt and the links.
Summary by CodeRabbit
WalkthroughAdds Socket guarded dependency installation, App checks, API findings reports, pull-request comments, and aggregate status handling to reusable JavaScript validation workflows. It also updates frontend integration, documentation, Dependabot grouping, local ignore rules, and GitHub Action pinning. ChangesSocket supply-chain security
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
lerian-studio
added
size/XL
Contributor
🔍 PR Validation Summary✅ PR Mergeable — no blocking failures
|
Contributor
🔍 Lint Analysis
|
Contributor
🛡️ CodeQL Analysis ResultsLanguages analyzed: ✅ No security issues found. 🔍 View full scan logs | 🛡️ Security tab |
…he go-tooling group across 1 directory (#617)
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Warning
CodeRabbit couldn't request changes on this pull request because it doesn't have sufficient GitHub permissions.
Please grant CodeRabbit Pull requests: Read and write permission and re-run the review.
Actionable comments posted: 20
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/frontend-pr-analysis.yml:
- Around line 55-69: Change the default for the enable_socket_firewall workflow
input from true to false so existing callers retain prior cache and registry
behavior. Keep the explicit opt-in path and related socket_firewall_version and
socket_fail_on_block inputs unchanged.
In @.github/workflows/js-pr-validation.yml:
- Around line 382-389: Update the comments at
.github/workflows/js-pr-validation.yml lines 382-389 and
docs/js-pr-validation.md line 134 to reflect that the Socket API Report step
consumes SOCKET_SECURITY_API_KEY, requiring the full-scans:list and
diff-scans:list scopes; document that the step skips with a notice when the key
is unavailable, matching the existing guidance at line 256, and remove the
outdated Socket Firewall enterprise consumer claim.
In `@src/security/socket-api-report/action.yml`:
- Around line 42-45: Remove the diagnostic probe-package input from the action
metadata and remove its associated probe block from the composite action
implementation. Verify the README inputs table exactly matches the remaining
action.yml inputs, including each name, required value, and default.
- Around line 577-583: Rename the global alert-type aggregation variable in the
block iterating over introduced and preexisting findings from by_type to
type_totals, including its initialization, updates, and sorted-values reference,
while leaving the per-package by_type aggregation unchanged.
- Around line 585-612: Update the README JSON schema/example to document the
actual keys preexistingHidden and preexistingHiddenFindings instead of
preexistingTruncated, matching the output written by the report action and
consumed by socket-reporter. Add the category field to the documented
per-finding objects, while leaving the implementation unchanged.
- Around line 414-418: Rename the parsed diff-scan creation response local in
the diff-scan creation branch from body to created, and update the subsequent
diff_scan lookup to use created. Preserve the original body binding for the
full-scan response.
- Around line 410-418: Update the condition guarding the Socket API Report
diff-scan creation flow to also require !inputs.dry_run, preventing the post
call and scan creation during dry runs while preserving the existing behavior
for normal runs.
In `@src/security/socket-api-report/README.md`:
- Around line 63-67: Update the attribution section in the README to match the
current diff resolution implemented by diff_for_head(): first use the
after_full_scan_id lookup, then use diff-scan-id only as a hint for resolving a
base and rebuilding the diff, and otherwise proceed without attribution. Remove
the outdated claim that the App comment is the sole source and that disabled
comments silently disable attribution.
- Around line 75-82: Update the “Token scopes” table in the README to include
the diff-scans:create scope required by the POST fallback in the action
configuration, and revise the “Grant nothing else” statement to reflect that the
action writes during attribution. Keep the existing read scopes and descriptions
intact.
In `@src/security/socket-app-gate/action.yml`:
- Around line 80-84: Update the jq filter in the check-runs API command to read
the app slug from the process environment via jq’s $ENV.APP_SLUG, and pass
APP_SLUG through the command environment instead of interpolating it into the jq
program. Preserve the existing filtering and output structure while removing
direct shell expansion from the filter.
- Around line 98-122: Move the DEADLINE comparison and timeout handling to the
beginning of the polling loop so it executes before the complete-but-unstable
branch and every other path. Preserve the existing timeout output, warning
message, and break behavior, while removing the now-duplicated later check.
- Around line 149-157: Update the verdict classification around TIMED_OUT,
TOTAL, BAD, and UNDECIDED so any timed-out wait is classified as inconclusive
before the TOTAL=0 missing branch, ensuring timed-out runs never pass as
missing. Preserve findings handling according to the intended priority, either
by evaluating inconclusive before findings or by preventing only the missing
branch when TIMED_OUT is true.
In `@src/security/socket-firewall/README.md`:
- Around line 53-62: Update the README documentation to match the JSON report
flow in the composite action definitions: in
src/security/socket-firewall/README.md lines 53-62, describe report-based
detection and make the zero-exit row pass only when no package is blocked; in
lines 28-35, add blocked-count and findings-file outputs and define blocked by
the report. Apply the equivalent changes in
src/setup/setup-node-guarded/README.md lines 49-58 by renaming the block-marker
column and correcting the zero-exit row, and in lines 26-33 by adding skipped,
blocked-count, and findings-file outputs and using the same blocked definition.
Also verify each inputs table exactly matches its action.yml, including names,
required values, and defaults.
- Line 66: Update the dry-run description in the README to state that dry-run is
preview-only and skips firewall setup, cache purge, installation, and
evaluation. Remove the claim that the install runs through the firewall or that
findings are reported, so the documentation matches the dry-run behavior defined
by the action.
In `@src/security/socket-reporter/action.yml`:
- Around line 99-101: Update the blockingCount calculation near hasFindings to
add blockedPkgs.length when firewall-blocked is true and the report is readable,
while retaining a count of 1 only when firewall-blocked is true but the report
file is unreadable; preserve the existing apiBlocking contribution.
- Around line 173-181: Update the JSON fallback in fixText to return the
truncated serialized object without backticks, since the result is later
processed by cell() and should not render curly quotes. Preserve the existing
string and object-field selection behavior, and let the call site handle any
code-span formatting.
- Around line 270-273: Update the preexisting-results block to pass
api.baseBranch through cell() before interpolating it into the code span,
preserving the fallback for a missing branch. Rename the inner base variable to
a distinct name so it no longer shadows the outer base declaration or creates a
temporal dead zone.
In `@src/security/socket-reporter/README.md`:
- Around line 93-119: Wrap the “As a composite step” YAML example in a complete
jobs.socket job, set runs-on to blacksmith-4vcpu-ubuntu-2404, include the
required permissions, and place the existing firewall, api-report, and Socket
findings steps under steps with consistent indentation.
In `@src/setup/setup-node-guarded/action.yml`:
- Around line 136-142: Update src/setup/setup-node-guarded/action.yml at lines
136-142 to coerce inputs.job-summary from none to errors when passing
job-summary to SocketDev/action, and add the explanatory comment used by the
sibling composite. Update the input description at lines 25-28 to document that
none is coerced to errors, keeping the advertised contract consistent with the
implementation.
- Around line 25-28: Update the job-summary input description and validation
around the job-summary handling logic to reflect the actual behavior when the
value is none: either document that none is coerced to a supported summary mode,
or remove none from the advertised supported values and reject it during
validation. Keep the description and implementation behavior consistent.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: ASSERTIVE
Plan: Pro
Run ID: a37573ba-154a-41a4-8320-5ca9ec6774e7
📒 Files selected for processing (16)
.github/dependabot.yml.github/workflows/frontend-pr-analysis.yml.github/workflows/js-pr-validation.yml.gitignoredocs/frontend-pr-analysis-workflow.mddocs/js-pr-validation.mdsrc/security/socket-api-report/README.mdsrc/security/socket-api-report/action.ymlsrc/security/socket-app-gate/README.mdsrc/security/socket-app-gate/action.ymlsrc/security/socket-firewall/README.mdsrc/security/socket-firewall/action.ymlsrc/security/socket-reporter/README.mdsrc/security/socket-reporter/action.ymlsrc/setup/setup-node-guarded/README.mdsrc/setup/setup-node-guarded/action.yml
There was a problem hiding this comment.
Warning
CodeRabbit couldn't request changes on this pull request because it doesn't have sufficient GitHub permissions.
Please grant CodeRabbit Pull requests: Read and write permission and re-run the review.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/go-ci.yml:
- Line 129: Update the go-coverage-report action reference in the workflow to
use the immutable commit SHA e432de98ee94a276e8f666d25bfd76347665f75b, retaining
the v1.3.1 version comment for readability.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: ASSERTIVE
Plan: Pro
Run ID: e8f62ff9-86b6-427c-a5d3-c86d8611dfc4
📒 Files selected for processing (1)
.github/workflows/go-ci.yml
Pin all 18 third-party action references by commit SHA with a version comment, as required by the Pinned Actions check. Tags are mutable and can be force-pushed upstream. Fix the shellcheck findings in the cross-compilation loop: read -r to stop backslash mangling of the jq output, and quote the target expansions so an os/arch value with whitespace cannot word-split.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Correctness: - socket-app-gate: classify a timed-out wait as inconclusive before the zero-checks 'missing' branch. A slow App expiring the deadline with no checks published produced a green gate, contradicting the documented contract that a timeout is never success. An adverse conclusion already observed keeps priority and is still reported as findings. - socket-app-gate: evaluate the deadline once per iteration so the complete-but-unstable path honours it. A set of checks that kept growing changed the signature on every poll and looped past timeout-seconds. - socket-app-gate: read app-slug from the process environment in the jq program instead of interpolating it into the filter text. - setup-node-guarded: coerce job-summary none to errors, as the sibling composite already did. Without it the pinned action never exports the report path and a genuine block is misread as an ordinary install failure. - js-pr-validation: skip Socket API Report on a dry run. It can POST diff-scans/from-ids, which creates a scan on the Socket organization. - socket-reporter: count refused packages in the blocking heading instead of one per block, so the number agrees with the table beneath it. - socket-reporter: drop the backticks fixText added, which cell() replaced with curly quotes; pass baseBranch through cell() and stop shadowing the outer base holding the dashboard URL. - go-ci: pass build_path and build_targets through env instead of interpolating them into the run block (CodeQL actions/code-injection). Contract and documentation: - socket-api-report: drop probe-package, a diagnostic input that shipped in the public contract undocumented. - Document diff-scans:create, required by the diff the action rebuilds, and drop the claim that it only reads. - Rewrite the attribution section as the three-step resolution the code performs: after_full_scan_id first, the App's id only as a hint for the before side, otherwise no attribution. - Correct the emitted-shape example to preexistingHidden and preexistingHiddenFindings. - Both firewall READMEs: describe report-based block detection, add the blocked-count, findings-file and skipped outputs, and correct the dry-run description, which claimed the install still ran through the firewall. - socket-reporter README: wrap the composite example in a job with the required runner. - Correct both statements that SOCKET_SECURITY_API_KEY is unconsumed.
There was a problem hiding this comment.
Warning
CodeRabbit couldn't request changes on this pull request because it doesn't have sufficient GitHub permissions.
Please grant CodeRabbit Pull requests: Read and write permission and re-run the review.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
.github/workflows/js-pr-validation.yml (2)
601-604: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winSecurity Misconfiguration (CWE-693)
Reachability: External · Exploitability: Moderate
Aggregate paginated comments before selecting the newest diff scan.
gh api --paginate --jq "$JQ"applies$JQper page.head -1can therefore select an older page-local result. That stale ID reachessocket-api-reportas the baseline and can classify introduced findings as pre-existing.Do not combine
--slurpand--jqingh api. Pipe all pages tojq -srand flatten them before applyinglast.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/js-pr-validation.yml around lines 601 - 604, Update the comment retrieval logic in the BODY/ID block to aggregate all paginated responses before selecting the newest socket-security[bot] comment. Remove the per-page --jq filtering, pipe gh api --paginate output to jq -sr, flatten the page arrays, then apply the existing user filter and last selection before extracting the diff-scan ID.
289-376: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winPreserve compatibility for private registries.
socket_enable_firewalldefaults totrueand enables Socket Firewall for every frontend install and the dedicated Socket job. Socket Firewall Free supports public registries only, so private-registry callers can fail during dependency installation. Retain the previous default or publish an explicit breaking migration before enabling this default for external callers.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/js-pr-validation.yml around lines 289 - 376, Change the default for socket_enable_firewall back to false to preserve compatibility with callers using private registries. Keep the existing firewall configuration and opt-in behavior unchanged for workflows that explicitly enable it.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/js-pr-validation.yml:
- Around line 612-616: Update .github/workflows/js-pr-validation.yml lines
612-616 in the Socket API Report gating/output logic so dry-run explicitly
reports API findings as unavailable and never renders an unset blocking count as
0, while preserving resolved-input and computed-variable preview behavior.
Update docs/js-pr-validation.md line 256 to state that dry-run skips API
findings and does not compute an API blocking count.
In `@src/security/socket-app-gate/action.yml`:
- Around line 111-113: Update the signature/deadline branch in the action’s
check-set evaluation so unchanged signatures still write timed_out=false, while
a changed signature with EXPIRED=true writes timed_out=true before breaking.
Preserve BAD priority so an already observed adverse check remains
authoritative.
---
Outside diff comments:
In @.github/workflows/js-pr-validation.yml:
- Around line 601-604: Update the comment retrieval logic in the BODY/ID block
to aggregate all paginated responses before selecting the newest
socket-security[bot] comment. Remove the per-page --jq filtering, pipe gh api
--paginate output to jq -sr, flatten the page arrays, then apply the existing
user filter and last selection before extracting the diff-scan ID.
- Around line 289-376: Change the default for socket_enable_firewall back to
false to preserve compatibility with callers using private registries. Keep the
existing firewall configuration and opt-in behavior unchanged for workflows that
explicitly enable it.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 2e5de80f-d63d-43ba-8828-6c065cefcbc3
📒 Files selected for processing (12)
.github/workflows/go-ci.yml.github/workflows/js-pr-validation.ymldocs/js-pr-validation.mdsrc/security/socket-api-report/README.mdsrc/security/socket-api-report/action.ymlsrc/security/socket-app-gate/action.ymlsrc/security/socket-firewall/README.mdsrc/security/socket-firewall/action.ymlsrc/security/socket-reporter/README.mdsrc/security/socket-reporter/action.ymlsrc/setup/setup-node-guarded/README.mdsrc/setup/setup-node-guarded/action.yml
socket-app-gate: a deadline expiring while the check set is still unstable now writes timed_out=true. The previous commit stopped the loop there but recorded a decided, clean wait, so a set that never produced the second stable snapshot evaluated to pass — reintroducing the exact race the stability check exists to close. BAD keeps priority, so an adverse check already observed is still findings. js-pr-validation: the dry-run notice printed 'Blocking findings: 0' for a layer that does not run under dry_run, rendering an unset output as a clean result. It now states the count is not computed and why, and the workflow doc records that this layer is not previewable.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
.github/workflows/js-pr-validation.yml (4)
673-688: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winPrint the resolved Socket configuration in dry-run mode.
This step prints the layer outcomes and the intentionally uncomputed API count. It does not print the resolved Socket inputs that control the preview, including
run_socket,socket_enable_firewall,socket_enable_app_gate,socket_enable_api_report, and the failure policies. Map non-secret inputs throughenvand print them with::notice::. Never printSOCKET_SECURITY_API_KEY.As per path instructions, dry-run mode must print every resolved input and computed variable.
Suggested dry-run output
env: + RUN_SOCKET: ${{ inputs.run_socket }} + SOCKET_ENABLE_FIREWALL: ${{ inputs.socket_enable_firewall }} + SOCKET_ENABLE_APP_GATE: ${{ inputs.socket_enable_app_gate }} + SOCKET_ENABLE_API_REPORT: ${{ inputs.socket_enable_api_report }} + SOCKET_FAIL_ON_BLOCK: ${{ inputs.socket_fail_on_block }} + SOCKET_API_FAIL_ON_ACTIONS: ${{ inputs.socket_api_fail_on_actions }} FIREWALL_OUTCOME: ${{ steps.firewall.outcome }} APP_GATE_OUTCOME: ${{ steps.app-gate.outcome }} API_BLOCKING: ${{ steps.api-report.outputs.blocking-count }} run: | echo "::notice::DRY RUN — Socket gate not enforced" + echo "::notice::Socket inputs: run_socket=$RUN_SOCKET firewall=$SOCKET_ENABLE_FIREWALL app_gate=$SOCKET_ENABLE_APP_GATE api_report=$SOCKET_ENABLE_API_REPORT fail_on_block=$SOCKET_FAIL_ON_BLOCK api_fail_on_actions=$SOCKET_API_FAIL_ON_ACTIONS"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/js-pr-validation.yml around lines 673 - 688, Update the “Gate - Socket findings (dry run)” step to expose every resolved non-secret Socket configuration input and computed variable required by the workflow, including run_socket, socket_enable_firewall, socket_enable_app_gate, socket_enable_api_report, and all failure policies. Map these values through the step’s env and print each with ::notice:: alongside the existing outcomes and API status; never expose SOCKET_SECURITY_API_KEY.Source: Path instructions
546-688: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd titled sections inside the Socket job.
The job contains separate checkout, Firewall, App Gate, scan-ID resolution, API report, PR report, enforced gate, and dry-run gate groups. Separate each group with a comment using the required
# ----------------- Title -----------------format.As per coding guidelines, jobs with more than one logical group must use titled section comments.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/js-pr-validation.yml around lines 546 - 688, Add titled section comments throughout the Socket job using the required “# ----------------- Title -----------------” format. Separate the Checkout, Socket Firewall, Socket App Gate, diff-scan resolution, API report, PR report, enforced gate, and dry-run gate groups, placing each title immediately before its corresponding step group.Source: Coding guidelines
592-610: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winSecurity Misconfiguration (CWE-693)
Reachability: External
Select the latest Socket App comment across all pages.
gh api --paginate --jqemits each page separately, solastis page-local. Becausegrep ... | head -1selects the first matching ID, an older page can provide a stale diff-scan baseline. Use--slurpand flatten all pages before applyinglast.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/js-pr-validation.yml around lines 592 - 610, Update the JQ query in the “Resolve Socket diff scan id” step to use gh api’s --slurp option and flatten the paginated comment results before selecting the last Socket App comment. Preserve the existing body extraction and ID parsing flow so the latest matching comment across all pages supplies ID.
614-630: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy liftSecurity Misconfiguration (CWE-693)
Reachability: External · Exploitability: Moderate
Preserve enforcement when Socket diff attribution is unavailable.
When
diff_scan_idis absent or unresolved,socket-api-reportclassifies findings as pre-existing and emits no blocking count. The workflow converts the missing count to0, sosocket_api_fail_on_actionscannot reject a pull request that introduces an actioned finding.Return an explicit attribution-unavailable state and apply the configured block-or-warn policy. Update
docs/js-pr-validation.mdto document this policy.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/js-pr-validation.yml around lines 614 - 630, Update the Socket API report flow in the “Socket API Report” step to preserve enforcement when diff attribution is absent or unresolved: return an explicit attribution-unavailable state instead of converting the missing blocking count to zero, then apply the configured block-or-warn policy based on socket_api_fail_on_actions. Update docs/js-pr-validation.md at line 268 to document this attribution-unavailable behavior and the resulting configured policy.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In @.github/workflows/js-pr-validation.yml:
- Around line 673-688: Update the “Gate - Socket findings (dry run)” step to
expose every resolved non-secret Socket configuration input and computed
variable required by the workflow, including run_socket, socket_enable_firewall,
socket_enable_app_gate, socket_enable_api_report, and all failure policies. Map
these values through the step’s env and print each with ::notice:: alongside the
existing outcomes and API status; never expose SOCKET_SECURITY_API_KEY.
- Around line 546-688: Add titled section comments throughout the Socket job
using the required “# ----------------- Title -----------------” format.
Separate the Checkout, Socket Firewall, Socket App Gate, diff-scan resolution,
API report, PR report, enforced gate, and dry-run gate groups, placing each
title immediately before its corresponding step group.
- Around line 592-610: Update the JQ query in the “Resolve Socket diff scan id”
step to use gh api’s --slurp option and flatten the paginated comment results
before selecting the last Socket App comment. Preserve the existing body
extraction and ID parsing flow so the latest matching comment across all pages
supplies ID.
- Around line 614-630: Update the Socket API report flow in the “Socket API
Report” step to preserve enforcement when diff attribution is absent or
unresolved: return an explicit attribution-unavailable state instead of
converting the missing blocking count to zero, then apply the configured
block-or-warn policy based on socket_api_fail_on_actions. Update
docs/js-pr-validation.md at line 268 to document this attribution-unavailable
behavior and the resulting configured policy.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 9b614fcd-6de0-4595-959e-9f1392f3aac9
📒 Files selected for processing (3)
.github/workflows/js-pr-validation.ymldocs/js-pr-validation.mdsrc/security/socket-app-gate/action.yml
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
3 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.
GitHub Actions Shared Workflows
Description
Type of Change
feat: New workflow or new input/output/step in an existing workflowfix: Bug fix in a workflow (incorrect behavior, broken step, wrong condition)perf: Performance improvement (e.g. caching, parallelism, reduced steps)refactor: Internal restructuring with no behavior changedocs: Documentation only (README, docs/, inline comments)ci: Changes to self-CI (workflows under.github/workflows/that run on this repo)chore: Dependency bumps, config updates, maintenancetest: Adding or updating testsBREAKING CHANGE: Callers must update their configuration after this PRBreaking Changes
None.
Testing
@this-branchor the beta tagCaller repo / workflow run:
Related Issues
Closes #