molecule-ai/molecule-core
53
0
Fork 2
Code Issues 184 Pull Requests 43 Actions 11 Packages Projects Releases Wiki Activity

cleanup: non-blocking fast-follow items from session reviews (CR2 RCs on already-merged PRs) #2921

New Issue
Closed
opened 2026-06-15 06:52:03 +00:00 by agent-dev-b · 0 comments
agent-dev-b commented 2026-06-15 06:52:03 +00:00
Member
Copy Link
Copy Source

cleanup: non-blocking fast-follow items from session reviews (CR2 RCs on already-merged PRs)

These items are non-blocking fast-follow review findings that CR2 flagged on
already-MERGED PRs. They're captured here so they're not lost; fixing them
is separate future work (no PR is required for THIS filing).

#2907 (SSRF) — registry.go

File: workspace-server/internal/registry/registry.go:432 (the Register
ingress — attacker-writable).

  • validate Register ingress with isSafeURL for defense-in-depth
    (the existing allowlist is the primary control; the validator is a
    belt-and-suspenders against allowlist drift).
  • TOCTOU / DNS-rebinding: pin the validated IP and dial it, so a
    DNS flip between validation and connect can't redirect the request to
    a different host.
  • Confirm strict-mode loopback / RFC-1918 block doesn't break
    self-hosted-prod transcript-to-loopback outside MOLECULE_ENV=dev. The
    strict block is the right default; verify the prod path is still
    functional under the strictness.

#2892 (memory redaction) — memories.go

File: workspace-server/internal/handlers/memories.go.

  • Add gho_ to the GitHub token set — current redaction regexp
    set is {ghp_, gho_, ghu_, ghs_, ghr_} missing gho_ (the real GitHub
    OAuth user token). Add it.
  • Fix the ghs_ label — ghs_ is GitHub App server-to-server token;
    the comment currently calls it GITHUB_OAUTH which is wrong. The real
    OAuth token is gho_. Correct both the label string and the comment.
  • Add AWS-pattern parity test — analogous to the
    "AKIA"+repeat("Q",16) test for the existing AWS_ACCESS_KEY_ID
    redaction. Verify the regexp catches a valid 16-char suffix on a
    AKIA prefix and doesn't false-positive on AKID (which is
    actually a real AWS access-key prefix in some docs; verify against the
    current AWS docs).
  • (optional scope) Slack (xox[abprs]-), Stripe (sk_live_ /
    sk_test_ / rk_live_), Google (AIza) prefixes. NOT in scope of
    this filing — listed for future consideration only.

#2896 (canvas topology) — canvas hydration path

File: canvas/src/components/canvas/... (the hydration SUCCESS path).

  • Clear hydrationError on the hydrate SUCCESS path — the current
    code sets {nodes, edges, hydrationError: <new value>} only on the
    error branch. On SUCCESS the error state from a prior failed hydrate
    can stick after in-place workspace re-hydration, leaving the user
    looking at a stale error banner. Set {nodes, edges, hydrationError: null}
    on the success branch.
  • console.error(err) in the catch block for observability — the
    error currently flows into the hydration-error UI but isn't logged, so
    support can't see the underlying failure in the browser console.

#2887 (canvas) — canSave scope

File: canvas/src/components/canvas/... (the canSave selector).

  • Scope canSave's model-block to the model action only (or use
    an inline-warning instead of a hard block) — the current
    model-not-resolved blocker may be over-broad and prevents unrelated
    config edits (template, MCP, etc.) from being saved. Goal: the
    model-not-resolved state should block only the model-changing action;
    other save actions should still work.
  • Add the "unresolved-model + unrelated-dirty → blocked" edge test
    to nail down the new behavior (and prevent regressions): when the
    model is unresolved but other config fields are dirty, the unrelated
    save action must still go through.

Notes

  • All four items are NON-BLOCKING on already-merged code; fixing them is
    separate future work.
  • Owner of this filing: agent-dev-b (me, per PM dispatch
    bf78ffdf-42c0-444c-96df-824014c2a12f).
  • The PR-pair for #30 concierge de-hardcode is more urgent than this
    filing (per PM: "DROP the instant the driver's #2919 arch-pick
    arrives — the Dockerfile.platform-agent impl is your real priority the
    moment that lands; this is just to use idle time, not to delay that").
  • The gho_ addition + ghs_ label-fix in #2892 is the only one that
    might warrant an urgent: or tier:high label once the rest of #30
    is merged (a real-OAuth-token not being redacted is a security
    finding, not just a hygiene one).
# cleanup: non-blocking fast-follow items from session reviews (CR2 RCs on already-merged PRs) These items are non-blocking fast-follow review findings that CR2 flagged on already-MERGED PRs. They're captured here so they're not lost; fixing them is separate future work (no PR is required for THIS filing). ## #2907 (SSRF) — registry.go File: `workspace-server/internal/registry/registry.go:432` (the `Register` ingress — attacker-writable). - [ ] **validate Register ingress with `isSafeURL`** for defense-in-depth (the existing allowlist is the primary control; the validator is a belt-and-suspenders against allowlist drift). - [ ] **TOCTOU / DNS-rebinding**: pin the validated IP and dial it, so a DNS flip between validation and connect can't redirect the request to a different host. - [ ] **Confirm strict-mode loopback / RFC-1918 block** doesn't break self-hosted-prod transcript-to-loopback outside `MOLECULE_ENV=dev`. The strict block is the right default; verify the prod path is still functional under the strictness. ## #2892 (memory redaction) — memories.go File: `workspace-server/internal/handlers/memories.go`. - [ ] **Add `gho_` to the GitHub token set** — current redaction regexp set is `{ghp_, gho_, ghu_, ghs_, ghr_}` missing `gho_` (the real GitHub OAuth user token). Add it. - [ ] **Fix the `ghs_` label** — `ghs_` is GitHub App server-to-server token; the comment currently calls it `GITHUB_OAUTH` which is wrong. The real OAuth token is `gho_`. Correct both the label string and the comment. - [ ] **Add AWS-pattern parity test** — analogous to the `"AKIA"+repeat("Q",16)` test for the existing `AWS_ACCESS_KEY_ID` redaction. Verify the regexp catches a valid 16-char suffix on a `AKIA` prefix and doesn't false-positive on `AKID` (which is actually a real AWS access-key prefix in some docs; verify against the current AWS docs). - [ ] *(optional scope)* Slack (`xox[abprs]-`), Stripe (`sk_live_` / `sk_test_` / `rk_live_`), Google (`AIza`) prefixes. NOT in scope of this filing — listed for future consideration only. ## #2896 (canvas topology) — canvas hydration path File: `canvas/src/components/canvas/...` (the hydration SUCCESS path). - [ ] **Clear `hydrationError` on the hydrate SUCCESS path** — the current code sets `{nodes, edges, hydrationError: <new value>}` only on the error branch. On SUCCESS the error state from a prior failed hydrate can stick after in-place workspace re-hydration, leaving the user looking at a stale error banner. Set `{nodes, edges, hydrationError: null}` on the success branch. - [ ] **`console.error(err)` in the `catch` block** for observability — the error currently flows into the hydration-error UI but isn't logged, so support can't see the underlying failure in the browser console. ## #2887 (canvas) — `canSave` scope File: `canvas/src/components/canvas/...` (the `canSave` selector). - [ ] **Scope `canSave`'s model-block to the model action only** (or use an inline-warning instead of a hard block) — the current model-not-resolved blocker may be over-broad and prevents unrelated config edits (template, MCP, etc.) from being saved. Goal: the model-not-resolved state should block only the model-changing action; other save actions should still work. - [ ] **Add the "unresolved-model + unrelated-dirty → blocked" edge test** to nail down the new behavior (and prevent regressions): when the model is unresolved but other config fields are dirty, the unrelated save action must still go through. --- ## Notes - All four items are NON-BLOCKING on already-merged code; fixing them is separate future work. - Owner of this filing: `agent-dev-b` (me, per PM dispatch `bf78ffdf-42c0-444c-96df-824014c2a12f`). - The PR-pair for #30 concierge de-hardcode is more urgent than this filing (per PM: "DROP the instant the driver's #2919 arch-pick arrives — the Dockerfile.platform-agent impl is your real priority the moment that lands; this is just to use idle time, not to delay that"). - The `gho_` addition + `ghs_` label-fix in #2892 is the only one that might warrant an `urgent:` or `tier:high` label once the rest of #30 is merged (a real-OAuth-token not being redacted is a security finding, not just a hygiene one).
agent-dev-b self-assigned this 2026-06-15 06:52:03 +00:00
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
area/ci
CI/CD pipeline issues
 do-not-auto-merge
Opt out of autonomous merge-queue merging
 do-not-merge
hold from auto-merge (design review in progress)
 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
 wip
Work in progress — do not auto-merge
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
agent-dev-a agent-dev-b agent-pm agent-researcher agent-reviewer agent-reviewer-1 agent-reviewer-cr2 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) godwin hongming hongming-ceo-delegated hongming-codex-laptop hongming-kimi-laptop hongming-pc2 hongming-personal 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 molecule-code-reviewer plugin-dev (Molecule AI · plugin-dev) pm publish-runtime-bot pypi-publisher (Molecule AI PyPI Publisher (RFC#596)) 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 agent-dev-b
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: molecule-ai/molecule-core#2921
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?