External/pmg
1
0
Fork 0
mirror of https://github.com/safedep/pmg.git synced 2026-08-03 07:24:09 +02:00
Code Issues Packages Projects Releases Wiki Activity

refactor: move proxy block message rendering to presentation layer (#364)

Browse Source 
* refactor: move proxy block message rendering to presentation layer

* refactor: introduce ui.ProxyPresenter with injected advisory source

* feat: friendly ecosystem labels in proxy block messages

* test: cover ecosystemLabel derivation

* fix: address review feedback on block context assertions and empty reference line
This commit is contained in:
Sahil Bansal
2026-07-10 17:42:26 +05:30
committed by GitHub
parent 2f1c348a06
commit 31e51b10dd
12 changed files with 358 additions and 97 deletions
Download Patch File Download Diff File Expand all files Collapse all files
proxy/interceptor.go
+37 -1
View File
@@ -4,6 +4,8 @@ import (
"net/http"
"net/url"
"time"
packagev1 "buf.build/gen/go/safedep/api/protocolbuffers/go/safedep/messages/package/v1"
)
// ResponseAction determines how the proxy should handle a request
@@ -41,12 +43,46 @@ type RequestContext struct {
Data map[string]interface{}
}
// BlockReason identifies why an interceptor blocked a request
type BlockReason int
const (
BlockReasonNone BlockReason = iota
BlockReasonMalware
BlockReasonUserDeclined
BlockReasonConfirmationFailed
BlockReasonDependencyCooldown
)
// BlockContext carries the structured facts of a block decision so a
// presentation layer can render the user-facing message. Interceptors
// populate it instead of composing message text themselves.
type BlockContext struct {
Ecosystem packagev1.Ecosystem
PackageName string
PackageVersion string
// For BlockReasonMalware and BlockReasonUserDeclined
MalwareSummary string
MalwareReferenceURL string
// For BlockReasonDependencyCooldown
CooldownDays int
CooldownDaysAgo int
CooldownDaysLeft int
}
// InterceptorResponse defines how the proxy should handle the request
type InterceptorResponse struct {
// Action to take
Action ResponseAction
// For Action = Block: error message to return
// For Action = Block: why and what was blocked. The proxy renders the
// response body from these via ProxyConfig.BlockMessageRenderer.
BlockReason BlockReason
BlockContext *BlockContext
// BlockMessage overrides the rendered message when non-empty
BlockMessage string
BlockCode int
proxy/interceptors/base_registry.go
+28 -30
View File
@@ -101,15 +101,6 @@ func (b *baseRegistryInterceptor) fastAllow(
return nil, false
}
// appendAdvisoryMessage appends the org-configured advisory_message, when set,
// to a block message body.
func appendAdvisoryMessage(message, advisory string) string {
if advisory == "" {
return message
}
return message + "\n\n" + advisory
}
// analyzePackage analyzes a package using the configured analyzer with caching
// This method is ecosystem-agnostic and can be used by any registry interceptor
func (b *baseRegistryInterceptor) analyzePackage(
@@ -203,16 +194,17 @@ func (b *baseRegistryInterceptor) handleAnalysisResult(
b.statsCollector.RecordBlocked(result)
}
message := appendAdvisoryMessage(fmt.Sprintf("Malicious package blocked: %s/%s@%s\n\nReason: %s\n\nReference: %s",
ecosystem.String(),
packageName, packageVersion,
result.Summary,
result.ReferenceURL), config.Get().Config.AdvisoryMessage)
return &proxy.InterceptorResponse{
Action: proxy.ActionBlock,
BlockCode: http.StatusForbidden,
BlockMessage: message,
Action: proxy.ActionBlock,
BlockCode: http.StatusForbidden,
BlockReason: proxy.BlockReasonMalware,
BlockContext: &proxy.BlockContext{
Ecosystem: ecosystem,
PackageName: packageName,
PackageVersion: packageVersion,
MalwareSummary: result.Summary,
MalwareReferenceURL: result.ReferenceURL,
},
}, nil
case analyzer.ActionConfirm:
@@ -227,9 +219,14 @@ func (b *baseRegistryInterceptor) handleAnalysisResult(
}
return &proxy.InterceptorResponse{
Action: proxy.ActionBlock,
BlockCode: http.StatusForbidden,
BlockMessage: fmt.Sprintf("Failed to get user confirmation for suspicious package %s/%s@%s", ecosystem.String(), packageName, packageVersion),
Action: proxy.ActionBlock,
BlockCode: http.StatusForbidden,
BlockReason: proxy.BlockReasonConfirmationFailed,
BlockContext: &proxy.BlockContext{
Ecosystem: ecosystem,
PackageName: packageName,
PackageVersion: packageVersion,
},
}, nil
}
@@ -242,16 +239,17 @@ func (b *baseRegistryInterceptor) handleAnalysisResult(
b.statsCollector.RecordUserCancelled(result)
}
message := appendAdvisoryMessage(fmt.Sprintf("Installation blocked by user: %s/%s@%s\n\nReason: %s\n\nReference: %s",
ecosystem.String(),
packageName, packageVersion,
result.Summary,
result.ReferenceURL), config.Get().Config.AdvisoryMessage)
return &proxy.InterceptorResponse{
Action: proxy.ActionBlock,
BlockCode: http.StatusForbidden,
BlockMessage: message,
Action: proxy.ActionBlock,
BlockCode: http.StatusForbidden,
BlockReason: proxy.BlockReasonUserDeclined,
BlockContext: &proxy.BlockContext{
Ecosystem: ecosystem,
PackageName: packageName,
PackageVersion: packageVersion,
MalwareSummary: result.Summary,
MalwareReferenceURL: result.ReferenceURL,
},
}, nil
}
proxy/interceptors/base_registry_test.go
+48 -58
View File
@@ -62,15 +62,15 @@ func TestFastAllow_InsecureReturnsAllow(t *testing.T) {
func TestBaseRegistryInterceptor_HandleAnalysisResult(t *testing.T) {
tests := []struct {
name string
ecosystem packagev1.Ecosystem
packageName string
packageVersion string
analysisResult *analyzer.PackageVersionAnalysisResult
userConfirms bool
expectedAction proxy.ResponseAction
expectedBlockCode int
expectBlockMessage bool
name string
ecosystem packagev1.Ecosystem
packageName string
packageVersion string
analysisResult *analyzer.PackageVersionAnalysisResult
userConfirms bool
expectedAction proxy.ResponseAction
expectedBlockCode int
expectedBlockReason proxy.BlockReason
}{
{
name: "ActionBlock - malicious package",
@@ -82,9 +82,9 @@ func TestBaseRegistryInterceptor_HandleAnalysisResult(t *testing.T) {
Summary: "Contains known malware",
ReferenceURL: "https://example.com/malware-report",
},
expectedAction: proxy.ActionBlock,
expectedBlockCode: http.StatusForbidden,
expectBlockMessage: true,
expectedAction: proxy.ActionBlock,
expectedBlockCode: http.StatusForbidden,
expectedBlockReason: proxy.BlockReasonMalware,
},
{
name: "ActionConfirm - user confirms installation",
@@ -96,10 +96,10 @@ func TestBaseRegistryInterceptor_HandleAnalysisResult(t *testing.T) {
Summary: "Suspicious behavior detected",
ReferenceURL: "https://example.com/suspicious-report",
},
userConfirms: true,
expectedAction: proxy.ActionAllow,
expectedBlockCode: 0,
expectBlockMessage: false,
userConfirms: true,
expectedAction: proxy.ActionAllow,
expectedBlockCode: 0,
expectedBlockReason: proxy.BlockReasonNone,
},
{
name: "ActionConfirm - user declines installation",
@@ -111,10 +111,10 @@ func TestBaseRegistryInterceptor_HandleAnalysisResult(t *testing.T) {
Summary: "Suspicious behavior detected",
ReferenceURL: "https://example.com/suspicious-report",
},
userConfirms: false,
expectedAction: proxy.ActionBlock,
expectedBlockCode: http.StatusForbidden,
expectBlockMessage: true,
userConfirms: false,
expectedAction: proxy.ActionBlock,
expectedBlockCode: http.StatusForbidden,
expectedBlockReason: proxy.BlockReasonUserDeclined,
},
// Note: Timeout test case is skipped as it would require waiting 5 minutes
// The timeout behavior is covered by the implementation but not tested here
@@ -129,9 +129,9 @@ func TestBaseRegistryInterceptor_HandleAnalysisResult(t *testing.T) {
Summary: "Package is safe",
ReferenceURL: "https://example.com/safe-report",
},
expectedAction: proxy.ActionAllow,
expectedBlockCode: 0,
expectBlockMessage: false,
expectedAction: proxy.ActionAllow,
expectedBlockCode: 0,
expectedBlockReason: proxy.BlockReasonNone,
},
{
name: "ActionUnknown - default to allow",
@@ -143,9 +143,9 @@ func TestBaseRegistryInterceptor_HandleAnalysisResult(t *testing.T) {
Summary: "Unknown action",
ReferenceURL: "https://example.com/unknown-report",
},
expectedAction: proxy.ActionAllow,
expectedBlockCode: 0,
expectBlockMessage: false,
expectedAction: proxy.ActionAllow,
expectedBlockCode: 0,
expectedBlockReason: proxy.BlockReasonNone,
},
{
name: "ActionBlock - pypi ecosystem",
@@ -157,9 +157,9 @@ func TestBaseRegistryInterceptor_HandleAnalysisResult(t *testing.T) {
Summary: "Malicious PyPI package",
ReferenceURL: "https://example.com/pypi-malware",
},
expectedAction: proxy.ActionBlock,
expectedBlockCode: http.StatusForbidden,
expectBlockMessage: true,
expectedAction: proxy.ActionBlock,
expectedBlockCode: http.StatusForbidden,
expectedBlockReason: proxy.BlockReasonMalware,
},
}
@@ -200,35 +200,25 @@ func TestBaseRegistryInterceptor_HandleAnalysisResult(t *testing.T) {
assert.NoError(t, err)
assert.Equal(t, tt.expectedAction, response.Action)
assert.Equal(t, tt.expectedBlockCode, response.BlockCode)
assert.Equal(t, tt.expectBlockMessage, response.BlockMessage != "")
assert.Equal(t, tt.expectedBlockReason, response.BlockReason)
assert.Empty(t, response.BlockMessage)
switch tt.expectedBlockReason {
case proxy.BlockReasonNone:
assert.Nil(t, response.BlockContext)
case proxy.BlockReasonMalware, proxy.BlockReasonUserDeclined:
require.NotNil(t, response.BlockContext)
assert.Equal(t, tt.ecosystem, response.BlockContext.Ecosystem)
assert.Equal(t, tt.packageName, response.BlockContext.PackageName)
assert.Equal(t, tt.packageVersion, response.BlockContext.PackageVersion)
assert.Equal(t, tt.analysisResult.Summary, response.BlockContext.MalwareSummary)
assert.Equal(t, tt.analysisResult.ReferenceURL, response.BlockContext.MalwareReferenceURL)
default:
require.NotNil(t, response.BlockContext)
assert.Equal(t, tt.ecosystem, response.BlockContext.Ecosystem)
assert.Equal(t, tt.packageName, response.BlockContext.PackageName)
assert.Equal(t, tt.packageVersion, response.BlockContext.PackageVersion)
}
})
}
}
func TestAppendAdvisoryMessage(t *testing.T) {
assert.Equal(t, "base", appendAdvisoryMessage("base", ""))
assert.Equal(t, "base\n\ncustom", appendAdvisoryMessage("base", "custom"))
}
func TestHandleAnalysisResultBlockCarriesAdvisoryMessage(t *testing.T) {
origMsg := pmgconfig.Get().Config.AdvisoryMessage
pmgconfig.Get().Config.AdvisoryMessage = "Contact #security-help"
t.Cleanup(func() { pmgconfig.Get().Config.AdvisoryMessage = origMsg })
b := &baseRegistryInterceptor{}
ctx := makeTestRequestContext("https://registry.npmjs.org/evil/-/evil-1.0.0.tgz")
result := &analyzer.PackageVersionAnalysisResult{
PackageVersion: &packagev1.PackageVersion{
Package: &packagev1.Package{Name: "evil", Ecosystem: packagev1.Ecosystem_ECOSYSTEM_NPM},
Version: "1.0.0",
},
Action: analyzer.ActionBlock,
Summary: "verified malware",
}
resp, err := b.handleAnalysisResult(ctx, packagev1.Ecosystem_ECOSYSTEM_NPM, "evil", "1.0.0", result)
require.NoError(t, err)
assert.Equal(t, proxy.ActionBlock, resp.Action)
assert.Contains(t, resp.BlockMessage, "Contact #security-help")
}
proxy/interceptors/go_cooldown.go
+11 -8
View File
@@ -126,15 +126,18 @@ func (h *goCooldownHandler) CheckZipDownload(ctx *proxy.RequestContext, baseURL,
pv.SetVersion(version)
audit.LogDependencyCooldown(pv, publishTime, cooldownDays, daysAgo, daysLeft)
message := appendAdvisoryMessage(
fmt.Sprintf("Package blocked by dependency cooldown: GO/%s@%s\n\nPublished %d day(s) ago; cooldown window is %d day(s) (%d remaining).",
module, version, daysAgo, cooldownDays, daysLeft),
pmgconfig.Get().Config.AdvisoryMessage)
return &proxy.InterceptorResponse{
Action: proxy.ActionBlock,
BlockCode: http.StatusForbidden,
BlockMessage: message,
Action: proxy.ActionBlock,
BlockCode: http.StatusForbidden,
BlockReason: proxy.BlockReasonDependencyCooldown,
BlockContext: &proxy.BlockContext{
Ecosystem: packagev1.Ecosystem_ECOSYSTEM_GO,
PackageName: module,
PackageVersion: version,
CooldownDays: cooldownDays,
CooldownDaysAgo: daysAgo,
CooldownDaysLeft: daysLeft,
},
}, true
}
proxy/interceptors/go_cooldown_test.go
+10
View File
@@ -7,6 +7,7 @@ import (
"testing"
"time"
packagev1 "buf.build/gen/go/safedep/api/protocolbuffers/go/safedep/messages/package/v1"
"github.com/safedep/pmg/proxy"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
@@ -39,6 +40,15 @@ func TestGoCooldownCheckZipDownloadSideFetch(t *testing.T) {
require.True(t, handled)
assert.Equal(t, proxy.ActionBlock, resp.Action)
assert.Equal(t, http.StatusForbidden, resp.BlockCode)
assert.Equal(t, proxy.BlockReasonDependencyCooldown, resp.BlockReason)
require.NotNil(t, resp.BlockContext)
assert.Equal(t, packagev1.Ecosystem_ECOSYSTEM_GO, resp.BlockContext.Ecosystem)
assert.Equal(t, "example.com/fresh", resp.BlockContext.PackageName)
assert.Equal(t, "v1.1.0", resp.BlockContext.PackageVersion)
assert.Equal(t, 7, resp.BlockContext.CooldownDays)
assert.Equal(t, 1, resp.BlockContext.CooldownDaysAgo)
assert.Equal(t, 6, resp.BlockContext.CooldownDaysLeft)
})
t.Run("fails open when the out-of-band fetch fails", func(t *testing.T) {
proxy/proxy.go
+8
View File
@@ -62,6 +62,11 @@ type ProxyConfig struct {
// Interceptors
Interceptors []Interceptor
// BlockMessageRenderer composes the response body for blocked requests
// from the interceptor's structured block decision. nil falls back to
// the generic block message.
BlockMessageRenderer func(BlockReason, *BlockContext) string
// Other configuration
EnableMITM bool
RequestTimeout time.Duration
@@ -561,6 +566,9 @@ func (ps *proxyServer) registerHandlers() {
}
message := resp.BlockMessage
if message == "" && ps.config.BlockMessageRenderer != nil {
message = ps.config.BlockMessageRenderer(resp.BlockReason, resp.BlockContext)
}
if message == "" {
message = "Blocked by proxy interceptor"
}