Skip to content

Daily review findings: 2026-06-09 #394

@garethpaul

Description

@garethpaul

I reviewed the current main branch and found 5 issues worth triage. I checked these against open and closed GitHub issues before filing.

1. GitHub comment automations let prior contributors prompt write-token agents

Area: @poe-code/github-workflows
Severity: high
Confidence: high
Score: 85

What I found:
The built-in comment-triggered GitHub automations trust the CONTRIBUTOR author association by default. That is broader than collaborator trust: a user can be a prior contributor without having current write access to the repository. For issue_comment events, the workflow runs in the base repository context, so secrets and the GitHub App token are available once the guard passes.

The guard only checks that COMMENT_AUTHOR_ASSOCIATION is listed in the automation frontmatter, and the built-in issue-comment and PR-comment prompts include CONTRIBUTOR. The same workflows then launch the agent with POE_API_KEY, GITHUB_TOKEN, and write permissions for issues and PRs, and the prompt includes the untrusted comment body directly.

Snapshot recheck at 2026-06-09T06:58:05Z: current origin/main is 1b1ea5e; checked-out HEAD has no diff from origin for this finding. The cited files still show CONTRIBUTOR in the default prompt frontmatter, the generated workflow still forwards github.event.comment.author_association into the guard, and check-user-allow still performs a plain allow-list membership check. Live GitHub issue search found no exact-title duplicate; closed #228 is related but does not cover the unsafe built-in default that remains in this snapshot.

Evidence:

  • packages/github-workflows/src/prompts/github-issue-comment-created.md:3-12 and packages/github-workflows/src/prompts/github-pull-request-comment-created.md:3-12 include OWNER, MEMBER, COLLABORATOR, and CONTRIBUTOR in the built-in allow frontmatter and accept prefixes such as poe-code.
  • .github/workflows/poe-code-github-issue-comment-created.yml:43-58 and .github/workflows/poe-code-github-pull-request-comment-created.yml:46-61 pass github.event.comment.author_association into require-user-allow, so the guard accepts any commenter classified as CONTRIBUTOR.
  • packages/github-workflows/src/exec/check-user-allow.ts:4-22 only checks membership in the frontmatter allow list; it does not distinguish collaborators from prior external contributors or require maintainer approval for comment-triggered runs.
  • .github/workflows/poe-code-github-issue-comment-created.yml:68-174 and .github/workflows/poe-code-github-pull-request-comment-created.yml:74-181 run the agent job in the base repository context with contents: write, issues: write, and pull-requests: write, create a GitHub App token, and set POE_API_KEY plus GITHUB_TOKEN for the agent command.
  • packages/github-workflows/src/prompts/github-issue-comment-created.md:13 and packages/github-workflows/src/prompts/github-pull-request-comment-created.md:13 render the commenter-supplied body into the prompt via {{comment.body}}.
  • packages/github-workflows/src/commands.ts:128-143 and 185-198 render automation prompts and spawn the configured agent from the current workflow process. The workflow step environment contains the Poe API key and GitHub token.
  • packages/github-workflows/src/variables.yaml:9-15 explicitly instructs the agent to use the gh CLI for GitHub operations, which makes the write token part of the intended automation path.
  • .github/workflows/poe-code-github-issue-opened.yml:9-13 is separately label-gated, so this finding is focused on the two issue_comment workflows where a comment author can directly trigger the run.

Suggested next step:
Change the built-in comment-triggered automation defaults to allow only OWNER, MEMBER, and COLLABORATOR, or require an explicit maintainer approval/label gate before a CONTRIBUTOR comment can run with write tokens.

Add workflow/frontmatter tests proving CONTRIBUTOR comments fail by default while collaborator comments still pass. After regenerating workflows, validate with npm run lint:workflows.

Validation:
Validate the fix by narrowing the built-in comment-triggered automation defaults to OWNER, MEMBER, and COLLABORATOR, or by adding an explicit maintainer approval gate before CONTRIBUTOR-authored comments can run with write tokens. Add workflow/frontmatter tests proving CONTRIBUTOR comments fail by default while collaborator comments still pass, then regenerate workflows and run npm run lint:workflows.

2. Host-runtime spawn timeouts can report cancellation while the child process keeps running

Area: @poe-code/process-runner
Severity: high
Confidence: high
Score: 84

What I found:
The default host runtime cancellation path is best-effort rather than bounded. runPoeCommand treats an abort or inactivity timeout as complete as soon as it calls handle.kill("SIGTERM"); waitForHandle then throws the abort or timeout error immediately instead of waiting for handle.result to settle. For the default host runner, that kill is child.kill("SIGTERM"), not a process-group kill and not followed by SIGKILL escalation.

That means a host agent process with a signal handler, stuck shutdown path, or child process that keeps doing work can outlive the Poe Code run. The caller sees AbortError or ActivityTimeoutError, job state can move on, streams are unpiped, and local cleanup may finish while the process is still active.

Snapshot recheck at 2026-06-09T06:58:05Z: current origin/main is 1b1ea5e; checked-out HEAD has no diff from origin for this finding. The host runner still sends a single SIGTERM on abort, and the agent-harness timeout path still returns on the termination race without waiting for host process exit unless a caller supplied forceKillAfterMs. Live GitHub issue search found no exact-title duplicate.

Evidence:

  • packages/process-runner/README.md:11-16 documents the host factory as the shared local process runner and notes process-group kill is only tied to the detached option.
  • packages/agent-spawn/src/register-factories.ts:1-9 registers hostExecutionEnvFactory from @poe-code/process-runner for agent-spawn runtime execution.
  • packages/agent-spawn/src/spawn.ts:251-280 routes CLI spawns through runPoeCommand with wrapForLogTee: false, captureOutput: true, activityTimeoutMs, and the caller AbortSignal.
  • packages/process-runner/src/host/host-runner.ts:40-47 sends signals to the process group only when createHostRunner({ detached: true }) was used; the default host runner calls child.kill(signal) on just the direct child.
  • packages/process-runner/src/host/host-runner.ts:55-61 handles AbortSignal by sending only SIGTERM, with no wait, SIGKILL escalation, or process-tree cleanup.
  • packages/agent-harness-tools/src/run-poe-command.ts:408-430 wires abort/activity-timeout handling into unwrapped runs through createAbortSync(...) and waitForHandle().
  • packages/agent-harness-tools/src/run-poe-command.ts:587-617 implements abort/activity-timeout termination by calling handle.kill("SIGTERM") once, with SIGKILL escalation only when the wrapped-command option supplies forceKillAfterMs.
  • packages/agent-harness-tools/src/run-poe-command.ts:661-674 races handle.result against terminationPromise and throws the timeout/abort error immediately when termination fires, without waiting for the child to exit after SIGTERM.
  • packages/agent-harness-tools/src/run-poe-command.test.ts:1090-1123 asserts a timeout rejects promptly and only checks that kill() was called; it uses a handle.result that never resolves and does not model a child that ignores SIGTERM or verify eventual process termination.
  • packages/process-runner/src/host/host-runner.test.ts:70-94 covers cooperative kill/abort behavior with sleep, but there is no nearby test for an uncooperative child or SIGKILL escalation.

Suggested next step:
Make host-runtime termination deterministic for abort and activity-timeout paths. A pragmatic fix is to terminate a process group for host runs or add a bounded graceful shutdown: send SIGTERM, wait briefly for handle.result, then escalate to SIGKILL on the process or process group. Add focused coverage with a child that ignores SIGTERM and assert the public timeout does not return while that child is still alive.

Validation:
Make host-runtime termination deterministic for abort and activity-timeout paths. A pragmatic validation path is to add a child fixture that ignores SIGTERM, assert the public timeout does not return while that child is still alive, then implement bounded graceful shutdown: send SIGTERM, wait briefly for handle.result, and escalate to SIGKILL on the process or process group. Run npm run test:unit -- --run packages/process-runner/src/host/host-runner.test.ts packages/agent-harness-tools/src/run-poe-command.test.ts packages/agent-spawn/src/agent-spawn.test.ts.

3. GitHub workflow caller templates inherit every caller secret into upstream reusable workflows

Area: @poe-code/github-workflows
Severity: critical
Confidence: high
Score: 83

What I found:
The default installed caller workflows pass a much larger secret set than the reusable workflows need.

All eight caller templates call PoeCode's upstream gh-* reusable workflows with secrets: inherit. GitHub documents that this passes all secrets available to the caller job to the directly called workflow, and inherited secrets can be referenced even if the called workflow did not declare them.

The reusable workflows only use POE_CODE_AGENT_APP_ID, POE_CODE_AGENT_PRIVATE_KEY, and POE_API_KEY. They create their own GitHub App token in the job. So a caller-mode install exposes unrelated repository or organization secrets to the called workflow's execution context, including comment-triggered automations.

Recheck at 2026-06-09T06:58:05Z: current origin/main is 1b1ea5e; checked-out HEAD has no diff from origin for this finding. All eight caller templates still use secrets: inherit, and live GitHub issue search found no exact-title duplicate.

Evidence:

  • packages/github-workflows/src/workflow-templates/github-issue-comment-created.caller.yml:12-13 - The issue-comment caller invokes the upstream reusable workflow at @main and passes secrets: inherit.
  • packages/github-workflows/src/workflow-templates/github-pull-request-comment-created.caller.yml:15-16 - The PR-comment caller uses the same secrets: inherit pattern for comment-triggered automation.
  • packages/github-workflows/src/workflow-templates/fix-vulnerabilities.caller.yml:15-16 - Scheduled or manually-triggered workflow callers use the same broad inherited secret boundary.
  • local repro:2026-06-09T06:25Z - A repository scan found secrets: inherit in all eight caller templates: fix-vulnerabilities, github-issue-comment-created, github-issue-opened, github-pull-request-comment-created, github-pull-request-opened, github-pull-request-synchronized, update-dependencies, and update-documentation.
  • .github/workflows/gh-github-issue-comment-created.yml:50-52,108-110,196-201 - The called reusable workflow only consumes POE_CODE_AGENT_APP_ID, POE_CODE_AGENT_PRIVATE_KEY, and POE_API_KEY; the agent command receives a GitHub App token produced inside the reusable workflow, not an arbitrary caller secret.
  • .github/workflows/gh-update-dependencies.yml:19-21,56-61 - The scheduled update workflow has the same narrow actual secret dependency: app id, app private key, Poe API key, and a locally-created GitHub App token.
  • GitHub Docs:https://docs.github.com/actions/how-tos/sharing-automations/reusing-workflows - GitHub's reusable workflow documentation says inherited secrets can be referenced even when not declared by on.workflow_call, and that secrets: inherit passes all of the calling workflow's secrets to the directly called workflow.

Suggested next step:
Stop using secrets: inherit in generated caller templates. Declare the three required named secrets on each reusable workflow and pass only those names from the caller. Add a template regression that scans all caller templates and fails on secrets: inherit.

Validation:
Declare required workflow_call.secrets for POE_CODE_AGENT_APP_ID, POE_CODE_AGENT_PRIVATE_KEY, and POE_API_KEY in each generated reusable workflow. Regenerate templates, add a workflow-template test that fails on secrets: inherit, and verify caller templates pass only those three named secrets.

4. Auth provider registry is hand-maintained in multiple packages

Area: providers
Severity: P2
Confidence: high
Score: 82

What I found:
Auth providers are declarative once defined, but registration is still manual in several consumers. The project rule says a new provider should be one provider file with the rest derived from provider config; current code requires editing at least the provider package exports, CLI container, SDK container, and config-store default registry.

Snapshot recheck at 2026-06-09T06:58:05Z: current origin/main is 1b1ea5e; checked-out HEAD has no diff from origin for this finding. The CLI and SDK containers still create separate literal auth provider arrays, and poe-code-config still creates its own default ProviderRegistry from the same provider literals. Live GitHub issue search found no exact-title duplicate or provider-registry title match.

Why it matters:
Provider discovery is part of configuration behavior: the CLI prompts from ProviderRegistry, the SDK uses the same registry for non-interactive flows, and poe-code-config derives missing apiShape during configured-service save/migration. If a new provider is added to one surface but not the others, users can configure a provider that later cannot be migrated, logged into, or used consistently from SDK paths.

Evidence:

  • src/cli/container.ts:122 hard-codes authProviders = [poeProvider, anthropicProvider, cloudflareProvider] for the CLI ProviderRegistry.
  • src/sdk/container.ts:96 repeats the same authProviders array for the SDK ProviderRegistry.
  • packages/poe-code-config/src/configured-services.ts:72 uses a default provider registry when deriving missing apiShape, and packages/poe-code-config/src/configured-services.ts:445-449 builds that registry from the same explicit provider list.
  • packages/providers/src/index.ts:21-23 exports each concrete provider individually and does not expose a package-owned aggregate list that consumers can import.
  • src/cli/container.test.ts:43-54 and src/sdk/container.test.ts:30-36 assert the exact three-provider list, so adding a provider requires test/code churn outside a single provider file.

Suggested next step:
Move auth provider aggregation into @poe-code/providers, preferably as a derived or generated allAuthProviders export owned by the provider package. Have CLI, SDK, and poe-code-config consume that single aggregate, then update tests to assert the aggregate is wired rather than duplicating the literal provider IDs in each package.

Validation:
Move auth provider aggregation into @poe-code/providers, preferably as a derived or generated allAuthProviders export owned by the provider package. Have CLI, SDK, and poe-code-config consume that single aggregate. Validate with focused provider registry/container tests and add coverage proving a fixture provider added to the provider package becomes visible to CLI, SDK, and configured-service apiShape derivation without editing those consumers.

5. Poe environment lookups accept inherited env variables

Area: poe-code / @poe-code/poe-agent
Severity: high
Confidence: high
Score: 82

What I found:
The recent config hardening commits close inherited-property reads inside several config documents, but Poe environment lookups still trust inherited environment properties. I rechecked this at 2026-06-09T06:58:05Z against current origin/main 1b1ea5e, and the central environment lookup paths are still unchanged.

The real CLI bootstrap passes process.env into createCliEnvironment(). That helper keeps the object as-is, reads variables.POE_BASE_URL directly, and exposes getVariable(name) as variables[name]. The shared config merge path also calls collectEnvOverrides(), which reads configured env keys with env[field.env].

The same pattern exists in direct SDK and Poe-agent helpers: getPoeApiKey(), ensurePoeApiKeyEnv(), the generate SDK, and the Poe-agent OpenAI auth/base URL helpers read process.env properties normally. If a process has prototype pollution, inherited env-looking values are treated as real Poe configuration.

The local repro used an empty own env map and only polluted Object.prototype. The CLI container still resolved:

  • POE_DEFAULT_AGENT to goose
  • POE_BASE_URL to https://polluted.example/v1
  • POE_API_KEY to sk-polluted

The SDK probe also showed getPoeApiKey() returning an inherited key and the base URL resolver returning an inherited Poe base URL after the own env vars were removed.

That means default-agent selection, Poe endpoint selection, API-key lookup, and generate model/base URL selection can be changed without any own environment variable or config file entry.

Evidence:

  • src/cli/bootstrap.ts:38-46 - The real CLI bootstrap passes process.env directly as the variable map for the command container.
  • src/cli/environment.ts:30-42 - createCliEnvironment() keeps init.variables directly, resolves Poe base URLs from variables.POE_BASE_URL, and implements getVariable() as variables[name], so inherited values are accepted.
  • src/sdk/credentials.ts:35-64 - getPoeApiKey() and ensurePoeApiKeyEnv() read process.env.POE_API_KEY directly. An inherited key is returned as a credential, and ensurePoeApiKeyEnv() treats it as already exported without creating an own env entry.
  • src/sdk/generate.ts:98-120 - The generate SDK resolves Poe base URLs and model env overrides from direct process.env.POE_BASE_URL, process.env.POE_API_BASE_URL, and process.env[envKey] lookups.
  • packages/poe-agent/src/plugins/openai-auth.ts:3-12 - The Poe-agent OpenAI auth helper accepts process.env.POE_API_KEY before falling back to the auth store.
  • packages/poe-agent/src/plugins/poe-agent-plugin-openai-responses.ts:699-710 - The OpenAI Responses plugin reads process.env.POE_BASE_URL directly for its client base URL.
  • packages/poe-agent/src/plugins/poe-agent-plugin-openai-chat-completions.ts:544-555 - The OpenAI Chat Completions plugin has the same direct process.env.POE_BASE_URL fallback.
  • packages/poe-code-config/src/inspect.ts:95-108 - collectEnvOverrides() reads each configured env var with env[field.env] and materializes it into an override document.
  • src/cli/commands/shared.ts:533-543 - resolveMergedDocument() applies collectEnvOverrides(knownConfigScopes, container.env.variables) on top of file config, so inherited env keys can override global and project config for shared command helpers.
  • src/services/config.ts:42-60,112-121 - The core config scope maps POE_API_KEY, POE_DEFAULT_AGENT, and POE_BASE_URL into known config scopes consumed by the shared merge path.
  • local repro:2026-06-09T05:51Z - With an empty own variables = {} and inherited POE_DEFAULT_AGENT = 'goose', POE_BASE_URL = 'https://polluted.example/v1', and POE_API_KEY = 'sk-polluted', resolveDefaultAgent(container) returned goose, resolveMergedDocument(container).core contained all three inherited values, container.env.poeApiBaseUrl used the polluted URL, and container.env.getVariable('POE_API_KEY') returned sk-polluted.
  • local repro:2026-06-09T05:50Z - A direct Node probe showed process.env does not own POE_BASE_URL, but process.env.POE_BASE_URL still reads an inherited Object.prototype.POE_BASE_URL value.
  • local repro:2026-06-09T05:53Z - With own Poe env vars temporarily removed and inherited POE_API_KEY/POE_BASE_URL, getPoeApiKey() returned the inherited key and resolvePoeApiBaseUrl(process.env) returned the inherited base URL.

Suggested next step:
Normalize the env map at the CLI boundary: copy only own enumerable string values into a null-prototype record. Then use own-property helpers for every Poe env lookup, including getVariable(), resolvePoeBaseUrls(), collectEnvOverrides(), SDK credential/generate helpers, and Poe-agent OpenAI auth/base URL helpers. Add prototype-pollution regressions for POE_API_KEY, POE_BASE_URL, POE_DEFAULT_AGENT, model envs, and one boolean env-backed TUI flag.

Duplicate checks

  • Refreshed issue cache for poe-platform/poe-code before preparing this issue.
  • Prepared at 2026-06-09T07:01:04.536Z.
  • Checked candidate metadata for duplicate links and exact GitHub title matches.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions