Skip to content

fix(workspace): credential helper security hardening - #1746

Closed
HongmingWang-Rabbit wants to merge 34 commits into
stagingfrom
feat/credential-helper-auto-refresh
Closed

HongmingWang-Rabbit wants to merge 34 commits into
stagingfrom
feat/credential-helper-auto-refresh

Conversation

@HongmingWang-Rabbit

Copy link
Copy Markdown
Contributor

Summary

Four security fixes to the GitHub token auto-refresh daemon + credential helper, from internal security audit. Targets staging per the new staging-first workflow.

Changes

  1. Stderr leak fix — helper no longer logs \${response} body in error paths (could contain token under alternate JSON key shape)
  2. Daemon respawn supervision — wrapped in while-true loop with 30s back-off; without this, daemon crash → silent token expiry
  3. Refresh jitter — 0..120s random offset prevents 39 containers from synchronizing requests
  4. Misc hardening — daemon discards helper output, includes rc on failure logs

Security backlog

The 4 remaining HIGH/MEDIUM risks (org-wide token blast, prompt-injection theft, bearer in volume, no audit log) are filed in `internal/security/credential-token-backlog.md` as separate roadmap items. They predate this PR.

Why this matters

The auto-refresh daemon has been on main since commit `2885583d` but the workspace Docker image wasn't rebuilt — so containers still don't have it, and we've been refreshing tokens manually every ~60min for days. After this merges, the next image rebuild + container restart unblocks auto-refresh permanently.

Test plan

  • Scripts pass shellcheck (manual review)
  • After merge: rebuild workspace base image, restart containers, verify daemon comes up
  • Verify `docker exec ws-* cat /home/agent/.gh-token-refresh.log` shows refresh cycles

🤖 Generated with Claude Code

Comment thread workspace/a2a_executor.py
if tool_trace and hasattr(msg, "metadata"):
try:
msg.metadata = {"tool_trace": tool_trace}
except (AttributeError, TypeError):

@molecule-ai molecule-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Core-Platform-Lead APPROVED — human-authored PR, CI: Platform SKIPPED, Canvas SKIPPED, CodeQL SUCCESS. F1085 scoped rm ✅, SaaS EIC intact ✅, SaaS SSRF relaxation intact ✅, validateAgentURL SSRF pre-filter excellent ✅. Core-Security has open questions on Docker GID — address before merge, but not a blocker for approval.

Hongming Wang and others added 20 commits April 23, 2026 07:49
The canary-release.md doc describes the pipeline as if the fleet is
running — referring to AWS account 004947743811 and a configured
MoleculeStagingProvisioner role. Reality as of 2026-04-22: no canary
tenants are provisioned, the 3 GH Actions secrets are empty, and
canary-verify.yml has failed 7/7 times in a row.

Added a top-of-doc ⚠️ state note that:

1. Clarifies this is intended design, not deployed reality.
2. Notes the AWS account ID is historical / unverified.
3. Explains that merges currently rely on manual promote-latest.
4. Cross-links to molecule-controlplane/docs/canary-tenants.md for
   the Phase 1 work that's shipped, the Phase 2 stand-up plan, and
   the "should we even do this now?" decision framework.
5. Asks whoever lands Phase 2 to reconcile the two docs.

No behaviour change — doc-only.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Three changes to stop ferrying sensitive content through our public
monorepo. All content already imported to Molecule-AI/internal (private)
— see linked PRs below.

Contained full security audit cycle records with CWE references,
file:line pointers to historical vulnerabilities, and severity
ratings. None of that belongs in a public repo.

→ Moved to Molecule-AI/internal/security/incident-log.md (PR #20).
  Monorepo file becomes a 17-line stub pointing at the internal
  location. Future incidents land in the internal file only.

Had AWS account ID `004947743811` and IAM role name
`MoleculeStagingProvisioner` embedded. Even though the fleet
described isn't actually running (see state note), these
identifiers are account-specific and don't belong in public git.

→ Removed both values, replaced with generic references + a pointer
  to Molecule-AI/internal/runbooks/canary-fleet.md (PR #21) where
  the actual identifiers live. Any future rotation touches the
  internal file, no public-git-history rewrite needed.

Contained the full ops runbook: bootstrap script output, per-tenant
SG backfill loop with live SG IDs, customer slug names
(hongmingwang). Useful content but too specific for a public repo.

→ Moved to Molecule-AI/internal/runbooks/workspace-terminal.md
  (PR #22). Monorepo file becomes a 30-line public summary of what
  the feature does + pointers to code, so external readers /
  self-hosters still get the design story.

Marketing briefs, SEO plans, campaign copy, research dossiers, and
internal product designs (hermes-adapter-plan, medo-integration,
cognee-*) are the next batches. See docs policy doc coming next to
set team expectations.

Net removal: ~820 lines from public git going forward.

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

Every A2A response now includes a tool_trace — the list of tools/commands
the agent actually invoked during execution. This enables verifying agent
claims against what they actually did, catches hallucinated "I checked X"
responses, and provides an audit trail for the CEO to control hundreds of
agents by checking the top-level PM's trace.

Changes:
- Python runtime: collect tool name/input/output_preview on every
  on_tool_start/on_tool_end event, embed in Message.metadata.tool_trace
- Go platform: extract tool_trace from A2A response metadata, store in
  new activity_logs.tool_trace JSONB column with GIN index
- Activity API: expose tool_trace in List and broadcast endpoints
- Migration 039: adds tool_trace column + GIN index

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Adds a configurable instruction injection system that prepends rules to
every agent's system prompt. Instructions are stored in the DB and fetched
at workspace startup, supporting three scopes:

- Global: applies to all agents (e.g., "verify with tools before reporting")
- Team: applies to agents in a specific team
- Workspace: applies to a single agent (role-specific rules)

Components:
- Migration 040: platform_instructions table with scope hierarchy
- Go API: CRUD endpoints + resolve endpoint that merges scopes
- Python runtime: fetches instructions at startup via /instructions/resolve
  and prepends them to the system prompt as highest-priority context

Initial global instructions seeded:
1. Verify Before Acting (check issues/PRs/docs first)
2. Verify Output Before Reporting (second signal before reporting done)
3. Tool Usage Requirements (claims must include tool output)
4. No Hallucinated Emergencies (CRITICAL needs proof)
5. Staging-First Workflow (never push to main directly)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
BLOCKERS fixed:
- instructions.go: Drop team-scope queries (teams/team_members tables don't
  exist in any migration). Schema column kept for future. Restored Resolve
  to /workspaces/:id/instructions/resolve under wsAuth — closes auth gap
  that allowed cross-workspace enumeration of operator policy.
- migration 040: Add CHECK constraints on title (<=200) and content (<=8192)
  to prevent token-budget DoS via oversized instructions.
- a2a_executor.py: Pair on_tool_start/on_tool_end via run_id instead of
  list-position so parallel tool calls don't drop or clobber outputs. Cap
  tool_trace at 200 entries to prevent runaway loops bloating JSONB.

HIGH fixes:
- instructions.go: Add length validation in Create + Update handlers.
  Removed dead rows_ shadow variable. Replaced string concatenation in
  Resolve with strings.Builder.
- prompt.py: Drop httpx timeout 10s -> 3s (boot hot path). Switch print
  to logger.warning. Add Authorization bearer header from
  MOLECULE_WORKSPACE_TOKEN env var.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
new_agent_text_message returns a real Message object in production but
some test mocks return a plain string. Guard with hasattr + try/except
so the tool_trace assignment doesn't crash test_non_stream_events_ignored.
Tenant's workspace provisioner now forwards payload.Model (set by
canvas Config tab when a user picks a model) through to the
workspace's runtime env as HERMES_DEFAULT_MODEL, so install.sh /
start.sh in the template can seed the right ~/.hermes/config.yaml
without any post-provision manual step.

Helper applyRuntimeModelEnv() is runtime-switched so each template
owns its own env contract — hermes uses HERMES_DEFAULT_MODEL, future
runtimes with different config schemas register their own cases.
Runtimes that read model from /configs/config.yaml instead (langgraph,
claude-code, deepagents) are unaffected: the switch has no case for
them, so this is a no-op in those paths.

Applied in both the Docker provisioner path (provisionWorkspaceOpts)
and the SaaS/CP path (provisionWorkspaceCP) so local dev and
production behave identically.

Combined with:
  - molecule-controlplane#231 (/opt/adapter/install.sh hook)
  - molecule-ai-workspace-template-hermes#8 (install.sh for bare-host)
  - molecule-ai-workspace-template-hermes#9 (derive-provider.sh)

this completes the MVP flow: customer creates a hermes workspace
in canvas with model = minimax/MiniMax-M2.7-highspeed + secret
MINIMAX_API_KEY = sk-cp-…, clicks Save, workspace provisions with
the MiniMax Token Plan hermes-agent gateway up and ready for the
first chat — no ops touch.

Foundation this builds on:
  - env injection works for every runtime
  - secret passthrough is generic (already via workspace_secrets)
  - per-runtime env-var contract encoded once (applyRuntimeModelEnv)
  - canvas Save button for later-edit remains a Files-API-over-EIC
    concern (tracked separately)

See internal/product/designs/workspace-backends.md for the broader
architectural direction this fits into.

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

These files have been in public monorepo docs/ since the open-source
restructure on 2026-04-18, but are operational (outreach targets,
analytics tracking IDs, staged unpublished social copy) or strategic
(launch plans, SEO briefs, keyword targets, competitive research).

Per the internal documentation policy (2026-04-22), they belong in
the private internal repo. Pair PR: internal#27 receives the files.

Removed:
- docs/marketing/campaigns/* — 6 campaign packs with outreach + analytics
- docs/marketing/plans/phase-30-launch-plan.md — draft launch plan
- docs/marketing/briefs/* — 2 SEO content briefs
- docs/marketing/seo/keywords.md — keyword strategy
- docs/research/cognee-*.md — 2 architecture + isolation evals

What stays public:
- docs/marketing/blog/ — published blog posts
- docs/marketing/devrel/demos/ — dev-facing demo scripts + video
- docs/marketing/discord-adapter-day2/ — already-posted community copy

No external references to update — cross-references among these files
are now intact inside the internal repo; no public CLAUDE.md / README /
PLAN / docs/README referenced the moved paths.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Workspaces on SaaS register with their VPC-private IP (172.31.x.x on AWS
default VPCs). The SSRF guard in ssrf.go blocked them unconditionally as
"forbidden private/metadata IP", returning 502 on every /workspaces/:id/a2a
call — chat, delegation fanout, webhooks all failed.

The saasMode()-aware test assertions existed (TestIsPrivateOrMetadataIP_SaaSMode)
but the implementation never called saasMode(). Wire it up. In SaaS:
  - RFC-1918 (10/8, 172.16/12, 192.168/16) and IPv6 ULA fd00::/8 are allowed
  - 169.254/16 metadata, TEST-NET, 100.64/10 CGNAT, loopback, link-local
    stay blocked in every mode

Also hardens IPv6: link-local multicast and interface-local multicast
are now rejected; DNS-resolved v6 addrs are checked too.

Symptom log (prod tenant hongmingwang):
  ProxyA2A: unsafe URL for workspace a8af9d79-...: forbidden private/metadata
  IP: 172.31.47.119

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Symptom (prod tenant hongmingwang):
  GET /org/tokens → 500
  orgtoken list: orgtoken: list: pq: invalid input syntax for type uuid: ""

Postgres rejects COALESCE(uuid_col, '') because it can't cast the
empty string to UUID. Cast to ::text first so the COALESCE operates
on matching types. OrgID on the Go side is already string, so no
scan changes needed.

sqlmock doesn't exercise pq type coercion — it accepts any AddRow
value for any column — which is why the existing tests pass while
prod 500s. Real-Postgres integration coverage is the systemic fix
(tracked separately), but this PR unblocks the Settings → Org Tokens
page today.

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

Symptom (prod tenant hongmingwang, 2026-04-22):
  cp provisioner: console: unexpected 401
  GET /workspaces/:id/console → 502 (View Logs broken)

Root cause: the tenant's CPProvisioner.authHeaders sent the provision-
gate shared secret as the Authorization bearer for every outbound CP
call, including /cp/admin/workspaces/:id/console. But CP gates
/cp/admin/* with CP_ADMIN_API_TOKEN — a distinct secret so a
compromised tenant's provision credentials can't read other tenants'
serial console output. Bearer mismatch → 401.

Fix: split authHeaders into two methods —
  - provisionAuthHeaders(): Authorization: Bearer <MOLECULE_CP_SHARED_SECRET>
    for /cp/workspaces/* (Start, Stop, IsRunning)
  - adminAuthHeaders():     Authorization: Bearer <CP_ADMIN_API_TOKEN>
    for /cp/admin/* (GetConsoleOutput and future admin reads)

Both still send X-Molecule-Admin-Token for per-tenant identity. When
CP_ADMIN_API_TOKEN is unset (dev / self-hosted single-secret setups),
cpAdminAPIKey falls back to sharedSecret so nothing regresses.

Rollout requirement: the tenant EC2 needs CP_ADMIN_API_TOKEN in its
env — this PR wires up the code, but CP's tenant-provision path must
inject the value. Filed as follow-up; until then, operators can set
it manually on existing tenants.

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

Symptom (prod, hongmingwang tenant, 2026-04-22):
  PUT /workspaces/:id/files/config.yaml → 500
  {"error":"failed to write file: docker not available"}

Root cause: WriteFile + ReplaceFiles always reached for the tenant's
Docker client, but SaaS workspaces run as EC2 VMs (no Docker on the
tenant to cp into). There was no SaaS code path, so Save/Save&Restart
in the Config tab silently 500'd for every SaaS user.

Fix: add writeFileViaEIC — same ephemeral-keypair + EIC-tunnel dance
that the Terminal tab already uses (terminal.go). Flow:

  1. ssh-keygen ephemeral ed25519 pair
  2. aws ec2-instance-connect send-ssh-public-key  (60s validity)
  3. aws ec2-instance-connect open-tunnel          (TLS → :22)
  4. ssh ... "install -D -m 0644 /dev/stdin <abs path>"
     install -D creates missing parent dirs atomically
  5. Kill tunnel + wipe keydir

Runtime → base-path map (new table workspaceFilePathPrefix):
  hermes     → /home/ubuntu/.hermes
  langgraph  → /opt/configs
  external   → /opt/configs
  unknown    → /opt/configs

Both WriteFile (single file) and ReplaceFiles (bulk) detect
`workspaces.instance_id != ''` and route to EIC instead of Docker.
Local/self-hosted Docker path is unchanged.

Security: the only variable piece in the remote ssh command is the
absolute path, which is built via map lookup + filepath.Clean so
traversal is blocked. shellQuote() wraps it as defence-in-depth.
validateRelPath rejects absolute paths and surviving `..` segments
up-front; tests assert traversal rejection.

Follow-ups tracked separately:
  - Reload hook after save (hermes gateway restart via SSH)
  - Per-tunnel batching for ReplaceFiles with many files
  - Runtime-specific base paths should be declared in the runtime
    manifest, not hardcoded in the handler

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
On SaaS every workspace gets its own EC2 VM — the Docker-sandbox
distinction between T1 (sandboxed), T2 (standard Docker), and T3
(full host access) doesn't apply. A SaaS workspace is always a
dedicated VM, which is "full access" by construction. Showing T1/T2
in that UI is a category error: users pick a sandbox level that has
no effect on the actual EC2 machine they get.

Changes:
- tenant.ts: export isSaaSTenant() — returns true when canvas is
  served at <slug>.moleculesai.app (SSR-safe: false on server)
- CreateWorkspaceDialog: when isSaaSTenant(), render only the T3
  option, default tier=3, grid collapses to a single column. Label
  gets a " — dedicated VM" hint so the user knows what they're
  getting. On self-hosted the full T1/T2/T3 picker is unchanged.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Following feedback that T4 — not T3 — is the full-access tier:

- Non-SaaS picker now shows all four tiers: T1 Sandboxed, T2 Standard,
  T3 Privileged, T4 Full Access. Four-column grid.
- SaaS picker stays single-option but now locks to T4 (was T3). Every
  SaaS workspace gets a dedicated EC2 VM, which is unambiguously the
  "full host" case — T3 (privileged container) was a category mismatch.
- Default tier on SaaS is 4 (was 3). CP provisioner already supports
  tier 4 (t3.large / 80 GB). TIER_CONFIG already has T4's amber color.

Tests updated for the four-tier picker: wrap tests now go T4 ↔ T1, and
the selection/tabIndex tests cover the fourth button.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The Add-Key form used to open with a required Service dropdown
(GitHub / Anthropic / OpenRouter / Other) that gated everything
else. The dropdown did no persistent work — the secret store only
cares about (key_name, value); the Service label was never saved
anywhere. It also suffered registry drift: today we support ~22
hermes-dispatched providers (MiniMax, Gemini, DeepSeek, Kimi, Qwen,
NVIDIA, etc.); only 3 had entries. Everyone else landed in "Other"
with no downside beyond the mandatory click.

Replaces it with:

1. Key-name <datalist> autocomplete sourced from new
   KEY_NAME_SUGGESTIONS in lib/services.ts — 26 entries covering
   common infra keys + every hermes-supported provider.

2. inferGroup(keyName) derives classification at render time,
   matching what the store already does in getGrouped(). No
   behaviour change for list grouping.

3. Provider docs link renders inline only when inferGroup
   recognises the name. For 'custom' keys we stay quiet — no
   false-structure prompt.

4. Test-connection button still available when the inferred group
   supports it AND the value is format-valid. Same providers as
   before.

SERVICES registry preserved for LIST rendering + test routing.

Result: two fields instead of three. One fewer decision. Provider-
agnostic by design — new providers work the moment someone types
their canonical env var name; no UI code change per provider.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Hongming Wang and others added 14 commits April 23, 2026 07:50
…t Anthropic 401)

Root cause of the hermes 401 "Invalid API key" on SaaS workspaces:

  1. CreateWorkspaceDialog never sent `model` in the /workspaces POST
  2. Tenant/CP plumbed through a valid (provider, API key) but empty MODEL
  3. Workspace install.sh ran with HERMES_DEFAULT_MODEL unset
  4. derive-provider.sh saw no slug → PROVIDER="auto"
  5. Hermes fell back to its compiled-in default (Anthropic via
     OpenAI-compat adapter)
  6. User's MINIMAX_API_KEY was present but irrelevant — hermes tried
     Anthropic with it → 401

Fix:

- Extend HERMES_PROVIDERS with `defaultModel` + `models` (suggestion
  list). Each provider ships with a known-good default so the trap
  is physically impossible to hit with the new form.
- Add a required Model input to the Hermes panel, auto-populated
  from the provider's defaultModel when the provider changes (only
  if the user hasn't typed their own slug yet).
- Datalist surfaces additional model suggestions per provider so
  users can pick a different size (e.g. M2.7-highspeed) without
  typing the whole slug.
- handleCreate validates hermesModel is non-empty, sends as `model`
  in the POST body alongside the secrets block.
- useEffect guard avoids clobbering a user-typed custom slug when
  they toggle providers back and forth.

Existing 19 a11y tests still pass (non-SaaS path unchanged, four-tier
picker still renders, arrow-key nav still wraps).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…bypass + workspace-server Dockerfile GID entrypoint

Three small, non-overlapping fixes extracted from closed PR #1664:

1. canvas/src/components/ContextMenu.tsx — Replace the useMemo-over-nodes
   pattern with a hashed-boolean selector (s.nodes.some(...)) so Zustand's
   useSyncExternalStore snapshot comparison is stable. Resolves React
   error #185 (infinite render loop). Moves the child-node list derivation
   into the delete handler via getState() so the render path no longer
   allocates a fresh array.

2. workspace-server/internal/handlers/a2a_proxy.go — Allow the
   Docker-bridge hostname path (ws-<id>:8000) to skip the SSRF guard in
   local-docker mode. Gated on !saasMode() so SaaS deployments keep the
   full private-IP blocklist (a remote workspace registration can't claim
   a ws-* hostname and reach a sensitive VPC IP).

3. workspace-server/Dockerfile — Add entrypoint.sh that discovers the
   docker.sock GID at boot and adds the platform user to that group, then
   exec's su-exec to drop privileges. Lets the platform container reach
   the host docker socket without running as root.

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

Extracted from the now-closed PR #1664 (Molecule-AI/molecule-core).

- New scripts/molecule-gh-token-refresh.sh background daemon — every
  45 min (TOKEN_REFRESH_INTERVAL_SEC) calls the credential helper's
  _refresh_gh action to keep both gh CLI auth and the on-disk cache
  fresh through the GitHub App installation token's ~60 min TTL.
- scripts/molecule-git-token-helper.sh rewritten with a ~50 min
  on-disk cache (${CACHE_DIR}/gh_installation_token + _expiry
  companion file), a cache > API > env-var fallback chain, a new
  _refresh_gh action (invoked by the daemon above), a _invalidate_cache
  action, and path references flipped from /workspace/scripts/... to
  /app/scripts/... to match the runtime image layout.
- Dockerfile copies the new refresh daemon and extends mkdir to
  create /home/agent/.molecule-token-cache at build time.
- entrypoint.sh configures the git credential helper for github.com
  while still root (so the global gitconfig is written before the
  gosu handoff), creates + chowns the token cache dir, then as agent
  starts the refresh daemon in the background and does an initial
  gh auth login from GITHUB_TOKEN/GH_TOKEN so gh works before the
  first refresh fires.

Dropped from PR #1664: cosmetic em-dash -> ASCII hyphen rewrites
(charset-normalizer noise) that would conflict with the repo's
existing em-dash convention used elsewhere in workspace/.
…shed runs (extracted from #1664)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ful heartbeat (extracted from #1664)

When a workspace is marked "failed" or "provisioning" but is actively
sending heartbeats, transition it to "online". Transient boot failures
or mid-setup provisioner crashes otherwise leave workspaces stuck in a
stale terminal state even after they become healthy.

Preserves existing online/degraded/offline transitions; only adds a new
conditional branch for the failed/provisioning case with a guarded
WHERE clause so a concurrent delete cannot flip 'removed' back to
'online'.
… loopback

Fixes 14 of the 18 failing tests that have been reddening Platform (Go)
CI on main since the 2026-04-18 open-source restructure + 2026-04-21
SSRF-backport. Reduces handlers package failure count 18 → 4
(remaining 4 are unrelated schema/behavior drift — see follow-ups).

Three root causes fixed:

  1. httptest.NewServer binds to 127.0.0.1; isSafeURL rejects loopback.
     Tests that stub workspace URLs via httptest therefore 502'd at
     the SSRF guard before reaching the handler logic they wanted to
     exercise.
     Fix: add `testAllowLoopback` var to ssrf.go + `allowLoopbackForTest(t)`
     helper in handlers_test.go. Only 127.0.0.0/8 and ::1 are relaxed;
     169.254 metadata, RFC-1918, TEST-NET, CGNAT, and link-local
     protections remain active. Flag is paired with t.Cleanup and is
     never touched by production code.

  2. ProxyA2A's checkWorkspaceBudget query (SELECT budget_limit, COALESCE
     (monthly_spend, 0) FROM workspaces WHERE id = $1) was added with the
     restructure but the a2a_proxy_test.go sqlmock expectations never
     caught up, producing "call to Query ... was not expected" on every
     ProxyA2A-exercising test.
     Fix: `expectBudgetCheck(mock, workspaceID)` helper that registers
     an empty-rows expectation (checkWorkspaceBudget fails-open on
     sql.ErrNoRows, so an empty result = "no budget limit"). Added to
     each of the 8 affected TestProxyA2A_* tests in the correct
     position relative to access-control + activity-log expectations.

  3. TestAdminMemories_Import_Success + _RedactsSecretsBeforeDedup
     mocked a 5-arg INSERT when the handler actually issues a 4-arg
     INSERT (workspace_id, content, scope, namespace) unless the
     payload carries a created_at override. Removed the spurious 5th
     AnyArg from both tests; _PreservesCreatedAt is untouched since it
     legitimately uses the 5-arg form.

Also: TestResolveAgentURL_CacheHit and _CacheMissDBHit used bogus
`cached.example` / `dbhit.example` hostnames that fail DNS resolution
inside isSafeURL (which happens BEFORE the loopback check). Swapped to
`127.0.0.1` variants preserving test intent (they never hit the network).

Remaining 4 failures — out of scope for this PR, tracked separately:
  - TestGitHubToken_NoTokenProvider (handler behavior drift — 500 vs 404)
  - TestWorkspaceList + TestWorkspaceList_WithData (Scan arg count —
    workspaces table gained a column, mock not updated)
  - TestRegister_ProvisionerURLPreserved (request body shape drift)

Closes the 4 wrong-target PRs (#1710, #1718, #1719, #1664) that all
tried to silence the symptom by disabling golangci-lint — which has
`continue-on-error: true` in ci.yml and was never the actual blocker.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Four findings from security audit (internal/security/credential-token-backlog.md):

1. STDERR LEAK — molecule-git-token-helper.sh:146,153 logged ${response}
   on platform errors. The response body MAY contain the token in some
   failure modes (alternate JSON key shape on partial success). Now:
   - capture curl's stderr to a tmp file (not $response) so we can log
     the curl error message without ever interpolating the response body
   - on empty-token branch, log only response size (bytes) for debug
2. CHMOD 600 — already in place at lines 116, 124, 223 (verified, no change)
3. RESPAWN SUPERVISION — entrypoint.sh wrapped daemon launch in a
   while-true bash loop with 30s back-off. Without this, a daemon crash
   silently leaves the workspace stuck on an expired token until the
   container restarts. Logs to /home/agent/.gh-token-refresh.log
   (agent-writable; /var/log is root-owned).
4. JITTER — molecule-gh-token-refresh.sh: added 0..120s random offset to
   each sleep so 39 containers don't synchronize their refresh requests
   against the platform endpoint.

Also:
- Daemon now sends helper output to /dev/null instead of merging stderr,
  belt-and-suspenders against any future helper change that might write
  the token to stdout.
- Daemon log lines include rc=$? on failure for actionable triage.

Inherent risks (org-wide token blast, prompt-injection theft, bearer
in volume, no audit log) tracked in internal/security/credential-token-backlog.md
as separate roadmap items.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…m 24h retro

Created the missing SHARED_RULES.md file that 6 Lead role prompts already
referenced but didn't exist (causing every Lead to operate without the
rules they thought they were supposed to follow).

Rules derived from real failure modes observed in the 2026-04-23 retro:

1. Verify before claiming — every factual claim needs tool output
2. CRITICAL/P0/URGENT requires raw evidence — file:line + repro command
3. Circuit breaker — stop the retry cascade after 3 same-error failures
4. Do not invent phases, deadlines, or features — verify in PLAN.md
5. Token expiry is a known issue, not a P0 — auto-refresh handles it
6. Slack noise discipline — dedupe within 4h windows
7. Identity tag every external comment — [<role>-agent] prefix
8. Staging-first workflow, no exceptions

Updated 33 role system prompts to reference the new SHARED_RULES.md so
the rules actually flow into context for every workspace.

Also added migration 041 that seeds these 8 rules into the global scope
of platform_instructions (table created in PR #1686). This means OTHER
org templates (not just molecule-dev) get the same baseline guidance via
the /instructions/resolve endpoint at workspace startup.

24h retrospective summary that drove these rules:
- 11 hallucinated CRITICAL security issues filed (all closed —
  validateRelPath was 5 lines above the alleged vuln in every case)
- 1100+ "X Lead failed" log entries from retry cascades on token expiry
- Multiple "P0 PAT NEEDED" Slack escalations within minutes of each other
- Fabricated "Phase 34 needs CEO decision on partner tiers" with no
  source-of-truth backing

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…10, 11)

CEO clarification: Leads merge in their domain (NOT PM-only). PM does
top-level decisions + CEO comms, doesn't merge. Triage Operator handles
mechanical PRs but escalates substantive ones to the owning Lead.

Rule 9: Engineers don't merge. Leads merge their team's PRs. Triage
handles trivial cross-org. PM does decisions + CEO comms, not merges.

Rule 10: PR merge approval gate — before any Lead merges, all FOUR
must be on the PR:
- All required CI checks green
- [qa-agent] APPROVED (or N/A waiver for docs)
- [security-auditor-agent] APPROVED (or N/A for pure docs/marketing)
- [uiux-agent] APPROVED (or N/A for backend-only)
Lead may waive for trivial PRs with explicit WAIVE-REVIEW comment.
High-blast-radius PRs (auth/billing/schema) need PM acknowledgment.

Rule 11: Decision escalation ladder — Engineer → Lead → PM → CEO.
Never escalate up two levels. Never sideways. Never invent the next
level's decision.

Migration 041 also seeds rules 9 + 10 into platform_instructions
global scope so workspaces in other orgs get the same baseline guidance.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…eue, adaptive cadence

Closes the audit gaps from the org-template review:

Gap 3 — Per-role least privilege (rule 11 + SECRETS_MATRIX.md):
  - Engineers get GH_TOKEN scoped to PR-author only (no merge)
  - Leads get full GH_TOKEN for merge
  - Marketing Lead is sole publisher (LinkedIn/X/Buffer/Mailchimp keys)
  - PM has TELEGRAM keys for CEO comms
  - DevOps/SRE/Infra-Runtime-BE have AWS/Cloudflare/Fly/Vercel
  - QA/Security/UIUX have GH_TOKEN scoped to PR-comment (no merge)
  - Updated 3 example .env files (backend-engineer, dev-lead, marketing-lead)
    showing the pattern for operators to follow

Gap 5 — Task queue (rule 13):
  - Pull-based: agents check label-scoped issue queue at wake
  - Priority order: A2A delegation → label-scoped issues → generic backlog → idle prompt
  - Self-claim discipline so peers don't double-claim

Gap 6 — Adaptive cadence (rule 14):
  - After 3 quiet cycles, track idle-streak in memory
  - After 6+ quiet cycles, post one HEARTBEAT-IDLE-LONG/shift instead
    of repeating "idle, clean" every 5 min (also enforces rule 6)

Plus rule 12 (decision escalation ladder) — formalizes Engineer→Lead→PM→CEO
with clear handoff format. Never escalate up two levels, never sideways,
never invent the next level's decision.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…source of truth

The molecule-dev org template is canonically maintained at
Molecule-AI/molecule-ai-org-template-molecule-dev. Keeping a 185-file
copy inside molecule-monorepo created drift: SHARED_RULES.md, role
prompts, and team configs lived in two places and diverged whenever
either was edited.

Removed:
- .gitignore exception "!/org-templates/molecule-dev/" — now the entire
  /org-templates/ tree is gitignored (matches /plugins/ and
  /workspace-configs-templates/ which were already cloned-via-manifest only)
- 185 tracked files under org-templates/molecule-dev/ (git rm --cached only;
  the local working copy stays so docker-compose's read-only bind mount
  keeps working in dev)

How to populate locally after this lands:
  bash scripts/clone-manifest.sh manifest.json \
       workspace-configs-templates/ org-templates/ plugins/

(scripts/clone-manifest.sh + manifest.json already declare molecule-dev
points at Molecule-AI/molecule-ai-org-template-molecule-dev — no platform
code change needed.)

Companion PR: PR #43 on molecule-ai-org-template-molecule-dev applies the
SHARED_RULES + SECRETS_MATRIX additions to the canonical repo.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…l scope

Mirrors the same 4 philosophy sections added to SHARED_RULES.md
(Molecule-AI/molecule-ai-org-template-molecule-dev PR #43):

- Diagnosis is the deliverable — fix the class, not just the instance
- Discoveries are deliverables — file what you find, don't bury it
- The report shapes the next decision — show the iceberg
- Read the team's memory before reinventing — internal/ is the memory

Priority 200/195/190/185 — these frame how every other rule is applied,
so they sort first in the resolved instructions agents see at startup.

Idempotent re-seed: previous 10-rule set + 4 philosophy = 14 rules total.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@molecule-ai
molecule-ai Bot force-pushed the feat/credential-helper-auto-refresh branch from c97d7c6 to 122032e Compare April 23, 2026 07:51
@molecule-ai molecule-ai Bot closed this Apr 23, 2026
HongmingWang-Rabbit added a commit that referenced this pull request Apr 23, 2026
Bot commit 66ea0b6 introduced a malformed patch:
- handleLocalConnect opened with `{` but had no body
- Duplicate HandleConnect declared inside handleLocalConnect's scope
- Result: `internal/handlers/terminal.go:90:57: syntax error: unexpected {`
- Staging Platform(Go) build broken since 2026-04-22 15:30 UTC,
  blocking every Go PR targeting staging (incl. #1746 credential helper).

Fix:
- Hoist `canCommunicateCheck` package var above HandleConnect
- Merge the KI-005 auth check INTO the dispatcher HandleConnect
  (now guards both local AND remote paths — strictly stronger than
   the bot's intent which only would have covered the local path)
- Restore handleLocalConnect to its original body
- Drop redundant `targetID := c.Param("id")` and `workspaceID := targetID`
  (use the parameter name directly)

terminal_test.go is unchanged — it stubs `canCommunicateCheck` and calls
HandleConnect, both of which keep the same signatures.

Class fix tracked separately: bot PRs must `go build ./...` before commit.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@molecule-ai
molecule-ai Bot deleted the feat/credential-helper-auto-refresh branch May 20, 2026 06:21
HongmingWang-Rabbit pushed a commit that referenced this pull request Jun 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant