mirror of
https://github.com/Dvorinka/Primora.git
synced 2026-10-28 15:51:23 +00:00
security: harden agent surface — vault_exec allow-list, SSRF, error hygiene
Stress-test remediation across MCP, CLI, and backend:
- primora_vault_exec rewritten as an allow-list (was deny-list):
colon-form single-token commands bypassed it, leaking every vault
secret; verbatim env-name injection allowed NODE_OPTIONS=--require
host RCE. Now fixed read-mostly command set, arg/path deny regex,
strict env-name grammar, loader-hook name blocklist. All prior
bypasses verified denied; allowed commands still inject.
- SSRF: webhook/job URL validation now blocks cloud metadata IPs
(169.254.169.254, fd00:ec2::254) inside the link-local allowance
and non-canonical IP encodings (decimal/hex bare hosts) smuggled
as "docker service names".
- Path params encodeURIComponent in all client consumers — encodeURI
left ?#& intact, smuggling query segments through {name} params.
- primora_upload_object strict-validates contentBase64 — Buffer.from
silently dropped invalid chars.
- MCP no-creds startup degrades instead of crashing mid-handshake.
- GET /api/v1/context — API-key self-discovery; fixes the CLI dead
end (login --api-key validates the key + seeds org/project,
use/context/whoami work under key auth).
- CLI: unknown commands / bare group exit 1 with hints; mri numeric
coercion reversed via raw-argv restore (007 stays "007");
login --json; orgs:rm / projects:rm / buckets:rm behind --yes;
fetch-failed errors name the cause.
- Error hygiene: pgx.ErrNoRows -> 404, pgconn codes -> clean
400/409 without SQLSTATE leaks, validator failures formatted,
services.InputError maps input faults to 400 (webhook URL/event
errors were 500s), presign on local driver -> 400.
- Webhook events accept wildcard patterns (*, ns.*, custom.*)
matched via SQL LIKE — client-published events can be webhooked.
Generated with [Devin](https://devin.ai)
Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
This commit is contained in:
1 parent
3b90dae767
commit
2d6135121e
28 files changed
+705
-32
No files matched your search
@@ -147,3 +147,7 @@ WHERE p.organization_id = $1;
|
||||
DELETE FROM core.organizations
|
||||
WHERE id = $1
|
||||
RETURNING *;
|
||||
|
||||
-- name: GetOrganizationByID :one
|
||||
SELECT * FROM core.organizations
|
||||
WHERE id = $1;
|
||||
@@ -11,7 +11,10 @@ WHERE id = $1;
|
||||
SELECT * FROM core.webhooks
|
||||
WHERE project_id = $1
|
||||
AND enabled
|
||||
AND (cardinality(events) = 0 OR $2::text = ANY(events));
|
||||
AND (cardinality(events) = 0 OR EXISTS (
|
||||
SELECT 1 FROM unnest(events) AS e
|
||||
WHERE e = '*' OR $2::text LIKE REPLACE(e, '*', '%')
|
||||
));
|
||||
|
||||
-- name: CreateWebhook :one
|
||||
INSERT INTO core.webhooks (
|
||||
|
||||
@@ -241,6 +241,23 @@ func (q *Queries) GetInvitationByTokenHash(ctx context.Context, tokenHash string
|
||||
return i, err
|
||||
}
|
||||
|
||||
const getOrganizationByID = `-- name: GetOrganizationByID :one
|
||||
SELECT id, slug, name, created_at FROM core.organizations
|
||||
WHERE id = $1
|
||||
`
|
||||
|
||||
func (q *Queries) GetOrganizationByID(ctx context.Context, id uuid.UUID) (CoreOrganization, error) {
|
||||
row := q.db.QueryRow(ctx, getOrganizationByID, id)
|
||||
var i CoreOrganization
|
||||
err := row.Scan(
|
||||
&i.ID,
|
||||
&i.Slug,
|
||||
&i.Name,
|
||||
&i.CreatedAt,
|
||||
)
|
||||
return i, err
|
||||
}
|
||||
|
||||
const getOrganizationMembership = `-- name: GetOrganizationMembership :one
|
||||
SELECT
|
||||
om.id, om.organization_id, om.user_id, om.role, om.created_at,
|
||||
|
||||
@@ -76,6 +76,7 @@ type Querier interface {
|
||||
GetIntegrationByID(ctx context.Context, id uuid.UUID) (CoreIntegration, error)
|
||||
GetInvitationByIDForOrganization(ctx context.Context, arg GetInvitationByIDForOrganizationParams) (CoreProjectInvitation, error)
|
||||
GetInvitationByTokenHash(ctx context.Context, tokenHash string) (CoreProjectInvitation, error)
|
||||
GetOrganizationByID(ctx context.Context, id uuid.UUID) (CoreOrganization, error)
|
||||
GetOrganizationMembership(ctx context.Context, arg GetOrganizationMembershipParams) (GetOrganizationMembershipRow, error)
|
||||
GetProjectByID(ctx context.Context, id uuid.UUID) (CoreProject, error)
|
||||
GetProjectMembership(ctx context.Context, arg GetProjectMembershipParams) (GetProjectMembershipRow, error)
|
||||
|
||||
@@ -291,7 +291,10 @@ const listWebhooksForEvent = `-- name: ListWebhooksForEvent :many
|
||||
SELECT id, project_id, url, secret, events, enabled, created_by_user_id, created_at, updated_at FROM core.webhooks
|
||||
WHERE project_id = $1
|
||||
AND enabled
|
||||
AND (cardinality(events) = 0 OR $2::text = ANY(events))
|
||||
AND (cardinality(events) = 0 OR EXISTS (
|
||||
SELECT 1 FROM unnest(events) AS e
|
||||
WHERE e = '*' OR $2::text LIKE REPLACE(e, '*', '%')
|
||||
))
|
||||
`
|
||||
|
||||
type ListWebhooksForEventParams struct {
|
||||
|
||||
@@ -2,6 +2,7 @@ package handlers
|
||||
|
||||
import (
|
||||
"errors"
|
||||
"fmt"
|
||||
"io"
|
||||
"net/http"
|
||||
"strconv"
|
||||
@@ -11,6 +12,7 @@ import (
|
||||
"github.com/go-playground/validator/v10"
|
||||
"github.com/google/uuid"
|
||||
"github.com/jackc/pgx/v5"
|
||||
"github.com/jackc/pgx/v5/pgconn"
|
||||
|
||||
db "github.com/tdvorak/primora/apps/backend/internal/database/db"
|
||||
"github.com/tdvorak/primora/apps/backend/internal/middleware"
|
||||
@@ -35,6 +37,7 @@ func (h *HTTPHandler) Register(router *gin.Engine) {
|
||||
|
||||
api := router.Group("/api/v1")
|
||||
api.GET("/me", h.me)
|
||||
api.GET("/context", h.actorContext)
|
||||
api.GET("/instance/public", h.instancePublic)
|
||||
api.GET("/instance/settings", h.listInstanceSettings)
|
||||
api.PUT("/instance/settings/:key", h.updateInstanceSetting)
|
||||
@@ -181,6 +184,19 @@ func (h *HTTPHandler) me(c *gin.Context) {
|
||||
c.JSON(http.StatusOK, result)
|
||||
}
|
||||
|
||||
func (h *HTTPHandler) actorContext(c *gin.Context) {
|
||||
actor, ok := middleware.RequireActor(c)
|
||||
if !ok {
|
||||
return
|
||||
}
|
||||
result, err := h.Platform.ActorContext(c.Request.Context(), actor)
|
||||
if err != nil {
|
||||
h.handleError(c, err)
|
||||
return
|
||||
}
|
||||
c.JSON(http.StatusOK, result)
|
||||
}
|
||||
|
||||
func (h *HTTPHandler) bootstrap(c *gin.Context) {
|
||||
actor, ok := middleware.RequireActor(c)
|
||||
if !ok {
|
||||
@@ -921,6 +937,10 @@ func (h *HTTPHandler) presignObject(c *gin.Context) {
|
||||
apperrors.Abort(c, http.StatusNotFound, "not_found", "bucket or object not found")
|
||||
return
|
||||
}
|
||||
if errors.Is(err, services.ErrPresignUnsupported) {
|
||||
apperrors.Abort(c, http.StatusBadRequest, "presign_unsupported", err.Error())
|
||||
return
|
||||
}
|
||||
h.handleError(c, err)
|
||||
return
|
||||
}
|
||||
@@ -1260,17 +1280,58 @@ func (h *HTTPHandler) bindAndValidate(c *gin.Context, target any) bool {
|
||||
return false
|
||||
}
|
||||
if err := h.Validate.Struct(target); err != nil {
|
||||
apperrors.Abort(c, http.StatusBadRequest, "validation_failed", err.Error())
|
||||
var valErrs validator.ValidationErrors
|
||||
if errors.As(err, &valErrs) && len(valErrs) > 0 {
|
||||
apperrors.Abort(c, http.StatusBadRequest, "validation_failed", formatFieldError(valErrs[0]))
|
||||
} else {
|
||||
apperrors.Abort(c, http.StatusBadRequest, "validation_failed", "invalid request body")
|
||||
}
|
||||
return false
|
||||
}
|
||||
return true
|
||||
}
|
||||
|
||||
func formatFieldError(f validator.FieldError) string {
|
||||
if f.Tag() == "required" {
|
||||
return fmt.Sprintf("%s is required", strings.ToLower(f.Field()))
|
||||
}
|
||||
return fmt.Sprintf("%s failed %s validation", strings.ToLower(f.Field()), f.Tag())
|
||||
}
|
||||
|
||||
func (h *HTTPHandler) handleError(c *gin.Context, err error) {
|
||||
switch {
|
||||
case errors.Is(err, io.EOF):
|
||||
apperrors.Abort(c, http.StatusBadRequest, "invalid_body", "request body is empty")
|
||||
case errors.Is(err, pgx.ErrNoRows):
|
||||
apperrors.Abort(c, http.StatusNotFound, "not_found", "resource not found")
|
||||
default:
|
||||
var inputErr *services.InputError
|
||||
if errors.As(err, &inputErr) {
|
||||
apperrors.Abort(c, http.StatusBadRequest, "invalid_request", inputErr.Error())
|
||||
return
|
||||
}
|
||||
var valErrs validator.ValidationErrors
|
||||
if errors.As(err, &valErrs) && len(valErrs) > 0 {
|
||||
apperrors.Abort(c, http.StatusBadRequest, "invalid_request", formatFieldError(valErrs[0]))
|
||||
return
|
||||
}
|
||||
var pgErr *pgconn.PgError
|
||||
if errors.As(err, &pgErr) {
|
||||
// Structured codes — never leak raw SQLSTATE text or constraint names.
|
||||
switch pgErr.Code {
|
||||
case "23505":
|
||||
apperrors.Abort(c, http.StatusConflict, "conflict", "resource already exists")
|
||||
case "23503":
|
||||
apperrors.Abort(c, http.StatusConflict, "conflict", "referenced resource does not exist")
|
||||
case "22001", "54000":
|
||||
apperrors.Abort(c, http.StatusBadRequest, "invalid_request", "value too large")
|
||||
case "22021":
|
||||
apperrors.Abort(c, http.StatusBadRequest, "invalid_request", "input is not valid UTF-8")
|
||||
default:
|
||||
apperrors.Abort(c, http.StatusInternalServerError, "request_failed", "database error")
|
||||
}
|
||||
return
|
||||
}
|
||||
status := http.StatusInternalServerError
|
||||
message := strings.ToLower(err.Error())
|
||||
if strings.Contains(message, "insufficient") || strings.Contains(message, "access denied") {
|
||||
|
||||
@@ -104,4 +104,39 @@ func TestValidateWebhookEvents(t *testing.T) {
|
||||
if _, err := validateWebhookEvents([]string{"bogus.event"}); err == nil {
|
||||
t.Error("expected unknown event rejected")
|
||||
}
|
||||
for _, ok := range [][]string{
|
||||
{"*"},
|
||||
{"document.*"},
|
||||
{"custom.deploy.done"},
|
||||
{"custom.*"},
|
||||
{"custom.deploy.*"},
|
||||
} {
|
||||
if _, err := validateWebhookEvents(ok); err != nil {
|
||||
t.Errorf("expected %v accepted, got %v", ok, err)
|
||||
}
|
||||
}
|
||||
for _, bad := range [][]string{
|
||||
{"banana.*"}, {"custom..*"}, {"custom.UPPER"}, {"document.exploded"},
|
||||
} {
|
||||
if _, err := validateWebhookEvents(bad); err == nil {
|
||||
t.Errorf("expected %v rejected", bad)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestPrivateWebhookHostHardening(t *testing.T) {
|
||||
private := []string{"localhost", "minio", "echo-svc", "host.docker.internal",
|
||||
"10.0.0.4", "172.16.5.5", "192.168.1.10", "fe80::1", "svc.local", "db.internal"}
|
||||
public := []string{"169.254.169.254", "fd00:ec2::254", "2130706433", "0x7f000001",
|
||||
"8.8.8.8", "example.com", "999999999999"}
|
||||
for _, h := range private {
|
||||
if !isPrivateWebhookHost(h) {
|
||||
t.Errorf("expected %q private", h)
|
||||
}
|
||||
}
|
||||
for _, h := range public {
|
||||
if isPrivateWebhookHost(h) {
|
||||
t.Errorf("expected %q public/blocked", h)
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -405,6 +405,51 @@ func (s *PlatformService) PublishProjectEvent(ctx context.Context, actor *models
|
||||
return nil
|
||||
}
|
||||
|
||||
// ActorContext describes the caller's own scope — the self-discovery
|
||||
// endpoint for API keys, which cannot call /me. User actors get just the
|
||||
// type marker; their full view is PlatformSummary.
|
||||
type ActorContext struct {
|
||||
Actor string `json:"actor"`
|
||||
Scopes []string `json:"scopes,omitempty"`
|
||||
KeyPrefix string `json:"key_prefix,omitempty"`
|
||||
Organization *ContextOrgInfo `json:"organization,omitempty"`
|
||||
Project *ContextRef `json:"project,omitempty"`
|
||||
}
|
||||
|
||||
type ContextOrgInfo struct {
|
||||
ContextRef
|
||||
Role string `json:"role,omitempty"`
|
||||
}
|
||||
|
||||
type ContextRef struct {
|
||||
ID uuid.UUID `json:"id"`
|
||||
Slug string `json:"slug"`
|
||||
Name string `json:"name"`
|
||||
}
|
||||
|
||||
func (s *PlatformService) ActorContext(ctx context.Context, actor *models.Actor) (ActorContext, error) {
|
||||
out := ActorContext{Actor: string(actor.Type)}
|
||||
if actor.Type != models.ActorTypeAPIKey {
|
||||
return out, nil
|
||||
}
|
||||
out.Scopes = actor.Scopes
|
||||
out.KeyPrefix = actor.APIKeyPrefix
|
||||
if actor.ProjectID == nil {
|
||||
return out, nil
|
||||
}
|
||||
project, err := s.repo.Queries().GetProjectByID(ctx, *actor.ProjectID)
|
||||
if err != nil {
|
||||
return out, err
|
||||
}
|
||||
out.Project = &ContextRef{ID: project.ID, Slug: project.Slug, Name: project.Name}
|
||||
org, err := s.repo.Queries().GetOrganizationByID(ctx, project.OrganizationID)
|
||||
if err != nil {
|
||||
return out, err
|
||||
}
|
||||
out.Organization = &ContextOrgInfo{ContextRef: ContextRef{ID: org.ID, Slug: org.Slug, Name: org.Name}}
|
||||
return out, nil
|
||||
}
|
||||
|
||||
func (s *PlatformService) Me(ctx context.Context, actor *models.Actor) (PlatformSummary, error) {
|
||||
if actor.UserID == nil {
|
||||
return PlatformSummary{}, errors.New("user actor required")
|
||||
@@ -1479,6 +1524,20 @@ func (s *PlatformService) GetObject(ctx context.Context, actor *models.Actor, bu
|
||||
return object, file, nil
|
||||
}
|
||||
|
||||
// ErrPresignUnsupported — the configured storage driver can't mint signed
|
||||
// URLs (only the S3 driver can). Handlers map it to 400, not 500.
|
||||
var ErrPresignUnsupported = errors.New("presigned URLs require BACKEND_STORAGE_DRIVER=s3")
|
||||
|
||||
// InputError marks a service-layer failure caused by caller input — the
|
||||
// HTTP layer maps it to 400 instead of 500.
|
||||
type InputError struct{ msg string }
|
||||
|
||||
func (e *InputError) Error() string { return e.msg }
|
||||
|
||||
func inputErrorf(format string, args ...any) error {
|
||||
return &InputError{msg: fmt.Sprintf(format, args...)}
|
||||
}
|
||||
|
||||
// PresignObject mints a short-lived direct-to-storage URL for a single
|
||||
// object. Requires the s3 storage driver — the local store has no external
|
||||
// URL surface to sign against.
|
||||
@@ -1507,7 +1566,7 @@ func (s *PlatformService) PresignObject(ctx context.Context, actor *models.Actor
|
||||
}
|
||||
p, ok := s.store.(storage.Presigner)
|
||||
if !ok {
|
||||
return nil, fmt.Errorf("presigned URLs require BACKEND_STORAGE_DRIVER=s3")
|
||||
return nil, ErrPresignUnsupported
|
||||
}
|
||||
ttl := time.Duration(ttlSeconds) * time.Second
|
||||
return p.Presign(ctx, bucketID.String(), objectKey, method, ttl)
|
||||
|
||||
@@ -14,6 +14,7 @@ import (
|
||||
"net"
|
||||
"net/http"
|
||||
"net/url"
|
||||
"regexp"
|
||||
"strings"
|
||||
"sync"
|
||||
"time"
|
||||
@@ -165,7 +166,7 @@ func normalizeWebhookURL(raw string) (string, error) {
|
||||
trimmed := strings.TrimSpace(raw)
|
||||
u, err := url.Parse(trimmed)
|
||||
if err != nil || u.Host == "" {
|
||||
return "", errors.New("webhook url must be an https URL")
|
||||
return "", inputErrorf("webhook url must be an https URL")
|
||||
}
|
||||
if u.Scheme == "https" {
|
||||
return trimmed, nil
|
||||
@@ -173,17 +174,35 @@ func normalizeWebhookURL(raw string) (string, error) {
|
||||
if u.Scheme == "http" && isPrivateWebhookHost(u.Hostname()) {
|
||||
return trimmed, nil
|
||||
}
|
||||
return "", errors.New("webhook url must be https (http allowed only for private/self-hosted targets)")
|
||||
return "", inputErrorf("webhook url must be https (http allowed only for private/self-hosted targets)")
|
||||
}
|
||||
|
||||
var decimalOrHexIPRe = regexp.MustCompile(`^(0x[0-9a-f]+|[0-9]+)$`)
|
||||
|
||||
// metadataAddrs are link-local addresses that serve cloud instance
|
||||
// metadata — "private" by range but a privileged SSRF target.
|
||||
var metadataAddrs = map[string]bool{
|
||||
"169.254.169.254": true, // AWS/GCP/Azure
|
||||
"fd00:ec2::254": true, // AWS IPv6 metadata
|
||||
}
|
||||
|
||||
func isPrivateWebhookHost(host string) bool {
|
||||
h := strings.ToLower(strings.Trim(host, "[]"))
|
||||
if metadataAddrs[h] {
|
||||
return false
|
||||
}
|
||||
if h == "localhost" || h == "host.docker.internal" ||
|
||||
strings.HasSuffix(h, ".local") || strings.HasSuffix(h, ".internal") || strings.HasSuffix(h, ".lan") {
|
||||
return true
|
||||
}
|
||||
if !strings.Contains(h, ".") && !strings.Contains(h, ":") {
|
||||
return true // bare hostname — docker service name, unqualified LAN host
|
||||
// Bare hostname — docker service name, unqualified LAN host. A
|
||||
// pure decimal/hex integer is never a service name: it's a
|
||||
// non-canonical IP encoding (2130706433 = 127.0.0.1, 0x7f000001).
|
||||
if decimalOrHexIPRe.MatchString(h) {
|
||||
return false
|
||||
}
|
||||
return true
|
||||
}
|
||||
ip := net.ParseIP(h)
|
||||
if ip == nil {
|
||||
@@ -192,16 +211,40 @@ func isPrivateWebhookHost(host string) bool {
|
||||
return ip.IsLoopback() || ip.IsPrivate() || ip.IsLinkLocalUnicast()
|
||||
}
|
||||
|
||||
// webhookEventNamespaces are the valid `<ns>.*` subscription prefixes —
|
||||
// derived from the known system event types.
|
||||
func webhookEventNamespaces() map[string]bool {
|
||||
ns := map[string]bool{}
|
||||
for t := range webhookEventTypes {
|
||||
if i := strings.Index(t, "."); i > 0 {
|
||||
ns[t[:i]] = true
|
||||
}
|
||||
}
|
||||
return ns
|
||||
}
|
||||
|
||||
// validateWebhookEvents accepts known system types, `*`, `<ns>.*` namespace
|
||||
// patterns, and `custom.*` wildcard patterns — the same grammar functions
|
||||
// use for event_pattern. Matching happens via SQL LIKE at dispatch.
|
||||
func validateWebhookEvents(events []string) ([]string, error) {
|
||||
out := make([]string, 0, len(events))
|
||||
seen := map[string]bool{}
|
||||
namespaces := webhookEventNamespaces()
|
||||
for _, e := range events {
|
||||
e = strings.TrimSpace(e)
|
||||
if e == "" {
|
||||
continue
|
||||
}
|
||||
if !webhookEventTypes[e] {
|
||||
return nil, fmt.Errorf("unknown event type %q", e)
|
||||
valid := webhookEventTypes[e] || e == "*"
|
||||
if !valid && strings.HasSuffix(e, ".*") {
|
||||
valid = namespaces[strings.TrimSuffix(e, ".*")]
|
||||
}
|
||||
if !valid && strings.HasPrefix(e, "custom.") {
|
||||
// `*` isn't in the type grammar — probe it as a literal.
|
||||
valid = customEventTypeRe.MatchString(strings.ReplaceAll(e, "*", "x"))
|
||||
}
|
||||
if !valid {
|
||||
return nil, inputErrorf("invalid event type %q — use a known type, <namespace>.*, custom.<pattern>, or *", e)
|
||||
}
|
||||
if !seen[e] {
|
||||
seen[e] = true
|
||||
|
||||
@@ -68,6 +68,22 @@ paths:
|
||||
$ref: "#/components/schemas/MeResponse"
|
||||
"401":
|
||||
$ref: "#/components/responses/ErrorResponse"
|
||||
/context:
|
||||
get:
|
||||
tags: [Platform]
|
||||
operationId: getActorContext
|
||||
summary: Caller scope — self-discovery for API keys, which cannot call /me
|
||||
security:
|
||||
- bearerAuth: []
|
||||
responses:
|
||||
"200":
|
||||
description: Actor type, scopes, and resolved org/project for API keys
|
||||
content:
|
||||
application/json:
|
||||
schema:
|
||||
$ref: "#/components/schemas/ActorContext"
|
||||
"401":
|
||||
$ref: "#/components/responses/ErrorResponse"
|
||||
/instance/public:
|
||||
get:
|
||||
tags: [Instance]
|
||||
@@ -2776,6 +2792,34 @@ components:
|
||||
items:
|
||||
$ref: "#/components/schemas/OrganizationSummary"
|
||||
required: [user, organizations]
|
||||
ActorContext:
|
||||
type: object
|
||||
properties:
|
||||
actor:
|
||||
type: string
|
||||
enum: [user, api_key]
|
||||
scopes:
|
||||
type: array
|
||||
items:
|
||||
type: string
|
||||
key_prefix:
|
||||
type: string
|
||||
organization:
|
||||
$ref: "#/components/schemas/ContextRef"
|
||||
project:
|
||||
$ref: "#/components/schemas/ContextRef"
|
||||
required: [actor]
|
||||
ContextRef:
|
||||
type: object
|
||||
properties:
|
||||
id:
|
||||
type: string
|
||||
format: uuid
|
||||
slug:
|
||||
type: string
|
||||
name:
|
||||
type: string
|
||||
required: [id, slug, name]
|
||||
OrganizationMembership:
|
||||
type: object
|
||||
properties:
|
||||
|
||||
Reference in new issue
Block a user