Add GitHub secrets provider#718
Conversation
Greptile SummaryThis PR introduces a new
Confidence Score: 3/5The new provider is mostly well-structured but has a JSON parsing gap that can produce confusing runtime failures, and two edge-case issues in the arg-building helpers. The unguarded packages/secrets/github/src/index.ts deserves a close look — specifically the Important Files Changed
Sequence DiagramsequenceDiagram
participant Caller
participant GitHubProvider
participant exec
participant gh as gh CLI
Note over Caller,gh: connect()
Caller->>GitHubProvider: connect(ctx, config)
GitHubProvider-->>Caller: "{ accountId } (no subprocess run)"
Note over Caller,gh: pull()
Caller->>GitHubProvider: pull(ctx, config)
GitHubProvider->>exec: gh secret list --app actions --json ... [--repo/--org/--env/--user]
exec->>gh: spawn process
gh-->>exec: stdout (JSON array)
exec-->>GitHubProvider: "{ stdout }"
GitHubProvider->>GitHubProvider: parseSecretList(stdout) → JSON.parse (unguarded)
GitHubProvider-->>Caller: SecretRef[]
Note over Caller,gh: push()
Caller->>GitHubProvider: push(ctx, secrets, config)
loop for each secret
GitHubProvider->>GitHubProvider: assertSecretKey(key)
GitHubProvider->>GitHubProvider: resolve value (secret.value ?? ctx.secret(key))
GitHubProvider->>exec: gh secret set --app actions [--repo/--org/--env] KEY --body VALUE
exec->>gh: spawn process
gh-->>exec: exit code
exec-->>GitHubProvider: result
end
GitHubProvider-->>Caller: "{ count }"
Reviews (1): Last reviewed commit: "Add GitHub secrets provider" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| function parseSecretList(stdout: string): SecretRef[] { | ||
| const body = stdout.trim(); | ||
| if (!body) return []; | ||
| const entries = JSON.parse(body) as GitHubSecretListEntry[]; | ||
| return entries.map((entry) => ({ | ||
| key: entry.name, | ||
| path: [ | ||
| entry.visibility, | ||
| entry.numSelectedRepos !== undefined ? `${entry.numSelectedRepos} selected repos` : undefined, | ||
| entry.updatedAt, | ||
| ].filter(Boolean).join(' · ') || undefined, | ||
| })); | ||
| } |
There was a problem hiding this comment.
JSON.parse will surface as an opaque SyntaxError
parseSecretList calls JSON.parse(body) with no try/catch. If gh secret list --json returns a non-zero exit code that somehow slips through, emits a warning/notice on stdout before the JSON array, or produces truncated output, the caller receives a raw SyntaxError with no indication of what the gh output actually contained. Wrapping the parse in a try/catch and rethrowing with the raw stdout snippet in the message would make diagnosis far easier in production.
| function orgVisibilityArgs(config: Config): string[] { | ||
| if (!text(config.org) && !config.user) return []; | ||
| if (config.noReposSelected) return ['--no-repos-selected']; | ||
| if (config.repos?.length) return ['--repos', config.repos.join(',')]; | ||
| if (config.visibility) return ['--visibility', config.visibility]; | ||
| return []; | ||
| } |
There was a problem hiding this comment.
--no-repos-selected incorrectly allowed for user-scoped secrets
orgVisibilityArgs gates only on !org && !user, so when config.user = true and config.noReposSelected = true the function returns ['--no-repos-selected']. The GitHub CLI only accepts --no-repos-selected on org secrets; passing it on a user-scoped call will cause gh to error. A guard like if (!text(config.org)) return [] before the noReposSelected branch would prevent this invalid combination from reaching the CLI.
| async connect(ctx, config) { | ||
| const scope = text(config.repo) ?? text(config.org) ?? (config.user ? 'user' : 'current repository'); | ||
| ctx.log(`gh auth status · scope=${scope}`); | ||
| return { accountId: scope }; | ||
| }, |
There was a problem hiding this comment.
connect logs but never runs gh auth status
The log line gh auth status · scope=… implies the command is executed, but the method only constructs a string and returns without running any subprocess. Authentication failures therefore aren't surfaced at connect time — the first real error appears at pull or push, typically with a less helpful message. Running exec('gh', ['auth', 'status'], { throwOnNonZero: true }) here would give callers an early, actionable failure.
1 participant
Adds a GitHub Secrets provider under packages/secrets/github for the environment-updater work in #710. It supports listing secret metadata with gh secret list and pushing repository, environment, organization, or user scoped secrets with gh secret set without logging secret values.\n\nAlso registers the provider in the CLI adapter registry and adds tests for list parsing, repository environment pushes, and organization visibility args.\n\nChecks run:\n- pnpm --filter @profullstack/sh1pt-core build\n- pnpm vitest run packages/secrets/github/src/index.test.ts\n- pnpm --filter @profullstack/sh1pt-secrets-github typecheck\n- pnpm --filter @profullstack/sh1pt typecheck