Skip to content

fix executes model-generated tests with the full environment inherited, including API keys #180

Open

Description

@vinicq

Disclosed in #179 rather than fixed, on purpose: the disclosure is a docs change, real isolation is a design change with an OS dependency. Splitting them keeps #179 reviewable and gets the warning to users today.

What happens

falsegreen-skill fix asks the model for a strengthened test file and then executes that generated file to check it. bin/fix-gate.js:68:

spawnSync(py, ['-B', '-m', 'pytest', '-q', '-x', '-p', 'no:cacheprovider', ...],
          { cwd, env: Object.assign({}, process.env, { PYTHONDONTWRITEBYTECODE: '1' }) })

The temp replica bounds relative paths. It does nothing else:

  • process.env is inherited in full, so a generated test can read the user's ANTHROPIC_API_KEY, OPENAI_API_KEY, FALSEGREEN_API_KEY, and everything else in the environment. This is the part that makes it more than a filesystem question: the command is handed credentials and then runs code it did not write.
  • Absolute paths write anywhere. cwd is not a jail, and the original checkout stays reachable.
  • The network is open. Nothing prevents an outbound call from the test.

Nobody has to be malicious for this to bite. A generated test that imports a module with import-time side effects is enough.

Options, roughly in order of cost

  1. Filter the environment. Pass a minimal allowlist (PATH, PYTHONDONTWRITEBYTECODE, SystemRoot/TMPDIR as the platform needs) instead of all of process.env. Cheapest real improvement, no new dependency, and it closes the credential path specifically. Needs care so pytest still resolves the interpreter and imports on every platform.
  2. Opt-in container isolation. Run the gate inside a container with no network and only the replica mounted. Actual isolation, but it adds an OS dependency and only helps users who have it, so it has to be opt-in with a documented fallback.
  3. Require explicit consent. Refuse to execute without --yes-run-generated-code or an equivalent, so nobody executes model output by accident.

Option 1 plus option 3 is probably the honest default: strip the credentials, and make the user say yes before code they did not write runs on their machine.

Test requirement

Whatever lands, prove it by removal, not by assertion. A test must show a generated test cannot read a secret from the environment: put a canary variable in the parent process, have the replica test try to read it, and assert the gate reports it absent. A guard with no negative test is a claim.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions