Skip to content

Add Socket Basics security scanning workflow - #5460

Open
kanwalpreetd wants to merge 6 commits into
stellar:masterfrom
kanwalpreetd:master
Open

kanwalpreetd wants to merge 6 commits into
stellar:masterfrom
kanwalpreetd:master

Conversation

@kanwalpreetd

@kanwalpreetd kanwalpreetd commented Sep 17, 2026 •

Copy link
Copy Markdown
Contributor

@kanwalpreetd
kanwalpreetd force-pushed the master branch 4 times, most recently from 62a98c8 to 34c0284 Compare September 26, 2026 01:30
@kanwalpreetd
kanwalpreetd marked this pull request as ready for review September 28, 2026 12:40
Copilot AI balanced review requested due to automatic review settings September 28, 2026 12:40

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

The missing-completion-marker path can report an error while still exiting successfully.

Review effort: Balanced
Findings: 1 Medium severity

Open (1)
What changed in this PR

Adds scheduled and on-demand Socket Basics security scanning.

Changes:

  • Configures SAST, secret, and Dockerfile scanning.
  • Adds scan exclusions and rule suppressions.
  • Runs a pinned scanner image and reports findings.
File Description
.github/​workflows/​socket-basics.yml Defines the security scan workflow.
.socket-basics.json Configures scanners and exclusions.
.semgrepignore Excludes generated, vendored, and test paths.
.trivyignore Suppresses selected Dockerfile rules.

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread .github/workflows/socket-basics.yml Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

The workflow can report success after scanner or upload failures, and intended golden-data exclusions do not match actual paths.

Review effort: Balanced
Findings: 1 High severity · 1 Medium severity

Open (2)
Resolved since last review (1)

Comment thread .github/workflows/socket-basics.yml Outdated
Comment thread .socket-basics.json Outdated
Copilot AI review requested due to automatic review settings September 29, 2026 08:53

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Trivy exclusions are ineffective, golden directories remain scanned, and abnormal container failures can be reported as successful.

Review effort: Balanced
Findings: 1 High severity · 3 Medium severity · 1 Low severity

Open (5)

Comment thread .github/workflows/socket-basics.yml Outdated
Comment thread .trivyignore
Comment thread .semgrepignore Outdated
Copilot AI balanced review requested due to automatic review settings September 29, 2026 22:05

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Copilot AI balanced review requested due to automatic review settings September 29, 2026 22:43

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Secret scanning broadly excludes tracked tests, fixtures, examples, and lockfiles, creating significant detection blind spots.

Review effort: Balanced
Findings: 1 High severity · 4 Medium severity · 1 Low severity

Open (6)

Comment thread .socket-basics.json Outdated
Copilot AI balanced review requested due to automatic review settings September 29, 2026 23:19

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🔵 Needs a closer look

The SAST exclusion references a nonexistent directory, leaving both generated LCM golden directories in scope.

Review effort: Balanced
Findings: 1 High severity · 3 Medium severity · 1 Low severity

Open (5)
Resolved since last review (1)

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment thread .trivyignore
Runs SAST through OpenGrep, secret scanning through TruffleHog, and
Dockerfile misconfiguration scanning through Trivy, submitting results
to Socket.dev.

  .github/workflows/socket-basics.yml  scheduled weekly + manual dispatch
  .socket-basics.json                  scanner configuration
  .semgrepignore                       SAST path exclusions
  .trivyignore                         Dockerfile lint rules with no
                                       security dimension (only present
                                       where the repo has a Dockerfile)

Separate from socket-scan.yml, which covers dependency CVEs and Tier 1
reachability.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Copilot AI balanced review requested due to automatic review settings October 2, 2026 00:49

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🔵 Needs a closer look

The extensive security-scan suppressions and third-party scanner integration warrant final human validation.

Review effort: Balanced
Findings: None

Resolved since last review (6)

Copilot AI balanced review requested due to automatic review settings October 2, 2026 00:53

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🔵 Needs a closer look

The broad security-scanner suppression policy and pinned external image require final human validation.

Review effort: Balanced
Findings: None

The generic test patterns (63 directory names, 23 file globs) were applied
to TruffleHog as well as OpenGrep. Excluding test code from SAST is sound,
but a credential committed to a test file is as live as one anywhere else,
and trufflehog_exclude_dir stops the file being opened -- making it a
permanent blind spot for every future secret in those paths.

Measured across all 120 repos: dropping the test patterns surfaces 77
additional detections in 20 files across 7 repos, and all 77 are false
positives -- 43 are UUIDs and random column values in CSV migration
fixtures, the rest one generated ECDSA test key pasted into nine files,
localhost postgres DSNs, and test JWTs. Those 20 files are now listed
individually, so every other test file is scanned for secrets.

.semgrepignore is unchanged; SAST keeps its test exclusions.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Copilot AI balanced review requested due to automatic review settings October 2, 2026 01:50

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🔵 Needs a closer look

The broad security-scan suppressions require human validation against actual scanner output.

Review effort: Balanced
Findings: None

The job inherited GitHub's 360-minute default. Measured scan work across
all 120 repos is 30.3s at the slowest and 8.3s at the median, so 30
minutes leaves roughly sixty times the observed worst case while still
failing a hung run in minutes rather than hours.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Copilot AI balanced review requested due to automatic review settings October 2, 2026 02:26

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🔵 Needs a closer look

Scanner and upload failures can currently produce a successful workflow result.

Review effort: Balanced
Findings: None

Previously missed (1)

In code that hasn't changed since last review

Medium severity Fail the job when scanner connectors or result upload fail

.github/​workflows/​socket-basics.yml:49

This direct invocation trusts the scanner's exit status, but the pinned scanner runs connectors with fail_fast: false, catches Socket API submission failures, and then exits based only on the high/critical finding count. An OpenGrep, TruffleHog, Trivy, or dashboard-upload failure can therefore leave this weekly job green with partial or missing results. Gate success on completion of every enabled connector and a successful full-scan upload, or pin a scanner release that fails closed for those errors.

The Protocol 30 merge added src/rust/soroban/p29 and p30 as submodules.
repo_ignores() reads .gitmodules, so regenerating .semgrepignore picks
them up alongside p21-p28; without this they would be scanned as
first-party code once checked out.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Copilot AI balanced review requested due to automatic review settings October 2, 2026 04:45

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Scanner failures can currently produce successful, incomplete security runs.

Review effort: Balanced
Findings: 1 High severity

Open (1)
Previously missed (1)

In code that hasn't changed since last review

Low severity Comment incorrectly claims trivy_disabled_rules is unsupported

.trivyignore:4

This explanation is incorrect for the configured Socket Basics generation: upstream exposes trivy_disabled_rules as a JSON configuration key (it is documented and present as far back as v3.0.0). Keeping the comment as written will mislead future tuning work into maintaining a separate ignore mechanism unnecessarily.

-e GITHUB_REPOSITORY \
-e GITHUB_REF_NAME \
-e GITHUB_SHA \
ghcr.io/socketdev/socket-basics@sha256:d463bae84f21d0240d5e197acb81e95144a966a20ab7cd05cca3a70e1796a4bb \
@kanwalpreetd
kanwalpreetd requested a review from anupsdf October 2, 2026 19:38
Copilot AI balanced review requested due to automatic review settings October 5, 2026 16:19

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🔵 Needs a closer look

The secret-bearing third-party security workflow and extensive suppression policy warrant final human security review.

Review effort: Balanced
Findings: 1 High severity · 1 Low severity

Open (2)

Comment thread .trivyignore
Comment on lines +3 to +4
# Trivy reads this file from the working directory. socket-basics has no config
# key that can disable a Trivy rule, so this is the only lever.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants