Commit Graph
28 Commits
Author SHA1 Message Date
Affaan MustafaandGitHub fc9273e5e0 Merge pull request #3126 from VarunGore36/fix/reviewer-followups
fix(security): security follow-ups for worker, installer, claw, hooks
2026-09-19 19:58:43 -04:00
Juan Garibay 4869db30c4 fix(hooks): match the index case-insensitively, and isolate the pytest probe
Red-teaming the guard from 3c317470 found two more ways to get a repository's own
code executed. Both are demonstrated by a planted binary that appends to a witness
file, counted before and after.

Case folding. git matches index pathspecs case-sensitively even where
core.ignorecase is set, but APFS does not -- so a repository that commits
`.venv/bin/Python` gets `$venv/bin/python` opening and running that file while the
guard's lowercase query finds nothing in the index and reports it untracked. The
witness logged two invocations. It applies to `venv` and `env` as well, and to any
folding of the name. The query now uses a `:(icase)` pathspec; all nine
directory-by-spelling combinations are refused, and an untracked venv still runs.

Module shadowing. `python -c "import pytest"` puts the working directory first on
sys.path, so a repository that commits a `pytest.py` in its root has that file
imported, and executed, by a check whose only job is to answer whether pytest is
installed. The probe is now `python -I -c "import pytest"` on the virtualenv, uv
and poetry paths alike. Isolation does not hide a real pytest -- it lives in the
interpreter's own site-packages, confirmed against a venv holding pytest 9.1.1.

Still true, and not something this hook can fix: running the repository's declared
suite runs the repository's code. `pytest` imports conftest.py, and the Node arm
runs package.json scripts. That is what a pre-push verification hook is for. The
line this guard draws is narrower and worth keeping -- a capability probe, and the
choice of which interpreter to trust, should not be things the pushed repository
gets to decide.
2026-09-17 17:17:19 -04:00
Juan Garibay 3c31747016 fix(hooks): resolve the venv path before asking git whether it is tracked
The guard added in 9cdc40e6 was incomplete. `git ls-files` reports paths as they
are indexed and does not follow symlinks, so a repository that commits `.venv` as
a symlink to its own root alongside a tracked `bin/python` gets asked about
`.venv/bin/python` -- a path git has never heard of -- and the answer is
"untracked". The interpreter then runs. Measured on that shape: the planted
executable logged two invocations against 9cdc40e6 and none against this commit.

`repo_ships_interpreter` now resolves the bin directory with `cd -P`/`pwd -P`,
resolves the worktree root the same way, and asks git about the resolved path
relative to it. The three cases that matter all hold: a plainly committed venv is
still refused, the symlink shape is now refused, and a developer's own untracked
venv still resolves and runs.

`cd -P`/`pwd -P` rather than `realpath` or `readlink -f`, because neither is
portable to a stock macOS.
2026-09-17 16:55:11 -04:00
Juan Garibay 9cdc40e6d1 fix(hooks): do not run a virtualenv interpreter the repository ships
This branch taught the hook to run `.venv/bin/python`, and that is a binary the
repository can supply. On main the Python arm only ever ran `pytest` from PATH --
the developer's own -- and on a machine without one it ran nothing at all, which
is exactly the machine this branch was written for. So the exposure is new, and
it arrived with the fix.

The hook is installed globally through core.hooksPath. Cloning a hostile
repository, committing nothing, and pushing it to your own fork is enough: the
pre-push hook finds the committed `.venv/bin/python`, runs it once to probe for
pytest and again to run the suite. Reproduced -- the planted executable logged
two invocations under the previous commit and none under this one.

A virtualenv is never committed. It is platform-specific binaries and every
Python project gitignores it, so `git ls-files --error-unmatch` separates the
two cases exactly: a developer's own venv is untracked and still resolves, a
tracked one is skipped with the reason printed. An absolute $VIRTUAL_ENV outside
the worktree reads as untracked, as it should.

Not addressed here, and worth a maintainer's view: `uv run` and `poetry run`
resolve from the repository's own lockfile, so they carry the same shape of
trust in a form this check cannot see. They are gated behind a lockfile being
present, and changing their semantics is a larger decision than this fix.
2026-09-17 16:44:33 -04:00
Juan Garibay 08b173f12f fix(hooks): a blank ECC_PYTEST_CMD is an override, and say when one is in use
`[[ -n "${ECC_PYTEST_CMD:-}" ]]` asked whether the variable had a value, not
whether it was set, so `ECC_PYTEST_CMD=` fell through to virtualenv discovery
while `ECC_PYTEST_CMD="   "` failed the push. Two spellings of the same mistake,
two behaviours. Falling through is the wrong one: an override that evaluated to
nothing -- a command substitution that found no pytest, say -- then silently ran
a different runner than the operator named, which is exactly the substitution
this resolver refuses to make anywhere else. Both now fail closed.

`${ECC_PYTEST_CMD+set}` rather than `[[ -v ECC_PYTEST_CMD ]]`, because `-v` is
bash 4.2 and a stock macOS /bin/bash is 3.2, where it is not a false but a
syntax error. The hook runs under whatever `env bash` resolves to.

The override is still not probed -- probing runs the operator's command, and a
wrapper that ignores `--version` executes the whole suite and is then rejected
for not printing a version. What the gate can honestly do about a stale override
is refuse to be quiet about it, so a push that uses one now says so, every time,
and says the hook has not checked that it is pytest. A bypass that announces
itself is not the silent gate this resolver exists to prevent.

The fixture env is built from nothing instead of inheriting process.env with two
keys blanked. Blanking is no longer neutral: a blanked ECC_PYTEST_CMD is now an
override, and every one of these tests would have taken that branch.
2026-09-17 16:33:37 -04:00
Juan Garibay c6195edb2f fix(hooks): stop probing the pytest override, and stop failing on exit 5
Three defects, found by reviewing this branch against a running pytest rather
than by reading it.

Exit 5 is not a failure. pytest reserves it for NO_TESTS_COLLECTED, and
`|| fail "pytest failed"` collapsed it into a blocked push. The `|| fail`
predates this branch, but this branch is what makes it reachable: a repository
whose pyproject.toml only configures ruff or black, with pytest in its venv and
no test files, used to hit the "pytest is not installed" skip and now gets
gated. $VIRTUAL_ENV is the first candidate, so merely having a venv activated in
the pushing shell drags any requirements.txt repository into this path, and the
hook is installed globally. Reproduced with pytest 9.1.1. Exit 5 is now
non-blocking but loud -- a bad rootdir, testpaths or an unimportable conftest
also collects nothing, and swallowing that silently would reopen the hole this
resolver exists to close. Other non-zero codes now carry the code, because 1
(tests failed) and 4 (usage error) call for different responses.

The ECC_PYTEST_CMD probe ran the operator's command. Validating the override
with `--version` assumed it would answer like pytest. A wrapper that sets an
environment variable and execs pytest ignores the flag and runs the whole suite,
so the probe executed the tests, then rejected the command for not printing a
version, then blocked the push -- with the suite green. That is worse than the
silent gate the probe was added to close, so the override is taken as given
again: it is a deliberate setting, the hook cannot inspect it without running
it, and pointing it at something that is not pytest is the operator's call.
`is_pytest` still guards the PATH candidate, which this script composes itself,
where `pytest --version` is harmless. An empty override still fails closed.

The tests inherited the ambient environment. `runHermeticPythonPrePush` passed
process.env through, so an exported ECC_PYTEST_CMD or an activated virtualenv
resolved a pytest the fixture never created and the venv test failed for anyone
who runs the suite that way. Both variables are now neutralised in the base env.

Coverage: the gate had no test proving it blocks. Changing the run line to
`|| true` left all three previous tests green. Seven now cover a spaced venv
path, a red suite, exit 5, an override invoked exactly once with no probe, an
empty override, and the PATH candidate in both directions.
2026-09-17 16:19:57 -04:00
Juan Garibay 1f5cd2af73 test(hooks): build the pre-push python fixture env without mutation
AGENTS.md makes immutability mandatory and the helper built `env` by assigning
into it. Rather than reassigning a `let` through spreads, the two stub paths are
now resolved before the object exists, so `env` is a single `const` built in one
expression with the conditional keys spread in. Nothing to mutate and nothing to
rebind.
2026-09-17 16:04:41 -04:00
Juan Garibay 5cbe78c22b fix(hooks): keep venv paths intact and check every pytest candidate
Two holes in the resolver this branch added, both found in review.

A virtualenv path may contain spaces. `resolve_pytest` returned one string and
the caller expanded it unquoted, so `/home/me/my env/bin/python -m pytest` split
into `/home/me/my` and `env/bin/python`. The probe that accepted the candidate
was correctly quoted, so the hook reported the venv as usable and then failed to
run anything in it -- rejecting the push for a reason with nothing to do with
the code being pushed. It now builds an argv array and runs `"${PYTEST_CMD[@]}"`.

The resolver's contract is that every candidate is confirmed to be pytest, and
two of them were not. `ECC_PYTEST_CMD` was returned unchecked, so
`ECC_PYTEST_CMD=true` made the hook run `true -q`, exit 0 and report a Python
project verified by nothing. The PATH branch used `command -v pytest`, which
proves only that a file of that name exists. Both now go through `is_pytest`,
which runs `--version` and requires the output to name pytest -- `--version`
alone is not evidence, since `true --version` also exits 0.

A bad `ECC_PYTEST_CMD` fails the push rather than falling through to the next
candidate. An operator who set it asked for that command, and silently running
a different one hides the misconfiguration -- which is the same silent-gate
failure this branch exists to remove, one level along.

Three regression tests cover the three paths: a venv whose directory name
contains a space, an override that is not pytest, and an override that is.
2026-09-17 15:57:09 -04:00
Geronimo c5bbee3cb8 fix(worker,installer,claw): correct approval flag, installer order, percent path hardening
- orchestrate-codex-worker: pass approval policy via --ask-for-approval,
  not -p profile
- codex global hooks: validate conflicting global hooksPath before backup/copy,
  so refused install is side-effect free
- claw: reject percent-delimited Windows paths in cmd.exe fallback to avoid
  %NAME% expansion
- codex-hooks: opt into ECC_PREPUSH_RUN_CHECKS=1 in existing verification
  fixtures and add default-skip coverage
2026-09-14 23:52:01 +05:30
haelyra 1bdda4bdac fix: close validator and path edge cases 2026-08-29 15:36:59 -04:00
haelyra 703163275d test: honor per-invocation Bash overrides 2026-08-29 15:11:38 -04:00
haelyra 4c7e965209 fix(gan): distinguish scores from verdict thresholds 2026-08-29 14:24:16 -04:00
benno0oandhaelyra 7c2bc54be2 test: verify Corepack uses the pinned pnpm version 2026-08-29 13:40:56 -04:00
benno0oandhaelyra b48f22f08c fix: address pre-push pnpm review findings 2026-08-29 13:40:53 -04:00
benno0oandhaelyra 03b441792e fix: resolve pnpm in Git Bash pre-push hook 2026-08-29 13:40:53 -04:00
mehmet turacandGitHub 683d291aa3 fix: add plugin cache health check (#2249)
* fix: add plugin cache health check

* fix: harden plugin cache diagnostics

* fix: reject escaping plugin cache refs

* test: remove unused plugin cache fixture
2026-06-15 14:01:25 -04:00
Affaan MustafaandGitHub 6319c7d309 fix: stability batch — hook stdin truncation, Codex exa TOML, Stop hook JSON, GateGuard repetition (#2227)
* fix(hooks): fail open on oversized stdin instead of echoing truncated JSON (#2222)

run-with-flags.js capped stdin at 1MB but every fallthrough path still
echoed the truncated string to stdout. The harness parses hook stdout as
JSON, got a document cut mid-stream, and blocked the tool call — so any
Edit/Write with a >1MB hook payload was permanently blocked by every
registered pre-write hook, before ECC_HOOK_PROFILE / ECC_DISABLED_HOOKS
gating could run.

- Exit 0 with empty stdout (no opinion) when the stdin cap trips, before
  any echo or gating logic.
- Flush stdout via write callback before process.exit: exiting right
  after stdout.write() dropped everything past the ~64KB pipe buffer,
  cutting even sub-cap pass-through payloads mid-JSON.

Regression tests cover the enabled, disabled, and missing-arg paths for
oversized payloads plus full echo of sub-cap >64KB payloads.

* fix(codex): stop emitting invalid exa url entry, align merge with connector policy (#2224)

The Codex MCP merge declared exa with a url key, but Codex's
[mcp_servers.*] TOML schema is stdio-only — the url key makes the
entire config.toml fail to load, bricking both the codex CLI and the
desktop app. Every install/update re-injected the line because the
urlEntry branch treated the broken entry as present.

- ECC_SERVERS now emits only the current default set per
  docs/MCP-CONNECTOR-POLICY.md: chrome-devtools (stdio, command/args).
  Retired servers (supabase, playwright, context7, exa, github, memory,
  sequential-thinking) are never re-emitted; existing user-managed
  entries are untouched.
- The merge now repairs the exact ECC-emitted broken form (url-only
  exa entry) on every run so re-running the installer fixes broken
  configs instead of preserving them. User stdio exa entries
  (command + mcp-remote) are left alone.
- check-codex-global-state.sh requires chrome-devtools instead of the
  retired set, and flags url-only exa entries with a repair hint.

Tests cover repair, re-run idempotence, stdio-entry preservation, and
no-retired-server emission in add, update, dry-run, and disabled modes.

* fix(hooks): never echo truncated stdin from Stop hooks (#2090)

Stop hooks follow the ECC pass-through convention (echo stdin on
stdout), but every echoing Stop hook capped stdin and echoed the capped
string. The Stop payload carries last_assistant_message, so a long
final assistant message produced a JSON document cut mid-stream on
stdout, which the harness reports as 'Stop hook error: JSON validation
failed' across the whole Stop chain.

Reproduced: a Stop payload with a >64KB last_assistant_message run
through run-with-flags + cost-tracker emitted exactly 65536 bytes of
invalid JSON (cost-tracker capped stdin at 64KB — far below realistic
Stop payloads).

- cost-tracker: raise the cap to 1MB (matching all other hooks) and
  suppress the pass-through echo when stdin was truncated.
- check-console-log, stop-format-typecheck, desktop-notify: suppress
  the echo when stdin was truncated; flush stdout before process.exit
  so sub-cap payloads are not cut at the ~64KB pipe buffer.
- All hooks keep exiting 0 (fail-open); diagnostics go to stderr.

New stop-hooks-stdout test asserts the contract for every registered
Stop hook: stdout is empty or valid JSON, exit code 0 — for realistic
100KB payloads and oversized >1MB payloads, via the production runner
and via direct invocation. Updated the old hooks.test.js case that
codified the truncated-echo behavior.

* fix(hooks): dampen GateGuard fact-force repetition in long sessions (#2142)

In long autonomous sessions the fact-force gate produced 10+
near-identical 'state facts -> blocked -> restate -> retry' blocks in
one context window, which measurably raises the odds of the model
collapsing into a degenerate single-token repetition loop.

- Track a per-session fact_force_denials counter in GateGuard state
  (merged max across concurrent writers, reset with the session, robust
  to malformed on-disk values).
- The first GATEGUARD_FACT_FORCE_FULL_DENIALS denials (default 3) keep
  the full four-fact block; later denials emit a condensed single-line
  message that carries the denial ordinal, so consecutive denials are
  structurally different and never textually identical.
- True retries of the same target remain allowed without re-prompting
  (unchanged). Destructive-Bash and routine-Bash gates are unchanged,
  as are the ECC_GATEGUARD=off / ECC_DISABLED_HOOKS escape hatches.

Eight new tests cover budget counting, condensed format, ordinal
advancement, retry pass-through, env tuning, malformed state, MultiEdit
dampening, and destructive-gate exemption.

* fix(hooks): keep security hooks able to block on oversized stdin (#2222)

Refine the truncation fail-open: instead of skipping the hook entirely,
the runner now suppresses only its own raw-echo when stdin was
truncated. The hook still executes and receives the truncated flag
(run() context / ECC_HOOK_INPUT_TRUNCATED), so config-protection keeps
blocking truncated protected-config payloads (its test requires exit 2)
while pass-through hooks fail open with empty stdout as before.

* style: apply repo formatter to touched hook files
2026-06-11 00:31:33 -04:00
Affaan Mustafa ae02b26cf9 test: cover mcp config merge edges 2026-04-29 18:57:55 -04:00
Affaan Mustafa cc89c40751 test: cover codex config merge edges 2026-04-29 18:51:56 -04:00
Affaan Mustafa 9a6080f2e1 feat: sync the codex baseline and agent roles 2026-04-01 16:08:03 -07:00
Affaan MustafaandGitHub 6cc85ef2ed fix: CI fixes, security audit, remotion skill, lead-intelligence, npm audit (#1039)
* fix(ci): resolve cross-platform test failures

- Sanity check script (check-codex-global-state.sh) now falls back to
  grep -E when ripgrep is not available, fixing the codex-hooks sync
  test on all CI platforms. Patterns converted to POSIX ERE for
  portability.
- Unicode safety test accepts both / and \ path separators so the
  executable-file assertion passes on Windows.
- Gacha test sets PYTHONUTF8=1 so Python uses UTF-8 stdout encoding on
  Windows instead of cp1252, preventing UnicodeEncodeError on box-drawing
  characters.
- Quoted-hook-path test skipped on Windows where NTFS disallows
  double-quote characters in filenames.

* feat: port remotion-video-creation skill (29 rules), restore missing files

New skill:
- remotion-video-creation: 29 domain-specific Remotion rules covering 3D/Three.js,
  animations, audio, captions, charts, compositions, fonts, GIFs, Lottie,
  measuring, sequencing, tailwind, text animations, timing, transitions,
  trimming, and video embedding. Ported from personal skills.

Restored:
- autonomous-agent-harness/SKILL.md (was in commit but missing from worktree)
- lead-intelligence/ (full directory restored from branch commit)

Updated:
- manifests/install-modules.json: added remotion-video-creation to media-generation
- README.md + AGENTS.md: synced counts to 139 skills

Catalog validates: 30 agents, 60 commands, 139 skills.

* fix(security): pin MCP server versions, add dependabot, pin github-script SHA

Critical:
- Pin all npx -y MCP server packages to specific versions in .mcp.json
  to prevent supply chain attacks via version hijacking:
  - @modelcontextprotocol/server-github@2025.4.8
  - @modelcontextprotocol/server-memory@2026.1.26
  - @modelcontextprotocol/server-sequential-thinking@2025.12.18
  - @playwright/mcp@0.0.69 (was 0.0.68)

Medium:
- Add .github/dependabot.yml for weekly npm + github-actions updates
  with grouped minor/patch PRs
- Pin actions/github-script to SHA (was @v7 tag, now pinned to commit)

* feat: add social-graph-ranker skill — weighted network proximity scoring

New skill: social-graph-ranker
- Weighted social graph traversal with exponential decay across hops
- Bridge Score: B(m) = Σ w(t) · λ^(d(m,t)-1) ranks mutuals by target proximity
- Extended Score incorporates 2nd-order network (mutual-of-mutual connections)
- Final ranking includes engagement bonus for responsive connections
- Runs in parallel with lead-intelligence skill for combined warm+cold outreach
- Supports X API + LinkedIn CSV for graph harvesting
- Outputs tiered action list: warm intros, direct outreach, network gap analysis

Added to business-content install module. Catalog validates: 30/60/140.

* fix(security): npm audit fix — resolve all dependency vulnerabilities

Applied npm audit fix --force to resolve:
- minimatch ReDoS (3 vulnerabilities, HIGH)
- smol-toml DoS (MODERATE)
- brace-expansion memory exhaustion (MODERATE)
- markdownlint-cli upgraded from 0.47.0 to 0.48.0

npm audit now reports 0 vulnerabilities.

* fix: resolve markdown lint and yarn lockfile sync

- MD047: ensure single trailing newline on all remotion rule files
- MD012: remove consecutive blank lines in lottie, measuring-dom-nodes, trimming
- MD034: wrap bare URLs in angle brackets (tailwind, transcribe-captions)
- yarn.lock: regenerated to sync with npm audit changes in package.json

* fix: replace unicode arrows in lead-intelligence (CI unicode safety check)
2026-03-31 15:08:55 -04:00
Affaan MustafaandGitHub e68233cd5d fix(ci): harden codex hook regression test (#1028) 2026-03-30 14:21:40 -04:00
Affaan MustafaandGitHub 7253d0ca98 test: isolate codex hook sync env (#1023) 2026-03-30 04:31:09 -04:00
Affaan MustafaandGitHub 8846210ca2 fix: unblock unicode safety CI lint (#1017)
* fix: unblock unicode safety CI lint

* fix: unblock shared CI regressions
2026-03-30 01:50:17 -04:00
Affaan Mustafa 0ebcfc368e fix(codex): broaden context7 config checks 2026-03-29 00:26:16 -04:00
Affaan Mustafa b19b4c6b5e fix: finish blocker lane hook and install regressions 2026-03-25 04:00:50 -04:00
Affaan Mustafa 9c5ca92e6e fix: finish hook fallback and canonical session follow-ups 2026-03-25 03:44:03 -04:00
Affaan Mustafa 1d0aa5ac2a fix: fold session manager blockers into one candidate 2026-03-24 23:08:27 -04:00