Skip to content

feat(scanners): add Endor dependency reachability analysis - #55

Draft
jesse-merhi wants to merge 5 commits into
mainfrom
jesse/endor-local-scanner
Draft

jesse-merhi wants to merge 5 commits into
mainfrom
jesse/endor-local-scanner

Conversation

@jesse-merhi

@jesse-merhi jesse-merhi commented Sep 16, 2026

Copy link
Copy Markdown
Member

Add clawscan --scanner endor for JavaScript and TypeScript dependency and function-reachability analysis in Docker. The companion ClawHub PR #3742 runs it from the background plugin scan queue.

The adapter preserves raw findings and reports command failures, invalid output and upstream analysis errors. Unsupported URLs now skip before credential or Docker preflight; local plugin targets retain their credential checks.

A sanitized input copy is mounted read-only. Git and Endor run in a private container copy so non-root Linux hosts can remove their input after success or failure. Git trusts only that private path, and install scripts and Git hooks are disabled.

Evidence

  • The actual CLI with --scanner endor and a URL, no Endor credentials and no Docker on PATH now exits 0 with a skipped result. A local package in the same environment still exits 1 for missing credentials. Full-entrypoint regression tests cover both paths and confirm Docker is never checked for the URL.
  • Focused tests, full go test -count=1 ./... and go vet ./... passed on the URL fix with Go 1.27.0 on Darwin ARM64. Focused native and independent review found no regressions in that fix. Hosted CI and both CodeQL analyses passed on this head.
  • Earlier Linux acceptance verified mixed-owner workspace access, rejection of an unrelated mixed-owner repository, source preservation and cleanup with network disabled. All six npm binary targets built, and tarball installation and smoke checks passed; these package checks preceded the URL-only capability fix.
  • Real scans covered Discord, Slack and WhatsApp. The unchanged Endor execution path completed a Slack scan in the pinned Linux x64 image in 52.543 seconds with one reachable-function finding and unchanged source.

Remaining review blockers

Credential isolation from target-controlled package-manager configuration is unresolved. A real Endor 1.7.1115 probe used synthetic credentials and a local capture endpoint with .npmrc interpolation. Endor stopped at its required tenant system-config response before dependency resolution. Zero requests reached the target registry; this neither confirms a leak nor proves isolation. The next check needs a controlled authenticated tenant or a valid vendor API fixture and must observe the final registry request.

The preexisting shared Docker runner can leave a container running after its client is killed on timeout. Generic container teardown remains a separate follow-up before hosted rollout. No package or image has been released. This PR remains draft while the credential boundary is unresolved.

Change Breakdown

Part Files +LOC -LOC
Implementation and tests 9 +980 -20
Documentation 1 +38 -0
Total 10 +1018 -20

@clawsweeper

clawsweeper Bot commented Sep 16, 2026

Copy link
Copy Markdown

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

@clawsweeper clawsweeper Bot added P2 Normal priority bug or improvement with limited blast radius. merge-risk: 🚨 security-boundary 🚨 Merging this PR could weaken sandboxing, authorization, credentials, or sensitive data. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Sep 16, 2026
@clawsweeper

clawsweeper Bot commented Sep 16, 2026

Copy link
Copy Markdown

Codex review: needs real behavior proof before merge. Reviewed September 16, 2026, 2:56 AM ET / 06:56 UTC (Revision 3).

ClawSweeper review

What this changes

Adds a Docker-only Endor scanner for JavaScript and TypeScript dependencies, with raw findings, credential discovery, CLI summaries, runtime-image instructions, and regression tests.

Merge readiness

Blocked before merge - 4 items remain

The prior URL-preflight finding is resolved. This remains useful work absent from current main, but the acknowledged credential-isolation gap still blocks merge.

Likely related people: jesse-merhi and Patrick Erichsen are routing candidates based on earlier scanner and sandbox work.

Priority: P2
Reviewed head: 8da3f951017e4bc4117e16c688e0c5ac2947c04f

Review scores

Measure Result What it means
Overall readiness 🦐 gold shrimp (3/6) Useful implementation and resolved preflight feedback are outweighed by the remaining material credential-boundary uncertainty.
Proof confidence 🦐 gold shrimp (3/6) Needs stronger real behavior proof before merge: Authority-chain proof required: the reported real Slack scan exercises runEndor successfully, but the credential-isolation probe stops at tenant system-config before the final registry request. Show authorized Endor API access and prevention of credential disclosure to the target-controlled registry through the real dependency path. Ordinary contributor proof remains exempt for this MEMBER-authored PR. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Patch quality 🦐 gold shrimp (3/6) Security review found an item that needs attention.

Verification

Check Result Evidence
Real behavior Needs proof Needs stronger real behavior proof before merge: Authority-chain proof required: the reported real Slack scan exercises runEndor successfully, but the credential-isolation probe stops at tenant system-config before the final registry request. Show authorized Endor API access and prevention of credential disclosure to the target-controlled registry through the real dependency path. Ordinary contributor proof remains exempt for this MEMBER-authored PR. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Evidence reviewed 7 items Repository policy: Read the complete root AGENTS.md. Registry integration, raw scanner evidence, environment-only credentials, and Docker execution informed the review. No applicable nested policy or maintainer-note directory was found. The host-managed review contract supersedes the bundled autoreview instruction; no helper, tests, or builds were executed.
Previous finding resolved: The head commit adds explicit Endor target capabilities and full Run-entrypoint tests for credential-free, Docker-free URL skipping and retained local credential validation. Existing runner.go filters runnable scanners before both preflight checks. GitHub's exact commit patch supplied continuity evidence after the local prior-head comparison encountered an unavailable blob.
Credential authority boundary: The new shell script executes endorctl against copied target files. copyTargetToWorkspace excludes Git metadata and prior result directories, but retains package-manager configuration; sandboxEnvNames and dockerCommandRunner forward the required Endor credentials into the network-enabled container. This directly depends on Endor's dependency-resolution behavior. Disabling lifecycle scripts does not establish credential isolation from registry configuration.
Findings None None.
Security Needs attention Unresolved credential exposure through dependency configuration: The adapter copies target package-manager configuration into the directory scanned by credential-bearing Endor. The reported probe never reached dependency resolution, so it cannot establish whether a target-controlled registry receives credentials. Treat this as a blocking uncertainty, not a confirmed leak.

How this fits together

ClawScan selects scanner adapters for local skills and plugins, runs them in Docker, and records their evidence for reporting or an external judge. The new adapter sends a disposable project copy through Endor dependency analysis and returns findings or an explicit failure.

flowchart TD
  A[Target and scanner selection] --> B{Supported target kind}
  B -->|Unsupported| C[Skipped result]
  B -->|Local project| D[Credential and Docker checks]
  D --> E[Disposable project copy]
  E --> F[Endor dependency analysis]
  F --> G[Endor API and dependency registries]
  F --> H[Raw findings and scan status]
Loading

Before merge

  • Add real behavior proof - Needs stronger real behavior proof before merge: Authority-chain proof required: the reported real Slack scan exercises runEndor successfully, but the credential-isolation probe stops at tenant system-config before the final registry request. Show authorized Endor API access and prevention of credential disclosure to the target-controlled registry through the real dependency path. Ordinary contributor proof remains exempt for this MEMBER-authored PR. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
  • Resolve security concern: Unresolved credential exposure through dependency configuration - The adapter copies target package-manager configuration into the directory scanned by credential-bearing Endor. The reported probe never reached dependency resolution, so it cannot establish whether a target-controlled registry receives credentials. Treat this as a blocking uncertainty, not a confirmed leak.
  • Resolve merge risk (P1) - Target-controlled package-manager configuration remains inside the credential-bearing Endor process boundary; whether it can cause scanner credentials to reach an unauthorized registry is unresolved.
  • Complete next step (P2) - Establish the Endor credential boundary with final-request evidence for authorized API access and a target-controlled registry before merge.

Findings

  • [high] Unresolved credential exposure through dependency configuration — internal/runner/endor_scanner.go:34
Agent review details

Security

Needs attention: Image pinning, checksum verification, and disabled hooks are useful safeguards, but credential isolation during dependency resolution remains unproven.

Review metrics

Metric Value Why it matters
Production versus test delta Production/runtime/schema +285/-15; tests +695/-5; docs +38 Production growth supports a distinct scanner adapter and runtime, with most added lines devoted to validation.

Merge-risk options

Maintainer options:

  1. Establish credential isolation (recommended)
    Keep merge blocked until the real dependency path demonstrates authorized Endor access without forwarding scanner credentials to a target-controlled registry.
  2. Retain the draft
    Pause this integration if the Endor credential boundary cannot yet be demonstrated safely.

Technical review

Best possible solution:

Preserve raw Endor evidence while ensuring dependency resolution cannot expose scanner credentials to target-selected destinations, with final-request proof of that boundary.

Do we have a high-confidence way to reproduce the issue?

Unclear for the remaining security concern: the reported probe stopped before the registry request, so no credential leak or isolation has been reproduced. The previous URL-preflight defect is resolved by the inspected capability gating and regression coverage.

Is this the best way to solve the issue?

The registry-based adapter and disposable workspace fit the existing architecture, but security suitability remains unclear until credential isolation is established through the actual dependency path.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning medium; reviewed against 84ee99ce015b.

Labels

Label justifications:

  • P2: This is an optional scanner integration with useful coverage and no established urgent regression in shipped behavior.
  • merge-risk: 🚨 security-boundary: The introduced Endor execution path combines scanner credentials with target-controlled dependency configuration before network requests.
  • rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🦐 gold shrimp.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs stronger real behavior proof before merge: Authority-chain proof required: the reported real Slack scan exercises runEndor successfully, but the credential-isolation probe stops at tenant system-config before the final registry request. Show authorized Endor API access and prevention of credential disclosure to the target-controlled registry through the real dependency path. Ordinary contributor proof remains exempt for this MEMBER-authored PR. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Evidence

Security concerns:

  • [high] Unresolved credential exposure through dependency configuration — internal/runner/endor_scanner.go:34
    The adapter copies target package-manager configuration into the directory scanned by credential-bearing Endor. The reported probe never reached dependency resolution, so it cannot establish whether a target-controlled registry receives credentials. Treat this as a blocking uncertainty, not a confirmed leak.
    Confidence: 0.96

What I checked:

  • Repository policy: Read the complete root AGENTS.md. Registry integration, raw scanner evidence, environment-only credentials, and Docker execution informed the review. No applicable nested policy or maintainer-note directory was found. The host-managed review contract supersedes the bundled autoreview instruction; no helper, tests, or builds were executed. (AGENTS.md:1, 8da3f951017e)
  • Previous finding resolved: The head commit adds explicit Endor target capabilities and full Run-entrypoint tests for credential-free, Docker-free URL skipping and retained local credential validation. Existing runner.go filters runnable scanners before both preflight checks. GitHub's exact commit patch supplied continuity evidence after the local prior-head comparison encountered an unavailable blob. (internal/runner/scanner_registry.go:310, 8da3f951017e)
  • Credential authority boundary: The new shell script executes endorctl against copied target files. copyTargetToWorkspace excludes Git metadata and prior result directories, but retains package-manager configuration; sandboxEnvNames and dockerCommandRunner forward the required Endor credentials into the network-enabled container. This directly depends on Endor's dependency-resolution behavior. Disabling lifecycle scripts does not establish credential isolation from registry configuration. (internal/runner/endor_scanner.go:34, 8da3f951017e)
  • Captured proof and explicit unresolved boundary: The supplied body at sourceRevision 46b02f03d8677f28b8f3127e0ed71bd97fafe48a880c59a87bbbe56c9cb11fd7 reports a 52.543-second real Slack plugin scan and successful source-preservation checks. It separately reports that the Endor 1.7.1115 synthetic-credential probe stopped at tenant system-config before dependency resolution, leaving final registry requests unobserved. The live body read matched that account. This is positive runtime evidence with an explicitly inconclusive security experiment, not missing ordinary contributor proof. (8da3f951017e)
  • Still necessary on main and release: Current main's scanner registry has no Endor adapter; the complete main-tree Endor search found only unrelated substring matches. The latest release remains v0.1.8, whose commit is the PR merge base. The bounded GitHub pull listing returned this PR as the only Endor/reachability match, with no merged replacement. (internal/runner/scanner_registry.go, 84ee99ce015b)
  • Merged-area routing history: Main history includes Jesse Merhi's explicit sandbox-mount changes and user-defined scanner work, plus Patrick Erichsen's scanner catalog and command-sandbox work. GitHub identifies jesse-merhi as author of the sandbox-mount commit. These support routing candidates, without asserting exact source-line introduction. Deeper content-history inspection encountered unavailable historical blobs. (internal/runner/sandbox.go, a9b45ab69396)

Likely related people:

  • jesse-merhi: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)
  • Patrick Erichsen: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

  • Provide final-request evidence that authorized Endor access succeeds while target-controlled registry configuration cannot receive scanner credentials; terminal traces or diagnostic logs are appropriate.
  • Redact credentials, private endpoints, and other private details before updating the PR body for automatic re-review; if it does not trigger, ask a maintainer to comment @clawsweeper re-review.

Rating scale

Score Internal tier Crab rank Meaning
6/6 S 🦀 challenger crab Exceptional readiness
5/6 A 🦞 diamond lobster Very strong readiness
4/6 B 🐚 platinum hermit Good normal PR; ordinary maintainer review
3/6 C 🦐 gold shrimp Useful, but confidence is limited
2/6 D 🦪 silver shellfish Proof or implementation needs work
1/6 F 🧂 unranked krab Not merge-ready
N/A NA 🌊 off-meta tidepool Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

History

Review history (2 earlier review cycles)
  • reviewed 2026-09-16T06:14:41.391Z sha b9f3dbc :: needs real behavior proof before merge. :: [P2] Exclude URL targets before Endor credential and Docker preflight
  • reviewed 2026-09-16T06:19:51.702Z sha b9f3dbc :: needs real behavior proof before merge. :: [P2] Exclude URL targets before Endor credential and Docker preflight

@jesse-merhi

Copy link
Copy Markdown
Member Author

/clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Sep 16, 2026

Copy link
Copy Markdown

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event exact_review_queue).
Result: when the review finishes, ClawSweeper will create the durable review comment if needed or update the existing comment in place.

Re-review progress:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 security-boundary 🚨 Merging this PR could weaken sandboxing, authorization, credentials, or sensitive data. P2 Normal priority bug or improvement with limited blast radius. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant