Skip to content

[release/2.8] Backport CVE-2026-33540 and CVE-2026-35172 fixes#1

Open
yuzichen12123 wants to merge 4 commits into
release/2.8from
alauda-release-2.8
Open

[release/2.8] Backport CVE-2026-33540 and CVE-2026-35172 fixes#1
yuzichen12123 wants to merge 4 commits into
release/2.8from
alauda-release-2.8

Conversation

@yuzichen12123
Copy link
Copy Markdown
Collaborator

Summary\n- backport proxy realm trust-boundary fix (CVE-2026-33540)\n- backport redis repo-scoped descriptor revocation fix (CVE-2026-35172)\n- add redis regression test for repo-scoped clear behavior on v2\n\n## Notes\n- keep v2-compatible implementation and test strategy\n- do not pull v3-only test dependencies into release/2.8\n

@alaudabot
Copy link
Copy Markdown

alaudabot commented Apr 22, 2026

🤖 AI Code Review

Property Value
Model opencode/minimax-m2.5-free
Style strict
Issues Found 1
Config Source centralized
Profile ❌ Not Found
Personalized Prompt ❌ No
Prompt Path .github/review/profiles/alaudadevops/distribution/pr-review.md
Alauda Skills ✅ base-authoring, base-m365, base-skill-setup, builders-component-knowledge, builders-confluence, builders-jira, builders-publish-errata, builders-story-split, devops-autodns, devops-bulk-string-replace, devops-candidate-version-supervisor, devops-connectors-acceptance-test, devops-connectors-explore, devops-connectors-poc-case, devops-connectors-review, devops-connectors-unit-test, devops-connectors-write-user-docs, devops-docker-keyword-analysis, devops-gen-advanced-form-descriptors, devops-go-vuln-fix, devops-knowledge-adoption, devops-refresh-alauda-tags, devops-sync-alauda-github-releases, devops-tekton-dynamic-form-optimizer, devops-tekton-operator-task-e2e, devops-tekton-task-generator, devops-tekton-task-overview-template, devops-tekton-task-version-upgrade, devops-tekton-upgrade-notes, devops-tool-report-troubleshoot, devops-ui-e2e-code-audit, devops-ui-e2e-fix-base-on-report, devops-ui-e2e-regression-and-fix, devops-ui-generate-e2e-from-feature, devops-ui-pre-setup, devops-update-containerfile-digests, devops-upgrade-go
Reviewed at 2026-04-22 04:29:51 UTC

Summary

This PR backports two security fixes: CVE-2026-33540 (proxy realm trust-boundary) and CVE-2026-35172 (redis repo-scoped descriptor revocation). The changes introduce a FilteringManager to validate bearer auth realms, add domain-based realm filtering to the proxy auth, and fix the Redis Clear method to properly remove all associated keys in an atomic transaction.

Review Statistics

Category Count
Critical Issues 1
Warnings 1
Suggestions 0
Files Reviewed 8

Critical Issues

  • [registry/storage/cache/redis/redis.go:218-231] (bug/connection-leak): The MULTI/EXEC transaction lacks proper error handling. If SREM, DEL, or HDEL returns an error, the function returns early without calling EXEC or DISCARD, leaving the Redis connection in the MULTI state. This leaks the connection back to the pool in a dirty state.

    // Current (buggy):
    if _, err := conn.Do("SREM", ...); err != nil {
        return err  // BUG: Returns without EXEC, leaking connection
    }
    
    // Fix: Either use EXEC regardless of errors, or use DISCARD on failure
    if _, err := conn.Do("SREM", ...); err != nil {
        conn.Do("DISCARD")  // Clean up transaction state
        return err
    }

Warnings

  • [reference/reference_deprecated.go:122] (refactor/behavior-change): The replacement of reference.SplitHostname(named) with reference.Domain(named), reference.Path(named) is a behavioral change. SplitHostname historically splits the reference into hostname + path remainder, while Domain returns only the domain portion. Ensure this backward-compatible change is tested against the actual function behavior.

Positive Feedback

  • CVE-2026-33540 fix: The realmAllowed() implementation correctly validates that bearer auth realms are from the same registrable domain using publicsuffix.EffectiveTLDPlusOne. The rejection of loopback/localhost realms is a security best practice.
  • CVE-2026-35172 fix: The Clear method now atomically removes repository membership, repository-specific descriptor hash, and upstream descriptor data, preventing stale access after clear operations.
  • FilteringManager pattern: Clean decorator pattern that separates filtering logic from the core Manager interface.
  • Test coverage: Comprehensive tests for both security fixes including same-authority realm acceptance, cross-origin realm rejection, and DockerHub-style auth scenarios.

ℹ️ About this review

This review was automatically generated using the run-actions workflow.

  • Shared prompt: .github/prompts/code-review.md
  • Config source: centralized
  • Profile path: Not Found
  • Profile ref: 32955711ef1af066b0af463dc0cecc5ccc39be41
  • No repository-specific prompt configured
  • Alauda skills: base-authoring, base-m365, base-skill-setup, builders-component-knowledge, builders-confluence, builders-jira, builders-publish-errata, builders-story-split, devops-autodns, devops-bulk-string-replace, devops-candidate-version-supervisor, devops-connectors-acceptance-test, devops-connectors-explore, devops-connectors-poc-case, devops-connectors-review, devops-connectors-unit-test, devops-connectors-write-user-docs, devops-docker-keyword-analysis, devops-gen-advanced-form-descriptors, devops-go-vuln-fix, devops-knowledge-adoption, devops-refresh-alauda-tags, devops-sync-alauda-github-releases, devops-tekton-dynamic-form-optimizer, devops-tekton-operator-task-e2e, devops-tekton-task-generator, devops-tekton-task-overview-template, devops-tekton-task-version-upgrade, devops-tekton-upgrade-notes, devops-tool-report-troubleshoot, devops-ui-e2e-code-audit, devops-ui-e2e-fix-base-on-report, devops-ui-e2e-regression-and-fix, devops-ui-generate-e2e-from-feature, devops-ui-pre-setup, devops-update-containerfile-digests, devops-upgrade-go

1seal and others added 3 commits April 22, 2026 10:36
bind proxy-mode bearer realms to the upstream trust boundary before sending credentials, move challenge filtering behind the auth manager, and use golang.org/x/net/publicsuffix for registrable-domain handling.

(cherry picked from commit f215e51)
Signed-off-by: yuzichen <1062955096@qq.com>
(cherry picked from commit 5cda728)
Signed-off-by: yuzichen <1062955096@qq.com>
Signed-off-by: yuzichen <1062955096@qq.com>
Signed-off-by: yuzichen <1062955096@qq.com>
// Remove the repository-scoped membership and descriptor metadata in one transaction.
if _, err := conn.Do("MULTI"); err != nil {
return err
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Critical (bug/connection-leak): This returns early without calling EXEC or DISCARD, leaving the Redis connection in MULTI state. The connection will be returned to the pool in a dirty state.

Suggested change
}
if _, err := conn.Do("SREM", rsrbds.repositoryBlobSetKey(rsrbds.repo), dgst); err != nil {
conn.Do("DISCARD")
return err
}

}
if _, err := conn.Do("SREM", rsrbds.repositoryBlobSetKey(rsrbds.repo), dgst); err != nil {
return err
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Critical (bug/connection-leak): Same issue - returns early without cleaning up the MULTI transaction state.

}
if _, err := conn.Do("DEL", rsrbds.blobDescriptorHashKey(dgst)); err != nil {
return err
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Critical (bug/connection-leak): Same issue - returns early without cleaning up the MULTI transaction state.

// Deprecated: Use [reference.Domain] or [reference.Path].
func SplitHostname(named reference.Named) (string, string) {
return reference.SplitHostname(named)
return reference.Domain(named), reference.Path(named)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Warning (refactor/behavior-change): This replaces SplitHostname with Domain + Path. These may have different semantics - please verify backward compatibility.

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.

3 participants