feat(provisioner): inject GIT_ASKPASS for env-driven HTTPS git auth #1525

Merged

hongming merged 2 commits from feat/provisioner-env-git-askpass into main 2026-05-18 21:15:14 +00:00

Conversation 3 Commits 2 Files Changed 6

+374

hongming commented 2026-05-18 20:02:59 +00:00

Owner

Copy Link

Copy Source

Summary

Wire container-side git HTTPS authentication to the persona credentials that already arrive via workspace_secrets (GITEA_USER / GITEA_TOKEN, GIT_HTTP_USERNAME / GIT_HTTP_PASSWORD) without mutating ~/.gitconfig or ~/.git-credentials inside the container.

This is the platform-side anchor PR. Three companion PRs on the open-source workspace templates ship the same generic askpass script at the same install path (/usr/local/bin/molecule-askpass).

Why this exists

Today, fresh prod-team workspaces on git.moleculesai.app boot with GITEA_USER + GITEA_TOKEN in their container env (via persona env files at /etc/molecule-bootstrap/personas/<files_dir>/env → loadPersonaEnvFile → workspace_secrets) but git push https://git.moleculesai.app/... still 401s because neither git config credential.helper nor any .git-credentials is wired to those env vars.

CTO directive 2026-05-18 ("这些 不可以通过env注入吗"): use GIT_ASKPASS env-only — no file injection into container homes.

How

1. Generic GIT_ASKPASS helper at workspace/scripts/molecule-askpass. Reads GIT_HTTP_USERNAME / GIT_HTTP_PASSWORD (with GITEA_USER / GITEA_TOKEN fallback) and emits them on git's credential-prompt protocol. Hostname-free, vendor-neutral — the deployer scopes credentials to a remote by virtue of choosing when to populate the env vars. Same script body ships in the 3 external template PRs.

2. Dockerfile bakes the helper at /usr/local/bin/molecule-askpass (mode 755).

3. applyAgentGitIdentity (already the per-agent commit-identity chokepoint at workspace_provision_shared.go:134) now also sets GIT_ASKPASS=/usr/local/bin/molecule-askpass via the new applyGitAskpass(envVars) helper. Idempotent — respects pre-existing workspace_secret / env-mutator overrides. All-or-nothing on empty workspace name (symmetric with the existing identity vars).

Empirical evidence

• loadPersonaEnvFile reads /etc/molecule-bootstrap/personas/<files_dir>/env and writes the contents into workspace_secrets at org/import time; loadWorkspaceSecrets then pulls them into the container env at every provision. Verified by reading org_helpers.go:221 and org_import.go:488.
• Established personas (core-be, core-devops, etc.) HAVE the env file with GITEA_USER + GITEA_TOKEN rows.
• Gap: the new prod-team personas (agent-dev-a, agent-dev-b) currently have only token + universal-auth.env (Infisical bootstrap) — NO env file. So GITEA_TOKEN does not arrive in those containers today. That gap is out of scope for this PR; once the env file is populated (via existing persona-provisioning runbook or workspace-secret PUT), this PR's wire-up takes effect immediately with zero further code change.
• For personas that already have the env file (the established roles), this PR closes the loop end-to-end.

Tests

agent_git_identity_test.go extended:

• TestApplyAgentGitIdentity_SetsGitAskpass — happy path.
• TestApplyAgentGitIdentity_RespectsAskpassOverride — operator/plugin override is preserved.
• TestApplyAgentGitIdentity_AskpassSkippedOnEmptyName — empty-name early-return symmetric with identity vars.
• TestApplyGitAskpass_NilMapIsSafe — defensive nil-map check.

Companion PRs

Repo	Branch	Purpose
molecule-ai/molecule-ai-workspace-template-codex	feat/git-askpass-env-helper	install helper at /usr/local/bin/molecule-askpass
molecule-ai/molecule-ai-workspace-template-hermes	feat/git-askpass-env-helper	install helper at /usr/local/bin/molecule-askpass
molecule-ai/molecule-ai-workspace-template-openclaw	feat/git-askpass-env-helper	install helper at /usr/local/bin/molecule-askpass

The four PRs are independent: this platform PR setting GIT_ASKPASS to a missing helper is harmless (git emits "exec: not found" and falls through to its prior auth chain — same baseline as before), and a template image baking the helper without the platform setting GIT_ASKPASS is also harmless (the helper just sits unused).

Test plan

• CI: existing go test ./... on workspace-server/... green.
• Manual: build a workspace runtime image off this branch + a template-codex / -hermes / -openclaw image off their companion branches, provision a workspace with GITEA_USER + GITEA_TOKEN in workspace_secrets, exec git ls-remote https://git.moleculesai.app/molecule-ai/internal HEAD → expect HTTP-200, not 401.
• Backwards-compat: provision a workspace WITHOUT setting GIT_HTTP_USERNAME / GITEA_USER — git auth behaviour MUST be identical to today (helper emits empty strings, git surfaces "Authentication failed" if the host needs auth, otherwise no change).

🤖 Generated with Claude Code

## Summary Wire container-side `git` HTTPS authentication to the persona credentials that already arrive via `workspace_secrets` (`GITEA_USER` / `GITEA_TOKEN`, `GIT_HTTP_USERNAME` / `GIT_HTTP_PASSWORD`) without mutating `~/.gitconfig` or `~/.git-credentials` inside the container. This is the platform-side anchor PR. Three companion PRs on the open-source workspace templates ship the same generic askpass script at the same install path (`/usr/local/bin/molecule-askpass`). ## Why this exists Today, fresh prod-team workspaces on `git.moleculesai.app` boot with `GITEA_USER` + `GITEA_TOKEN` in their container env (via persona env files at `/etc/molecule-bootstrap/personas/<files_dir>/env` → `loadPersonaEnvFile` → `workspace_secrets`) but `git push https://git.moleculesai.app/...` still 401s because neither `git config credential.helper` nor any `.git-credentials` is wired to those env vars. CTO directive 2026-05-18 ("这些 不可以通过env注入吗"): use `GIT_ASKPASS` env-only — no file injection into container homes. ## How 1. **Generic GIT_ASKPASS helper** at `workspace/scripts/molecule-askpass`. Reads `GIT_HTTP_USERNAME` / `GIT_HTTP_PASSWORD` (with `GITEA_USER` / `GITEA_TOKEN` fallback) and emits them on git's credential-prompt protocol. Hostname-free, vendor-neutral — the deployer scopes credentials to a remote by virtue of choosing when to populate the env vars. Same script body ships in the 3 external template PRs. 2. **Dockerfile** bakes the helper at `/usr/local/bin/molecule-askpass` (mode 755). 3. **`applyAgentGitIdentity`** (already the per-agent commit-identity chokepoint at `workspace_provision_shared.go:134`) now also sets `GIT_ASKPASS=/usr/local/bin/molecule-askpass` via the new `applyGitAskpass(envVars)` helper. Idempotent — respects pre-existing `workspace_secret` / env-mutator overrides. All-or-nothing on empty workspace name (symmetric with the existing identity vars). ## Empirical evidence - `loadPersonaEnvFile` reads `/etc/molecule-bootstrap/personas/<files_dir>/env` and writes the contents into `workspace_secrets` at org/import time; `loadWorkspaceSecrets` then pulls them into the container env at every provision. Verified by reading `org_helpers.go:221` and `org_import.go:488`. - Established personas (`core-be`, `core-devops`, etc.) HAVE the `env` file with `GITEA_USER` + `GITEA_TOKEN` rows. - **Gap**: the new prod-team personas (`agent-dev-a`, `agent-dev-b`) currently have only `token` + `universal-auth.env` (Infisical bootstrap) — NO `env` file. So `GITEA_TOKEN` does not arrive in those containers today. That gap is out of scope for this PR; once the env file is populated (via existing persona-provisioning runbook or workspace-secret PUT), this PR's wire-up takes effect immediately with zero further code change. - For personas that already have the env file (the established roles), this PR closes the loop end-to-end. ## Tests `agent_git_identity_test.go` extended: - `TestApplyAgentGitIdentity_SetsGitAskpass` — happy path. - `TestApplyAgentGitIdentity_RespectsAskpassOverride` — operator/plugin override is preserved. - `TestApplyAgentGitIdentity_AskpassSkippedOnEmptyName` — empty-name early-return symmetric with identity vars. - `TestApplyGitAskpass_NilMapIsSafe` — defensive nil-map check. ## Companion PRs | Repo | Branch | Purpose | |---|---|---| | `molecule-ai/molecule-ai-workspace-template-codex` | `feat/git-askpass-env-helper` | install helper at `/usr/local/bin/molecule-askpass` | | `molecule-ai/molecule-ai-workspace-template-hermes` | `feat/git-askpass-env-helper` | install helper at `/usr/local/bin/molecule-askpass` | | `molecule-ai/molecule-ai-workspace-template-openclaw` | `feat/git-askpass-env-helper` | install helper at `/usr/local/bin/molecule-askpass` | The four PRs are independent: this platform PR setting `GIT_ASKPASS` to a missing helper is harmless (`git` emits "exec: not found" and falls through to its prior auth chain — same baseline as before), and a template image baking the helper without the platform setting `GIT_ASKPASS` is also harmless (the helper just sits unused). ## Test plan - [ ] CI: existing `go test ./...` on `workspace-server/...` green. - [ ] Manual: build a workspace runtime image off this branch + a template-codex / -hermes / -openclaw image off their companion branches, provision a workspace with `GITEA_USER` + `GITEA_TOKEN` in `workspace_secrets`, exec `git ls-remote https://git.moleculesai.app/molecule-ai/internal HEAD` → expect HTTP-200, not 401. - [ ] Backwards-compat: provision a workspace WITHOUT setting `GIT_HTTP_USERNAME` / `GITEA_USER` — git auth behaviour MUST be identical to today (helper emits empty strings, git surfaces "Authentication failed" if the host needs auth, otherwise no change). 🤖 Generated with [Claude Code](https://claude.com/claude-code)

hongming added 1 commit 2026-05-18 20:02:59 +00:00

feat(provisioner): inject GIT_ASKPASS for env-driven HTTPS git auth ... 7c0836ea69

Wire container-side `git` HTTPS authentication to the persona credentials
that already arrive via workspace_secrets (GITEA_USER / GITEA_TOKEN,
GIT_HTTP_USERNAME / GIT_HTTP_PASSWORD) without mutating ~/.gitconfig or
~/.git-credentials inside the container.

Mechanism:
  1. New generic GIT_ASKPASS helper baked into the workspace runtime
     image at /usr/local/bin/molecule-askpass. Script body is hostname-
     free and vendor-neutral — the deployer decides which remote the
     credentials apply to by virtue of populating the env vars.
  2. applyAgentGitIdentity (already the per-agent commit-identity
     chokepoint at workspace_provision_shared.go:134) now also sets
     GIT_ASKPASS=/usr/local/bin/molecule-askpass via the new
     applyGitAskpass helper. Idempotent — respects pre-existing
     workspace_secret / env-mutator overrides.

When git encounters an HTTPS auth challenge on a host with no configured
credential.helper, it invokes GIT_ASKPASS to read the username + password
from env. This is the cleanest possible wire-up: no on-disk credential
files, no hostname literals in code, fail-loud on misconfiguration.

Tests added: GIT_ASKPASS set on success, operator-override respected,
empty-name no-op symmetry, nil-map safety.

Companion PRs on the 3 open-source workspace templates ship the same
generic askpass script at scripts/git-askpass.sh → identical install
path. Image build + helper script are intentionally split so the
platform PR can ship without breaking external template builds, and vice
versa: applyGitAskpass setting a missing helper is harmless (git would
just emit "exec: not found" and fall through to whatever auth chain
existed before — same baseline as no env-only patch at all).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

hongming referenced this pull request from molecule-ai/molecule-ai-workspace-template-codex 2026-05-18 20:03:21 +00:00

feat: generic GIT_ASKPASS helper for env-driven HTTPS git auth #12

hongming referenced this pull request from molecule-ai/molecule-ai-workspace-template-hermes 2026-05-18 20:03:31 +00:00

feat: generic GIT_ASKPASS helper for env-driven HTTPS git auth #28

hongming referenced this pull request from molecule-ai/molecule-ai-workspace-template-openclaw 2026-05-18 20:03:40 +00:00

feat: generic GIT_ASKPASS helper for env-driven HTTPS git auth #24

hongming referenced this pull request 2026-05-18 20:04:50 +00:00

feat(provisioner): inject per-persona Gitea credential helper at provision time #1523

hongming-pc2 approved these changes 2026-05-18 20:10:04 +00:00

Dismissed

hongming-pc2 left a comment

Owner

Copy Link

Copy Source

Independent non-author second-eyes review (reviewer = hongming-pc2, not the author hongming).

Verified against current head 7c0836ea6912. Per-context CI: 3/29 green, 26 pending (fresh PR; checks still spinning up). No failures yet.

Read the full +134/0 diff. 4 files: agent_git_identity.go + its test, Dockerfile, new askpass script.

Design verified end-to-end:

1. applyGitAskpass(envVars) in agent_git_identity.go — uses the same setIfEmpty pattern as applyAgentGitIdentity. Idempotent: a workspace_secret or env-mutator plugin that already set GIT_ASKPASS wins. The constant gitAskpassHelperPath = "/usr/local/bin/molecule-askpass" is the same path the Dockerfile COPYs to and the same path the 3 sibling template PRs install to. Cross-confirmed.

2. workspace/scripts/molecule-askpass — 35-line POSIX /bin/sh script. Reads GIT_HTTP_USERNAME/GIT_HTTP_PASSWORD with GITEA_USER/GITEA_TOKEN as fallback pair. Pattern-matches on Username* / Password* per the git-credential-prompt protocol. Failure mode (env unset → empty username/password → git surfaces "Authentication failed") is the right loud-failure shape per the comment. No hardcoded hostnames or vendor literals anywhere in the script body — confirmed by reading the whole file. The deployer decides which remote the credentials apply to by virtue of populating those env vars.

3. workspace/Dockerfile — single COPY scripts/molecule-askpass /usr/local/bin/molecule-askpass + chmod +x. Filename molecule-askpass is the project-specific destination marker; the script body itself is generic. Comment is explicit about that distinction.

4. Cross-repo consistency: I diffed the askpass script body in this PR against the scripts/git-askpass.sh in template-codex#12, template-hermes#28, template-openclaw#24. The script body is bit-identical across all 4 (only filenames differ). That's the right invariant — there's exactly one script implementation, replicated across all images that get GIT_ASKPASS set.

Tests (agent_git_identity_test.go +47 lines, 3 new tests):

• SetsGitAskpass — pins the canonical helper path.
• RespectsAskpassOverride — verifies idempotent semantics (existing GIT_ASKPASS preserved).
• AskpassSkippedOnEmptyName — pins the early-return contract so a provisioning glitch that dropped the workspace name doesn't half-configure the container (identity vars empty but askpass wired). All-or-nothing. Nice catch — the empty-name path was already covered for identity vars; this extends the invariant to the new code path.

One discrepancy I want to flag (not blocking, just heads-up): CEO-Assistant's brief mentioned this PR also has "loadPersonaTokenFile fallback for agent-dev-a/b personas (they only have token+universal-auth.env, no env file, so existing loadPersonaEnvFile silently no-ops on them — fixed)." That code is NOT in this PR's diff — I only see the askpass-side changes. Either the loadPersonaTokenFile fallback is in a separate (companion) PR, or it landed earlier in main, or the brief was conflating two PRs. Flagging in case the unblock actually needs both PRs to be merged together for agent-dev-a/b to work end-to-end. The askpass code on its own is sound and reviewable in isolation.

Open-source-template-friendly: confirmed — the script has no git.moleculesai.app / Molecule-AI literals. The GIT_HTTP_USERNAME / GIT_HTTP_PASSWORD env-var names are generic git conventions; the GITEA_USER / GITEA_TOKEN fallback pair is the only thing that hints at the upstream deployer's choice, and even those names don't bake-in a hostname.

No regression risk: Default-only injection (preserves overrides), and the helper script itself is invoked only when git encounters an HTTPS auth challenge — workspaces with no git push activity are unaffected.

LGTM. Approving.

**Independent non-author second-eyes review (reviewer = hongming-pc2, not the author hongming).** Verified against current head `7c0836ea6912`. Per-context CI: 3/29 green, 26 pending (fresh PR; checks still spinning up). No failures yet. **Read the full +134/0 diff. 4 files: agent_git_identity.go + its test, Dockerfile, new askpass script.** **Design verified end-to-end:** 1. **`applyGitAskpass(envVars)` in `agent_git_identity.go`** — uses the same `setIfEmpty` pattern as `applyAgentGitIdentity`. Idempotent: a workspace_secret or env-mutator plugin that already set `GIT_ASKPASS` wins. The constant `gitAskpassHelperPath = "/usr/local/bin/molecule-askpass"` is the same path the Dockerfile COPYs to and the same path the 3 sibling template PRs install to. Cross-confirmed. 2. **`workspace/scripts/molecule-askpass`** — 35-line POSIX `/bin/sh` script. Reads `GIT_HTTP_USERNAME`/`GIT_HTTP_PASSWORD` with `GITEA_USER`/`GITEA_TOKEN` as fallback pair. Pattern-matches on `Username*` / `Password*` per the git-credential-prompt protocol. Failure mode (env unset → empty username/password → git surfaces "Authentication failed") is the right loud-failure shape per the comment. **No hardcoded hostnames or vendor literals anywhere in the script body** — confirmed by reading the whole file. The deployer decides which remote the credentials apply to by virtue of populating those env vars. 3. **`workspace/Dockerfile`** — single `COPY scripts/molecule-askpass /usr/local/bin/molecule-askpass + chmod +x`. Filename `molecule-askpass` is the project-specific destination marker; the script body itself is generic. Comment is explicit about that distinction. 4. **Cross-repo consistency**: I diffed the askpass script body in this PR against the `scripts/git-askpass.sh` in template-codex#12, template-hermes#28, template-openclaw#24. The script body is **bit-identical across all 4** (only filenames differ). That's the right invariant — there's exactly one script implementation, replicated across all images that get `GIT_ASKPASS` set. **Tests (`agent_git_identity_test.go` +47 lines, 3 new tests):** - `SetsGitAskpass` — pins the canonical helper path. - `RespectsAskpassOverride` — verifies idempotent semantics (existing GIT_ASKPASS preserved). - `AskpassSkippedOnEmptyName` — pins the early-return contract so a provisioning glitch that dropped the workspace name doesn't half-configure the container (identity vars empty but askpass wired). All-or-nothing. Nice catch — the empty-name path was already covered for identity vars; this extends the invariant to the new code path. **One discrepancy I want to flag (not blocking, just heads-up):** CEO-Assistant's brief mentioned this PR also has "loadPersonaTokenFile fallback for agent-dev-a/b personas (they only have token+universal-auth.env, no env file, so existing loadPersonaEnvFile silently no-ops on them — fixed)." **That code is NOT in this PR's diff** — I only see the askpass-side changes. Either the loadPersonaTokenFile fallback is in a separate (companion) PR, or it landed earlier in main, or the brief was conflating two PRs. Flagging in case the unblock actually needs both PRs to be merged together for agent-dev-a/b to work end-to-end. The askpass code on its own is sound and reviewable in isolation. **Open-source-template-friendly:** confirmed — the script has no `git.moleculesai.app` / `Molecule-AI` literals. The `GIT_HTTP_USERNAME` / `GIT_HTTP_PASSWORD` env-var names are generic git conventions; the `GITEA_USER` / `GITEA_TOKEN` fallback pair is the only thing that hints at the upstream deployer's choice, and even those names don't bake-in a hostname. **No regression risk:** Default-only injection (preserves overrides), and the helper script itself is invoked only when git encounters an HTTPS auth challenge — workspaces with no git push activity are unaffected. LGTM. Approving.

core-devops added 1 commit 2026-05-18 20:19:43 +00:00

feat(provisioner): loadPersonaTokenFile fallback for env-file-less personas ... 9dbdaf3f4e

The new prod-team personas (agent-dev-a, agent-dev-b, agent-pm) ship
only `token` + `universal-auth.env` (Infisical UA bootstrap), no `env`
file. loadPersonaEnvFile silently no-ops on them today. With this
fallback, GITEA_TOKEN/USER/EMAIL get populated from the token file
when no env file exists.

Combined with the GIT_ASKPASS injection earlier in this PR, this
makes the askpass helper functional for the new personas.

core-devops dismissed hongming-pc2's review 2026-05-18 20:19:43 +00:00

Reason:

New commits pushed, approval review dismissed automatically according to repository settings

hongming-pc2 approved these changes 2026-05-18 20:24:03 +00:00

hongming-pc2 left a comment

Owner

Copy Link

Copy Source

Independent re-review of the delta only (reviewer = hongming-pc2, not the author hongming). Per BP dismiss_stale_approvals=true, my prior #4656 is correctly dismissed.

Verified against current head 9dbdaf3f4e6f136ead0f00790607a602c2dc7602. File-count went 4 → 6 (added org_helpers.go +62 and org_persona_env_test.go +178); the original askpass-side (agent_git_identity{,_test}.go + Dockerfile + molecule-askpass) is unchanged.

End-state verified, not the ack (per feedback_verify_actual_endstate_not_ack_follow_sop — last round the "loadPersonaTokenFile is in this PR" claim turned out to be wrong, so I checked the actual file content this time):

loadPersonaTokenFile (org_helpers.go +62 lines) — correct:

• Reads $MOLECULE_PERSONA_ROOT/<role>/token (default /etc/molecule-bootstrap/personas).
• strings.TrimSpace on contents — handles the canonical printf "%s\n" "$token" > token shape from the operator-host bootstrap kit. Comment explicitly notes Gitea's PAT validator rejects embedded whitespace, which is why trim must happen.
• Synthesises GITEA_USER_EMAIL = role + "@" + gitIdentityEmailDomain — reuses the constant from agent_git_identity.go, so the email domain (agents.moleculesai.app) lives in one place. No host literal duplicated.
• setIfEmpty-style: each key only set if not already present in out. Caller's later layers + any prior parseEnvFile rows always win.
• isSafeRoleName gate before any filesystem touch — same defense as loadPersonaEnvFile. Defense-in-depth.
• Nil-map early return.

loadPersonaEnvFile integration is the right shape:

• Snapshots before := len(out) BEFORE parseEnvFile, then checks if len(out) == before to decide whether to invoke the fallback. This correctly handles all three "no env file rows" cases (file absent / file present-but-empty / file present-but-only-comments) the same way.
• Env-file rows still win when present — the fallback doesn't even fire if any rows landed. Pinned by TestLoadPersonaTokenFile_EnvFileWins.

Test coverage (org_persona_env_test.go +178 lines — 7 new tests, not 8 as the brief said, but all 4 cases I flagged are covered + 3 bonus defense-in-depth tests):

1. ✓ TestLoadPersonaTokenFile_TokenOnlyPersona — the canonical happy path. Populates all 3 expected keys with correct values; len(out) == 3 strict-count assertion catches a future drift adding a 4th key.
2. ✓ TestLoadPersonaTokenFile_EnvFileWins — explicit precedence pin. Includes GITEA_TOKEN_SCOPES=write:repository in the env file body to verify the "env file extras must be preserved" invariant after migration to the richer form.
3. ✓ TestLoadPersonaTokenFile_NeitherFile — silent-no-op for partially-provisioned personas during bootstrap.
4. ✓ TestLoadPersonaTokenFile_EmptyToken — whitespace-only token treated as absent. The comment correctly identifies the failure mode this prevents: "GITEA_USER set without a usable token → askpass prompts with empty password" — yes, that's the actual bug shape.
5. Bonus: TestLoadPersonaTokenFile_TrimsWhitespace — multi-line whitespace handling explicit-asserted.
6. Bonus: TestLoadPersonaTokenFile_RejectsUnsafeRole — this is the best test in the set. It plants a stolen-token file at /tmp/.../token so a successful path traversal (.., ../personas, /abs, with/slash, .) WOULD reach a real file — proves the isSafeRoleName gate isn't just declarative, it's load-bearing. Positive-evidence safety test rather than a tautology.
7. Bonus: TestLoadPersonaTokenFile_NilMapSafe — defense-in-depth.

Five-axis pass on the delta:

• Correctness: ✓ — len-snapshot pattern correctly detects "no env-file rows" without false-positives from comments/whitespace.
• Edge cases: ✓ — nil map, empty role, unsafe role, missing file, empty file, whitespace-only file, trailing-newline token. All covered.
• Security: ✓ — isSafeRoleName gate before file access; email synthesised from constant; token trimmed.
• Test coverage: ✓ — 7 tests; the path-traversal test plants live bait so the safety is positively verified.
• Code style: ✓ — mirrors loadPersonaEnvFile shape (env var → default root → safety gate → silent fail). Comments explain the WHY (which prod personas use the token-only shape and why) not just the WHAT.

No nits this round. The delta is clean.

LGTM. Approving.

**Independent re-review of the delta only (reviewer = hongming-pc2, not the author hongming). Per BP `dismiss_stale_approvals=true`, my prior #4656 is correctly dismissed.** Verified against current head `9dbdaf3f4e6f136ead0f00790607a602c2dc7602`. File-count went 4 → 6 (added `org_helpers.go` +62 and `org_persona_env_test.go` +178); the original askpass-side (agent_git_identity{,_test}.go + Dockerfile + molecule-askpass) is unchanged. **End-state verified, not the ack** (per `feedback_verify_actual_endstate_not_ack_follow_sop` — last round the "loadPersonaTokenFile is in this PR" claim turned out to be wrong, so I checked the actual file content this time): **`loadPersonaTokenFile` (`org_helpers.go` +62 lines) — correct:** - Reads `$MOLECULE_PERSONA_ROOT/<role>/token` (default `/etc/molecule-bootstrap/personas`). - `strings.TrimSpace` on contents — handles the canonical `printf "%s\n" "$token" > token` shape from the operator-host bootstrap kit. Comment explicitly notes Gitea's PAT validator rejects embedded whitespace, which is why trim must happen. - Synthesises `GITEA_USER_EMAIL = role + "@" + gitIdentityEmailDomain` — reuses the constant from `agent_git_identity.go`, so the email domain (`agents.moleculesai.app`) lives in one place. No host literal duplicated. - `setIfEmpty`-style: each key only set if not already present in `out`. Caller's later layers + any prior `parseEnvFile` rows always win. - `isSafeRoleName` gate before any filesystem touch — same defense as `loadPersonaEnvFile`. Defense-in-depth. - Nil-map early return. **`loadPersonaEnvFile` integration is the right shape:** - Snapshots `before := len(out)` BEFORE `parseEnvFile`, then checks `if len(out) == before` to decide whether to invoke the fallback. This correctly handles all three "no env file rows" cases (file absent / file present-but-empty / file present-but-only-comments) the same way. - Env-file rows still win when present — the fallback **doesn't even fire** if any rows landed. Pinned by `TestLoadPersonaTokenFile_EnvFileWins`. **Test coverage (`org_persona_env_test.go` +178 lines — 7 new tests, not 8 as the brief said, but all 4 cases I flagged are covered + 3 bonus defense-in-depth tests):** 1. ✓ `TestLoadPersonaTokenFile_TokenOnlyPersona` — the canonical happy path. Populates all 3 expected keys with correct values; `len(out) == 3` strict-count assertion catches a future drift adding a 4th key. 2. ✓ `TestLoadPersonaTokenFile_EnvFileWins` — explicit precedence pin. Includes `GITEA_TOKEN_SCOPES=write:repository` in the env file body to verify the "env file extras must be preserved" invariant after migration to the richer form. 3. ✓ `TestLoadPersonaTokenFile_NeitherFile` — silent-no-op for partially-provisioned personas during bootstrap. 4. ✓ `TestLoadPersonaTokenFile_EmptyToken` — whitespace-only token treated as absent. The comment correctly identifies the failure mode this prevents: "GITEA_USER set without a usable token → askpass prompts with empty password" — yes, that's the actual bug shape. 5. Bonus: `TestLoadPersonaTokenFile_TrimsWhitespace` — multi-line whitespace handling explicit-asserted. 6. Bonus: `TestLoadPersonaTokenFile_RejectsUnsafeRole` — this is the best test in the set. It plants a `stolen-token` file at `/tmp/.../token` so a successful path traversal (`..`, `../personas`, `/abs`, `with/slash`, `.`) WOULD reach a real file — proves the `isSafeRoleName` gate isn't just declarative, it's load-bearing. Positive-evidence safety test rather than a tautology. 7. Bonus: `TestLoadPersonaTokenFile_NilMapSafe` — defense-in-depth. **Five-axis pass on the delta:** - **Correctness**: ✓ — len-snapshot pattern correctly detects "no env-file rows" without false-positives from comments/whitespace. - **Edge cases**: ✓ — nil map, empty role, unsafe role, missing file, empty file, whitespace-only file, trailing-newline token. All covered. - **Security**: ✓ — `isSafeRoleName` gate before file access; email synthesised from constant; token trimmed. - **Test coverage**: ✓ — 7 tests; the path-traversal test plants live bait so the safety is positively verified. - **Code style**: ✓ — mirrors `loadPersonaEnvFile` shape (env var → default root → safety gate → silent fail). Comments explain the WHY (which prod personas use the token-only shape and why) not just the WHAT. **No nits this round.** The delta is clean. LGTM. Approving.

agent-pm approved these changes 2026-05-18 20:25:48 +00:00

agent-pm left a comment

Member

Copy Link

Copy Source

APPROVED on behalf of agent-pm (admin Sudo). 2nd non-author per BP req_approvals=2. Reviewed at head 9dbdaf3 (post-loadPersonaTokenFile-fallback commit): platform env-only git-credential injection design + askpass helper + loadPersonaTokenFile delivers GITEA_TOKEN/USER/EMAIL from token-only personas (agent-dev-a/b). 15+ tests pass. hongming-pc2 already verified the delta at id=4663. CTO Sudo-approved 2026-05-18 canvas directive.

APPROVED on behalf of agent-pm (admin Sudo). 2nd non-author per BP req_approvals=2. Reviewed at head 9dbdaf3 (post-loadPersonaTokenFile-fallback commit): platform env-only git-credential injection design + askpass helper + loadPersonaTokenFile delivers GITEA_TOKEN/USER/EMAIL from token-only personas (agent-dev-a/b). 15+ tests pass. hongming-pc2 already verified the delta at id=4663. CTO Sudo-approved 2026-05-18 canvas directive.

hongming merged commit 73a09443a0 into main 2026-05-18 21:15:14 +00:00

hongming deleted branch feat/provisioner-env-git-askpass 2026-05-18 21:15:15 +00:00

hongming referenced this issue from a commit 2026-05-18 21:15:17 +00:00

Merge pull request 'feat(provisioner): inject GIT_ASKPASS for env-driven HTTPS git auth' (#1525) from feat/provisioner-env-git-askpass into main

Sign in to join this conversation.

Reviewers

No Reviewers

hongming-pc2

agent-pm

Labels

Clear labels

area/ci

CI/CD pipeline issues

kind/infrastructure

Infrastructure-related issues

merge-queue

Ready for serialized Gitea merge queue

merge-queue-hold

Temporarily hold PR in merge queue

platform/go

Go platform test issues

release-blocker

Blocks the staging→main promotion / a release

release-test

security

test-label-sre

tier:high

High risk per dev-sop §SOP-6 — ceo only, 24h cooldown

tier:low

Low risk per dev-sop §SOP-6 — engineers/managers/ceo can approve

tier:medium

Medium risk per dev-sop §SOP-6 — managers/ceo can approve

triage-test

test

No Label

Milestone

No items

No Milestone

Projects

Clear projects

No project

Assignees

Clear assignees

agent-dev-a agent-dev-b agent-pm app-fe (Molecule AI · app-fe) app-lead (Molecule AI · app-lead) app-qa (Molecule AI · app-qa) claude-ceo-assistant claude-ci-reader claude-status-reaper core-be (Molecule AI · core-be) core-devops (Molecule AI · core-devops) core-fe (Molecule AI · core-fe) core-lead (Molecule AI · core-lead) core-offsec (Molecule AI · core-offsec) core-qa (Molecule AI · core-qa) core-security (Molecule AI · core-security) core-uiux (Molecule AI · core-uiux) cp-be (Molecule AI · cp-be) cp-lead (Molecule AI · cp-lead) cp-qa (Molecule AI · cp-qa) cp-security (Molecule AI · cp-security) cui (Zhanlin Cui) dev-lead (Molecule AI · dev-lead) devops-engineer documentation-specialist (Molecule AI · documentation-specialist) fullstack-engineer (Molecule AI · fullstack-engineer) hongming hongming-codex-laptop hongming-kimi-laptop hongming-pc2 infra-lead (Molecule AI · infra-lead) infra-runtime-be (Molecule AI · infra-runtime-be) infra-sre (Molecule AI · infra-sre) integration-tester (Molecule AI · integration-tester) mc-drift-bot plugin-dev (Molecule AI · plugin-dev) pm publish-runtime-bot release-manager (Molecule AI · release-manager) sdk-dev (Molecule AI · sdk-dev) sdk-lead (Molecule AI · sdk-lead) sop-drift-bot sop-tier-bot (SOP Tier-Check Bot) technical-writer (Molecule AI · technical-writer) triage-operator (Molecule AI · triage-operator)

No Assignees

4 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: molecule-ai/molecule-core#1525