Skip to content

feat(miner-hands): add shared subprocess redaction/env-allowlist helper to gittensory-engine (#4284) - #4451

Closed
nickmopen wants to merge 2 commits into
JSONbored:mainfrom
nickmopen:feat/engine-subprocess-env-helper
Closed

feat(miner-hands): add shared subprocess redaction/env-allowlist helper to gittensory-engine (#4284)#4451
nickmopen wants to merge 2 commits into
JSONbored:mainfrom
nickmopen:feat/engine-subprocess-env-helper

Conversation

@nickmopen

Copy link
Copy Markdown
Contributor

Closes #4284. Re-submit of #4442, which the gate auto-closed on a stale codecov/patch — it closed the PR before codecov recomputed from the fixed coverage upload. The coverage fix is included and confirmed locally (lcov: subprocess-env.ts LF:14/LH:14 = 100%, zero uncovered).

Promotes the review-CLI subprocess safety pattern — a strict allowlisted child env + secret redaction — out of src/selfhost/ai.ts into @jsonbored/gittensory-engine, so the coming gittensory-miner coding-agent drivers depend on one source of truth.

What's here

  • packages/gittensory-engine/src/subprocess-env.tsSUBPROCESS_CLI_ENV_ALLOWLIST + a parameterized buildAllowlistedEnv(parent, allowlist, extra) (caller passes its own allowlist, not hardcoded) + SECRET_PATTERNS (OpenAI/Anthropic, GitHub PAT + fine-grained, JWT, AWS — ported verbatim) + redactSecrets(text, knownSecrets). Re-exported from the barrel.
  • src/selfhost/ai.ts — migration story documented: its copy kept parallel for now (its subscriptionCliEnv also folds in CLI-specific PATH resolution), with a cross-reference comment. No behavior change.
  • Tests — engine node:test (324/324) and an app-vitest test importing the engine src (so codecov/patch counts it — the opportunity-ranker convention).

Validation

  • codecov/patch confirmed locally via lcov (14/14 lines hit on the changed file).
  • Engine 324/324; app typecheck clean; full suite 12674 passed, 0 failed.

nickmopen added 2 commits July 9, 2026 13:46
…er to gittensory-engine (JSONbored#4284)

Promotes the review-CLI subprocess safety pattern (a strict allowlisted child env +
secret redaction) out of src/selfhost/ai.ts into the engine, so the coming
gittensory-miner coding-agent drivers depend on one source of truth instead of
copy-pasting it.

- packages/gittensory-engine/src/subprocess-env.ts: SUBPROCESS_CLI_ENV_ALLOWLIST (the
  standard list) + a PARAMETERIZED buildAllowlistedEnv(parent, allowlist, extra) (a
  caller can pass a different/larger allowlist — not hardcoded), plus SECRET_PATTERNS
  (OpenAI/Anthropic, GitHub PAT/fine-grained, JWT, AWS — ported verbatim, not weakened)
  and redactSecrets(text, knownSecrets). Re-exported from the engine barrel.
- src/selfhost/ai.ts: migration story documented — its copy is deliberately kept
  PARALLEL for now (its subscriptionCliEnv also folds in CLI-specific PATH resolution),
  with a cross-reference comment to the shared engine helper (shim later if it drifts,
  like predicted-gate.ts). No behavior change to ai.ts.
- Tests (node:test): parameterized allowlist honored + extra/undefined handling; every
  SECRET_PATTERNS family redacted + the known-secret length guard.

Verified: engine 324/324 pass; app typecheck clean; full suite 12674 passed, 0 failed.
…nv helper

codecov/patch is computed from the app vitest run (vitest.config coverage includes
packages/gittensory-engine/src/**), and the engine's own node:test doesn't feed it.
Add an app-vitest test importing the engine SRC directly (the opportunity-ranker
convention) so the changed engine lines are covered. Confirmed locally: lcov shows
subprocess-env.ts LF:14/LH:14 (100%, 0 uncovered).
@nickmopen
nickmopen requested a review from JSONbored as a code owner July 9, 2026 19:13
@superagent-security

Copy link
Copy Markdown
Contributor

Superagent didn't find any vulnerabilities or security issues in this PR.

@codecov

codecov Bot commented Jul 9, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.95%. Comparing base (564a27c) to head (7447a4a).
⚠️ Report is 15 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #4451   +/-   ##
=======================================
  Coverage   93.95%   93.95%           
=======================================
  Files         412      413    +1     
  Lines       37212    37228   +16     
  Branches    13591    13595    +4     
=======================================
+ Hits        34963    34979   +16     
  Misses       1594     1594           
  Partials      655      655           
Files with missing lines Coverage Δ
packages/gittensory-engine/src/subprocess-env.ts 100.00% <100.00%> (ø)
src/selfhost/ai.ts 98.65% <ø> (ø)
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@loopover-orb loopover-orb Bot added the gittensor:feature Gittensor-scored feature linked to a feature issue — scores a 0.25x multiplier. label Jul 9, 2026
@loopover-orb

loopover-orb Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Caution

🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥

🛑 Gittensory review result - reject/close recommended

Review updated: 2026-07-09 19:27:52 UTC

5 files · 1 AI reviewer · 1 blocker · readiness 100/100 · CI green · unstable

🛑 Suggested Action - Reject/Close

  • Possible leaked secret in the diff (aws_access_key, generic_secret_assignment, github_pat, github_token) — Remove the secret from the diff, rotate the exposed credential, then re-run the gate.

Review summary
This is a clean, additive promotion of the existing subscription-CLI env-allowlist + secret-redaction pattern from src/selfhost/ai.ts into a new packages/gittensory-engine/src/subprocess-env.ts module, exported from the engine barrel. ai.ts itself is untouched except for a documentation comment noting the two copies are deliberately kept parallel (its PATH-resolution logic differs) — so there is no behavior change on the existing review path, matching the PR description. The new buildAllowlistedEnv/redactSecrets functions are pure, parameterized (no hardcoded owner/repo assumptions), and covered by both an engine node:test suite and an app-vitest test that imports the src directly for codecov/patch, with CI green across build/typecheck/tests/security/codecov.

Nits — 5 non-blocking
  • The 'leaked secret' flags in packages/gittensory-engine/test/subprocess-env.test.ts:25 and test/unit/engine-subprocess-env.test.ts:29 are false positives — AKIAIOSFODNN7EXAMPLE is AWS's standard published example key used for testing regex matches, not a real credential, but worth a quick scanner-allowlist entry so it stops surfacing on every PR touching this pattern.
  • Two copies of the allowlist/redaction logic now exist (ai.ts's SUBSCRIPTION_CLI_ENV_ALLOWLIST and SECRET_PATTERNS vs. the new engine module) — the PR's own comment at src/selfhost/ai.ts flags this and points to a follow-up shim; worth filing that follow-up now so it doesn't silently drift like the comment warns.
  • packages/gittensory-engine/src/index.ts crossing ~417 lines as a flat barrel is a maintainability smell independent of this PR, but this diff adds to it — consider whether new export groups should get their own re-exported sub-barrel going forward.
  • File or link a follow-up issue for collapsing src/selfhost/ai.ts's SUBSCRIPTION_CLI_ENV_ALLOWLIST/SECRET_PATTERNS onto the new engine copy, per the PR's own comment at src/selfhost/ai.ts, so 'keep in sync' doesn't quietly bit-rot.
  • Consider adding an inline scanner-suppression comment near the AKIAIOSFODNN7EXAMPLE test fixtures noting it's AWS's canonical example key, to preempt future secret-scan false positives on this exact line.

Why this is blocked

  • Possible leaked secret in the diff (aws_access_key, generic_secret_assignment, github_pat, github_token) — Remove the secret from the diff, rotate the exposed credential, then re-run the gate.
Signal Result Evidence
Code review ❌ 1 blocker 1 reviewer
Linked issue ✅ Linked #4284
Related work ✅ No active overlap found No same-issue or scoped active PR overlap found.
Change scope ✅ 20/20 Low review scope from cached public metadata (1 linked issue).
Validation posture ✅ 25/25 PR body includes validation/test evidence.
Contributor workload ✅ 10/10 Author activity: 158 registered-repo PR(s), 100 merged, 11 issue(s).
Contributor context ✅ Confirmed Gittensor contributor nickmopen; Gittensor profile; 158 PR(s), 11 issue(s).
Gate result ❌ Blocking Repo-configured hard blocker found.
Linked issue satisfaction

Addressed
The PR adds packages/gittensory-engine/src/subprocess-env.ts with a parameterized buildAllowlistedEnv, ported SECRET_PATTERNS, and redactSecrets, exports them from the barrel, documents a deliberate parallel-copy migration story for src/selfhost/ai.ts with a cross-reference comment (mirroring the predicted-gate.ts shim precedent), and adds tests in both the engine and app suites covering a caller-

Review context
  • Author: nickmopen
  • Role context: outside_contributor
  • Public audience mode: oss maintainer
  • Lane context: Repository is configured for direct PR review.
  • Public profile languages: TypeScript, Python
  • Official Gittensor activity: 158 PR(s), 11 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Keep the PR focused and include validation evidence before maintainer review.
Signal definitions
  • Related work = same linked issue, overlapping active PRs, or title/path similarity.
  • Change scope = cached public metadata such as size labels, draft state, and review-burden hints.
  • Validation posture = whether the PR provides enough public validation/test evidence for maintainer review.
  • Contributor workload = public contributor activity and cleanup pressure, not a repo-wide quality failure.
  • Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.

🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed


💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →.

Checked by Gittensory, a quiet PR intelligence layer for OSS maintainers.

  • Re-run Gittensory review

@loopover-orb

loopover-orb Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Gittensory is closing this pull request on the maintainer's behalf (Possible leaked secret in the diff (aws_access_key, generic_secret_assignment, github_pat, github_token)). This is an automated maintenance action — to pursue this change, please open a new pull request with the issues resolved. Closed PRs may be analyzed later to improve review accuracy, but they are not automatically reopened or re-reviewed.

@loopover-orb loopover-orb Bot closed this Jul 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gittensor:feature Gittensor-scored feature linked to a feature issue — scores a 0.25x multiplier.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(miner-hands): shared subprocess redaction/env-allowlist helper in gittensory-engine

1 participant