diff --git a/appview/ingester.go b/appview/ingester.go --- a/appview/ingester.go +++ b/appview/ingester.go @@ -33,7 +33,6 @@ "tangled.org/core/appview/models" "tangled.org/core/appview/notify" "tangled.org/core/appview/repoverify" "tangled.org/core/appview/serververify" - "tangled.org/core/appview/validator" "tangled.org/core/idresolver" "tangled.org/core/orm" "tangled.org/core/rbac" @@ -48,7 +47,6 @@ IdResolver *idresolver.Resolver Cache *cache.Cache Config *config.Config Logger *slog.Logger - Validator *validator.Validator MentionsResolver *mentions.Resolver Notifier notify.Notifier Verifier repoverify.Verifier @@ -932,7 +930,7 @@ } string := models.StringFromRecord(did, rkey, record) - if err = i.Validator.ValidateString(&string); err != nil { + if err = string.Validate(); err != nil { l.Error("invalid record", "err", err) return err } @@ -1363,7 +1361,7 @@ if _, err := syntax.ParseDID(string(issue.RepoDid)); err != nil { return fmt.Errorf("issue record repo field is not a valid DID: %w", err) } - if err := i.Validator.ValidateIssue(&issue); err != nil { + if err := issue.Validate(); err != nil { return fmt.Errorf("failed to validate issue: %w", err) } @@ -1512,9 +1510,29 @@ pull, err := models.PullFromRecord(did, rkey, record, readers) if err != nil { return fmt.Errorf("failed to parse pull from record: %w", err) } - if err := i.Validator.ValidatePull(pull); err != nil { + if err := pull.Validate(); err != nil { return fmt.Errorf("failed to validate pull: %w", err) } + if pull.DependentOn != nil { + if err := func() error { + dependentPull, err := db.GetPull( + i.Db, + orm.FilterEq("dependent_on", pull.DependentOn.String()), + ) + if errors.Is(err, sql.ErrNoRows) { + return nil + } + if err != nil { + return fmt.Errorf("failed to fetch pulls with same dependency: %w", err) + } + if dependentPull.AtUri() == pull.AtUri() { + return nil + } + return fmt.Errorf("another pull already depends on %s, which would form a DAG, this is presently disallowed", pull.DependentOn.String()) + }(); err != nil { + return fmt.Errorf("failed to validate pull stack: %w", err) + } + } tx, err := i.Db.BeginTx(ctx, nil) if err != nil { @@ -1755,7 +1773,7 @@ if err != nil { return fmt.Errorf("failed to parse labeldef from record: %w", err) } - if err := i.Validator.ValidateLabelDefinition(def); err != nil { + if err := def.Validate(); err != nil { return fmt.Errorf("failed to validate labeldef: %w", err) } @@ -1833,11 +1851,21 @@ def, ok := actx.Defs[o.OperandKey] if !ok { return fmt.Errorf("failed to find label def for key: %s, expected: %q", o.OperandKey, slices.Collect(maps.Keys(actx.Defs))) } - if err := i.Validator.ValidateLabelOp(ctx, def, repo, &o); err != nil { - if !errors.Is(err, knotacl.ErrKnotUnreachable) { - return fmt.Errorf("failed to validate labelop: %w", err) + // validate permissions: only collaborators can apply labels currently + // + // TODO: introduce a repo:triage permission + allowed, permErr := i.Acl.HasRepoPermissionErr(ctx, repo, o.Did, "repo:push") + if permErr != nil { + if !errors.Is(permErr, knotacl.ErrKnotUnreachable) { + return fmt.Errorf("enforcing permission: %w", permErr) } - l.Warn("ingesting labelop without permission check", "did", o.Did, "err", err) + l.Warn("ingesting labelop without permission check", "did", o.Did, "err", permErr) + } else if !allowed { + return fmt.Errorf("unauthorized label operation") + } + + if err := def.ValidateOperandValue(&o); err != nil { + return fmt.Errorf("failed to validate labelop: %w", err) } } diff --git a/appview/ingester_string_test.go b/appview/ingester_string_test.go --- a/appview/ingester_string_test.go +++ b/appview/ingester_string_test.go @@ -12,7 +12,6 @@ jmodels "github.com/bluesky-social/jetstream/pkg/models" "tangled.org/core/api/tangled" "tangled.org/core/appview/db" "tangled.org/core/appview/models" - "tangled.org/core/appview/validator" "tangled.org/core/orm" ) @@ -25,9 +24,8 @@ t.Fatalf("db.Make: %v", err) } t.Cleanup(func() { d.Close() }) return &Ingester{ - Db: d, - Logger: slog.New(slog.DiscardHandler), - Validator: &validator.Validator{}, + Db: d, + Logger: slog.New(slog.DiscardHandler), } } diff --git a/appview/issues/issues.go b/appview/issues/issues.go --- a/appview/issues/issues.go +++ b/appview/issues/issues.go @@ -28,7 +28,6 @@ "tangled.org/core/appview/pages" "tangled.org/core/appview/pagination" "tangled.org/core/appview/reporesolver" "tangled.org/core/appview/searchquery" - "tangled.org/core/appview/validator" "tangled.org/core/idresolver" "tangled.org/core/ogre" "tangled.org/core/orm" @@ -46,7 +45,6 @@ db *db.DB config *config.Config notifier notify.Notifier logger *slog.Logger - validator *validator.Validator indexer *issues_indexer.Indexer ogreClient *ogre.Client } @@ -61,7 +59,6 @@ mentionsResolver *mentions.Resolver, db *db.DB, config *config.Config, notifier notify.Notifier, - validator *validator.Validator, indexer *issues_indexer.Indexer, logger *slog.Logger, ) *Issues { @@ -76,7 +73,6 @@ db: db, config: config, notifier: notifier, logger: logger, - validator: validator, indexer: indexer, ogreClient: ogre.NewClient(config.Ogre.Host), } @@ -206,7 +202,7 @@ newIssue.Title = r.FormValue("title") newIssue.Body = r.FormValue("body") newIssue.Mentions, newIssue.References = rp.mentionsResolver.Resolve(r.Context(), newIssue.Body) - if err := rp.validator.ValidateIssue(newIssue); err != nil { + if err := newIssue.Validate(); err != nil { l.Error("validation error", "err", err) rp.pages.Notice(w, noticeId, fmt.Sprintf("Failed to edit issue: %s", err)) return @@ -680,7 +676,7 @@ References: references, Repo: f, } - if err := rp.validator.ValidateIssue(issue); err != nil { + if err := issue.Validate(); err != nil { l.Error("validation error", "err", err) rp.pages.Notice(w, "issues", fmt.Sprintf("Failed to create issue: %s", err)) return diff --git a/appview/labels/labels.go b/appview/labels/labels.go --- a/appview/labels/labels.go +++ b/appview/labels/labels.go @@ -11,50 +11,50 @@ "time" "tangled.org/core/api/tangled" "tangled.org/core/appview/db" + "tangled.org/core/appview/knotacl" "tangled.org/core/appview/middleware" "tangled.org/core/appview/models" "tangled.org/core/appview/notify" "tangled.org/core/appview/oauth" "tangled.org/core/appview/pages" - "tangled.org/core/appview/validator" "tangled.org/core/orm" - "tangled.org/core/rbac" "tangled.org/core/tid" comatproto "github.com/bluesky-social/indigo/api/atproto" "github.com/bluesky-social/indigo/atproto/atclient" + "github.com/bluesky-social/indigo/atproto/identity" "github.com/bluesky-social/indigo/atproto/syntax" lexutil "github.com/bluesky-social/indigo/lex/util" "github.com/go-chi/chi/v5" ) type Labels struct { - oauth *oauth.OAuth - pages *pages.Pages - db *db.DB - logger *slog.Logger - validator *validator.Validator - enforcer *rbac.Enforcer - notifier notify.Notifier + oauth *oauth.OAuth + pages *pages.Pages + db *db.DB + dir identity.Directory + logger *slog.Logger + acl *knotacl.Service + notifier notify.Notifier } func New( oauth *oauth.OAuth, pages *pages.Pages, db *db.DB, - validator *validator.Validator, - enforcer *rbac.Enforcer, + dir identity.Directory, + acl *knotacl.Service, notifier notify.Notifier, logger *slog.Logger, ) *Labels { return &Labels{ - oauth: oauth, - pages: pages, - db: db, - logger: logger, - validator: validator, - enforcer: enforcer, - notifier: notifier, + oauth: oauth, + pages: pages, + db: db, + dir: dir, + logger: logger, + acl: acl, + notifier: notifier, } } @@ -167,10 +167,38 @@ } for i := range labelOps { def := actx.Defs[labelOps[i].OperandKey] - if err := l.validator.ValidateLabelOp(r.Context(), def, repo, &labelOps[i]); err != nil { + op := labelOps[i] + + // validate permissions: only collaborators can apply labels currently + // + // TODO: introduce a repo:triage permission + ok, err := l.acl.HasRepoPermissionErr(r.Context(), repo, op.Did, "repo:push") + if err != nil { + fail("Failed to enforce permissions. Please try again later", fmt.Errorf("enforcing permission: %w", err)) + return + } + if !ok { + fail("Unauthorized label operation", fmt.Errorf("unauthorized label operation")) + return + } + + // resolve Handle to DID + if def.ValueType.IsString() && def.ValueType.IsDidFormat() { + val := syntax.AtIdentifier(op.OperandValue) + if val.IsHandle() { + ident, err := l.dir.Lookup(r.Context(), val) + if err != nil { + fail(fmt.Sprintf("Failed to resolve handle %q: %s", val, err), err) + } + op.OperandValue = ident.DID.String() + } + } + + if err := def.ValidateOperandValue(&op); err != nil { fail(fmt.Sprintf("Invalid form data: %s", err), err) return } + labelOps[i] = op } // reduce the opset diff --git a/appview/models/issue.go b/appview/models/issue.go --- a/appview/models/issue.go +++ b/appview/models/issue.go @@ -2,10 +2,12 @@ package models import ( "fmt" + "strings" "time" "github.com/bluesky-social/indigo/atproto/syntax" "tangled.org/core/api/tangled" + "tangled.org/core/appview/pages/markup/sanitizer" ) type Issue struct { @@ -59,6 +61,26 @@ if i.Open { return "open" } return "closed" +} + +var _ Validator = new(Issue) + +func (i *Issue) Validate() error { + if i.Title == "" { + return fmt.Errorf("issue title is empty") + } + if i.Body == "" { + return fmt.Errorf("issue body is empty") + } + + if st := strings.TrimSpace(sanitizer.SanitizeDescription(i.Title)); st == "" { + return fmt.Errorf("title is empty after HTML sanitization") + } + + if st := strings.TrimSpace(sanitizer.SanitizeDefault(i.Body)); st == "" { + return fmt.Errorf("body is empty after HTML sanitization") + } + return nil } func (i *Issue) Participants() []syntax.DID { diff --git a/appview/models/label.go b/appview/models/label.go --- a/appview/models/label.go +++ b/appview/models/label.go @@ -7,7 +7,9 @@ "encoding/hex" "encoding/json" "errors" "fmt" + "regexp" "slices" + "strings" "time" "github.com/bluesky-social/indigo/api/atproto" @@ -120,6 +122,167 @@ ValueType: &vt, } } +var ( + // Label name should be alphanumeric with hyphens/underscores, but not start/end with them + labelNameRegex = regexp.MustCompile(`^[a-zA-Z0-9]([a-zA-Z0-9_-]*[a-zA-Z0-9])?$`) + // Color should be a valid hex color + colorRegex = regexp.MustCompile(`^#[a-fA-F0-9]{6}$`) + // You can only label issues and pulls presently + validScopes = []string{tangled.RepoIssueNSID, tangled.RepoPullNSID} +) + +var _ Validator = new(LabelDefinition) + +func (l *LabelDefinition) Validate() error { + if l.Name == "" { + return fmt.Errorf("label name is empty") + } + if len(l.Name) > 40 { + return fmt.Errorf("label name too long (max 40 graphemes)") + } + if len(l.Name) < 1 { + return fmt.Errorf("label name too short (min 1 grapheme)") + } + if !labelNameRegex.MatchString(l.Name) { + return fmt.Errorf("label name contains invalid characters (use only letters, numbers, hyphens, and underscores)") + } + + if !l.ValueType.IsConcreteType() { + return fmt.Errorf("invalid value type: %q (must be one of: null, boolean, integer, string)", l.ValueType.Type) + } + + // null type checks: cannot be enums, multiple or explicit format + if l.ValueType.IsNull() && l.ValueType.IsEnum() { + return fmt.Errorf("null type cannot be used in conjunction with enum type") + } + if l.ValueType.IsNull() && l.Multiple { + return fmt.Errorf("null type labels cannot be multiple") + } + if l.ValueType.IsNull() && !l.ValueType.IsAnyFormat() { + return fmt.Errorf("format cannot be used in conjunction with null type") + } + + // format checks: cannot be used with enum, or integers + if !l.ValueType.IsAnyFormat() && l.ValueType.IsEnum() { + return fmt.Errorf("enum types cannot be used in conjunction with format specification") + } + + if !l.ValueType.IsAnyFormat() && !l.ValueType.IsString() { + return fmt.Errorf("format specifications are only permitted on string types") + } + + // validate scope (nsid format) + if l.Scope == nil { + return fmt.Errorf("scope is required") + } + for _, s := range l.Scope { + if _, err := syntax.ParseNSID(s); err != nil { + return fmt.Errorf("failed to parse scope: %w", err) + } + if !slices.Contains(validScopes, s) { + return fmt.Errorf("invalid scope: scope must be present in %q", validScopes) + } + } + + // validate color if provided + if l.Color != nil { + color := strings.TrimSpace(*l.Color) + if color == "" { + // empty color is fine, set to nil + l.Color = nil + } else { + if !colorRegex.MatchString(color) { + return fmt.Errorf("color must be a valid hex color (e.g. #79FFE1 or #000)") + } + // expand 3-digit hex to 6-digit hex + if len(color) == 4 { // #ABC + color = fmt.Sprintf("#%c%c%c%c%c%c", color[1], color[1], color[2], color[2], color[3], color[3]) + } + // convert to uppercase for consistency + color = strings.ToUpper(color) + l.Color = &color + } + } + + return nil +} + +// ValidateOperandValue validates the label operation operand value based on +// label definition. +// +// NOTE: This can modify the [LabelOp] +func (def *LabelDefinition) ValidateOperandValue(op *LabelOp) error { + expectedKey := def.AtUri().String() + if op.OperandKey != def.AtUri().String() { + return fmt.Errorf("operand key %q does not match label definition URI %q", op.OperandKey, expectedKey) + } + + valueType := def.ValueType + + // this is permitted, it "unsets" a label + if op.OperandValue == "" { + op.Operation = LabelOperationDel + return nil + } + + switch valueType.Type { + case ConcreteTypeNull: + // For null type, value should be empty + if op.OperandValue != "null" { + return fmt.Errorf("null type requires empty value, got %q", op.OperandValue) + } + + case ConcreteTypeString: + // For string type, validate enum constraints if present + if valueType.IsEnum() { + if !slices.Contains(valueType.Enum, op.OperandValue) { + return fmt.Errorf("value %q is not in allowed enum values %v", op.OperandValue, valueType.Enum) + } + } + + switch valueType.Format { + case ValueTypeFormatDid: + if _, err := syntax.ParseDID(op.OperandValue); err != nil { + return fmt.Errorf("failed to resolve did/handle: %w", err) + } + case ValueTypeFormatAny, "": + default: + return fmt.Errorf("unsupported format constraint: %q", valueType.Format) + } + + case ConcreteTypeInt: + if op.OperandValue == "" { + return fmt.Errorf("integer type requires non-empty value") + } + if _, err := fmt.Sscanf(op.OperandValue, "%d", new(int)); err != nil { + return fmt.Errorf("value %q is not a valid integer", op.OperandValue) + } + + if valueType.IsEnum() { + if !slices.Contains(valueType.Enum, op.OperandValue) { + return fmt.Errorf("value %q is not in allowed enum values %v", op.OperandValue, valueType.Enum) + } + } + + case ConcreteTypeBool: + if op.OperandValue != "true" && op.OperandValue != "false" { + return fmt.Errorf("boolean type requires value to be 'true' or 'false', got %q", op.OperandValue) + } + + // validate enum constraints if present (though uncommon for booleans) + if valueType.IsEnum() { + if !slices.Contains(valueType.Enum, op.OperandValue) { + return fmt.Errorf("value %q is not in allowed enum values %v", op.OperandValue, valueType.Enum) + } + } + + default: + return fmt.Errorf("unsupported value type: %q", valueType.Type) + } + + return nil +} + // random color for a given seed func randomColor(seed string) string { hash := sha1.Sum([]byte(seed)) @@ -131,14 +294,14 @@ return fmt.Sprintf("#%s%s%s", r, g, b) } -func (ld LabelDefinition) GetColor() string { - if ld.Color == nil { - seed := fmt.Sprintf("%d:%s:%s", ld.Id, ld.Did, ld.Rkey) +func (l LabelDefinition) GetColor() string { + if l.Color == nil { + seed := fmt.Sprintf("%d:%s:%s", l.Id, l.Did, l.Rkey) color := randomColor(seed) return color } - return *ld.Color + return *l.Color } func LabelDefinitionFromRecord(did, rkey string, record tangled.LabelDefinition) (*LabelDefinition, error) { @@ -203,6 +366,22 @@ } // otherwise, createdat is in the future relative to indexedat -> use indexedat return indexedAt +} + +var _ Validator = new(LabelOp) + +func (l *LabelOp) Validate() error { + if _, err := syntax.ParseATURI(string(l.Subject)); err != nil { + return fmt.Errorf("invalid subject URI: %w", err) + } + if l.Operation != LabelOperationAdd && l.Operation != LabelOperationDel { + return fmt.Errorf("invalid operation: %q (must be 'add' or 'del')", l.Operation) + } + // Validate performed time is not zero/invalid + if l.PerformedAt.IsZero() { + return fmt.Errorf("performed_at timestamp is required") + } + return nil } type LabelOperation string diff --git a/appview/models/pull.go b/appview/models/pull.go --- a/appview/models/pull.go +++ b/appview/models/pull.go @@ -11,6 +11,7 @@ "strings" "time" "tangled.org/core/api/tangled" + "tangled.org/core/appview/pages/markup/sanitizer" "tangled.org/core/patchutil" "tangled.org/core/types" @@ -122,6 +123,39 @@ Rounds: rounds, Source: p.PullSource.AsRecord(), DependentOn: dependentOn, } +} + +func (pull *Pull) Validate() error { + if len(pull.Submissions) == 0 { + return fmt.Errorf("pull must have at least one submission") + } + + latestSubmission := pull.LatestSubmission() + if latestSubmission == nil { + return fmt.Errorf("pull must have a valid latest submission") + } + + isFormatPatch := patchutil.IsFormatPatch(latestSubmission.Patch) + + // title and body can only be empty if the patch is a format-patch + if !isFormatPatch { + if pull.Title == "" { + return fmt.Errorf("pull title is empty (required for non-format-patch pulls)") + } + + if pull.Body == "" { + return fmt.Errorf("pull body is empty (required for non-format-patch pulls)") + } + + if st := strings.TrimSpace(sanitizer.SanitizeDescription(pull.Title)); st == "" { + return fmt.Errorf("title is empty after HTML sanitization") + } + + if sb := strings.TrimSpace(sanitizer.SanitizeDefault(pull.Body)); sb == "" { + return fmt.Errorf("body is empty after HTML sanitization") + } + } + return nil } func PullFromRecord(did, rkey string, record tangled.RepoPull, blobs []*io.ReadCloser) (*Pull, error) { diff --git a/appview/models/string.go b/appview/models/string.go --- a/appview/models/string.go +++ b/appview/models/string.go @@ -2,10 +2,12 @@ package models import ( "bytes" + "errors" "fmt" "io" "strings" "time" + "unicode/utf8" "github.com/bluesky-social/indigo/atproto/syntax" "tangled.org/core/api/tangled" @@ -33,6 +35,25 @@ Description: s.Description, Contents: s.Contents, CreatedAt: s.Created.Format(time.RFC3339), } +} + +var _ Validator = new(String) + +func (s *String) Validate() error { + var err error + if utf8.RuneCountInString(s.Filename) > 140 { + err = errors.Join(err, fmt.Errorf("filename too long")) + } + + if utf8.RuneCountInString(s.Description) > 280 { + err = errors.Join(err, fmt.Errorf("description too long")) + } + + if len(s.Contents) == 0 { + err = errors.Join(err, fmt.Errorf("contents is empty")) + } + + return err } func StringFromRecord(did, rkey string, record tangled.String) String { diff --git a/appview/models/validator.go b/appview/models/validator.go new file mode 100644 --- /dev/null +++ b/appview/models/validator.go @@ -0,0 +1,6 @@ +package models + +type Validator interface { + // Validate checks the object and returns any error. + Validate() error +} diff --git a/appview/pages/funcmap.go b/appview/pages/funcmap.go --- a/appview/pages/funcmap.go +++ b/appview/pages/funcmap.go @@ -34,6 +34,7 @@ "tangled.org/core/appview/db" "tangled.org/core/appview/models" "tangled.org/core/appview/oauth" "tangled.org/core/appview/pages/markup" + "tangled.org/core/appview/pages/markup/sanitizer" "tangled.org/core/crypto" "tangled.org/core/idresolver" ) @@ -313,7 +314,7 @@ "markdown": func(text string) template.HTML { rctx := p.rctx.Clone() rctx.RendererType = markup.RendererTypeDefault htmlString := rctx.RenderMarkdown(text) - sanitized := rctx.SanitizeDefault(htmlString) + sanitized := sanitizer.SanitizeDefault(htmlString) return template.HTML(sanitized) }, "description": func(text string) template.HTML { @@ -324,14 +325,14 @@ goldmark.WithExtensions( emoji.Emoji, ), )) - sanitized := rctx.SanitizeDescription(htmlString) + sanitized := sanitizer.SanitizeDescription(htmlString) return template.HTML(sanitized) }, "readme": func(text string) template.HTML { rctx := p.rctx.Clone() rctx.RendererType = markup.RendererTypeRepoMarkdown htmlString := rctx.RenderMarkdown(text) - sanitized := rctx.SanitizeDefault(htmlString) + sanitized := sanitizer.SanitizeDefault(htmlString) return template.HTML(sanitized) }, "code": func(content, path string) string { diff --git a/appview/pages/markup/markdown.go b/appview/pages/markup/markdown.go --- a/appview/pages/markup/markdown.go +++ b/appview/pages/markup/markdown.go @@ -48,7 +48,6 @@ repoinfo.RepoInfo IsDev bool Hostname string RendererType RendererType - Sanitizer Sanitizer Files fs.FS } @@ -203,14 +202,6 @@ visitNode(ctx, n) } default: } -} - -func (rctx *RenderContext) SanitizeDefault(html string) string { - return rctx.Sanitizer.SanitizeDefault(html) -} - -func (rctx *RenderContext) SanitizeDescription(html string) string { - return rctx.Sanitizer.SanitizeDescription(html) } type MarkdownTransformer struct { diff --git a/appview/pages/markup/sanitizer.go b/appview/pages/markup/sanitizer/sanitizer.go rename from appview/pages/markup/sanitizer.go rename to appview/pages/markup/sanitizer/sanitizer.go --- a/appview/pages/markup/sanitizer.go +++ b/appview/pages/markup/sanitizer/sanitizer.go @@ -1,4 +1,4 @@ -package markup +package sanitizer import ( "maps" @@ -23,28 +23,14 @@ sharedDescriptionPolicy = buildDescriptionPolicy() sharedLogsPolicy = buildLogsPolicy() } -type Sanitizer struct { - defaultPolicy *bluemonday.Policy - descriptionPolicy *bluemonday.Policy - logsPolicy *bluemonday.Policy +func SanitizeDefault(html string) string { + return sharedDefaultPolicy.Sanitize(html) } - -func NewSanitizer() Sanitizer { - return Sanitizer{ - defaultPolicy: sharedDefaultPolicy, - descriptionPolicy: sharedDescriptionPolicy, - logsPolicy: sharedLogsPolicy, - } +func SanitizeDescription(html string) string { + return sharedDescriptionPolicy.Sanitize(html) } - -func (s *Sanitizer) SanitizeDefault(html string) string { - return s.defaultPolicy.Sanitize(html) -} -func (s *Sanitizer) SanitizeDescription(html string) string { - return s.descriptionPolicy.Sanitize(html) -} -func (s *Sanitizer) SanitizeLogs(html string) string { - return s.logsPolicy.Sanitize(html) +func SanitizeLogs(html string) string { + return sharedLogsPolicy.Sanitize(html) } func buildDefaultPolicy() *bluemonday.Policy { @@ -78,7 +64,6 @@ "div", "ins", "del", "sup", "sub", "p", "ol", "ul", "table", "thead", "tbody", "tfoot", "blockquote", "label", "dl", "dt", "dd", "kbd", "q", "samp", "var", "hr", "ruby", "rt", "rp", "li", "tr", "td", "th", "s", "strike", "summary", "details", "caption", "figure", "figcaption", "abbr", "bdo", "cite", "dfn", "mark", "small", "span", "time", "video", "wbr", - "picture", "source", } policy.AllowAttrs(generalSafeAttrs...).OnElements(generalSafeElements...) diff --git a/appview/pages/pages.go b/appview/pages/pages.go --- a/appview/pages/pages.go +++ b/appview/pages/pages.go @@ -25,6 +25,7 @@ "tangled.org/core/appview/db" "tangled.org/core/appview/models" "tangled.org/core/appview/oauth" "tangled.org/core/appview/pages/markup" + "tangled.org/core/appview/pages/markup/sanitizer" "tangled.org/core/appview/pages/repoinfo" "tangled.org/core/appview/pagination" "tangled.org/core/idresolver" @@ -90,7 +91,6 @@ IsDev: config.Core.Dev, Hostname: config.Core.AppviewHost, CamoUrl: config.Camo.Host, CamoSecret: config.Camo.SharedSecret, - Sanitizer: markup.NewSanitizer(), Files: Files, } @@ -346,7 +346,7 @@ rctx := p.rctx.Clone() rctx.RendererType = markup.RendererTypeDefault htmlString := rctx.RenderMarkdown(string(markdownBytes)) - sanitized := rctx.SanitizeDefault(htmlString) + sanitized := sanitizer.SanitizeDefault(htmlString) params.Content = template.HTML(sanitized) return p.execute("legal/terms", w, params) @@ -375,7 +375,7 @@ rctx := p.rctx.Clone() rctx.RendererType = markup.RendererTypeDefault htmlString := rctx.RenderMarkdown(string(markdownBytes)) - sanitized := rctx.SanitizeDefault(htmlString) + sanitized := sanitizer.SanitizeDefault(htmlString) params.Content = template.HTML(sanitized) return p.execute("legal/privacy", w, params) @@ -920,7 +920,7 @@ switch markup.GetFormat(params.ReadmeFileName) { case markup.FormatMarkdown: params.Raw = false htmlString := rctx.RenderMarkdown(params.Readme) - sanitized := rctx.SanitizeDefault(htmlString) + sanitized := sanitizer.SanitizeDefault(htmlString) params.HTMLReadme = template.HTML(sanitized) default: params.Raw = true @@ -1039,7 +1039,7 @@ switch markup.GetFormat(params.ReadmeFileName) { case markup.FormatMarkdown: params.Raw = false htmlString := rctx.RenderMarkdown(params.Readme) - sanitized := rctx.SanitizeDefault(htmlString) + sanitized := sanitizer.SanitizeDefault(htmlString) params.HTMLReadme = template.HTML(sanitized) default: params.Raw = true diff --git a/appview/pipelines/logs.go b/appview/pipelines/logs.go --- a/appview/pipelines/logs.go +++ b/appview/pipelines/logs.go @@ -8,7 +8,7 @@ "strings" terminal "github.com/buildkite/terminal-to-html/v3" "github.com/gorilla/websocket" - "tangled.org/core/appview/pages/markup" + "tangled.org/core/appview/pages/markup/sanitizer" "tangled.org/core/hostutil" ) @@ -20,14 +20,12 @@ // each non-reset SGR code is pushed onto the stack; a reset clears it. // // the stack contents are prepended to each new line so colours carry over. type ansiState struct { - stack []string - sanitizer markup.Sanitizer + stack []string } func NewAnsiState() *ansiState { return &ansiState{ - stack: []string{}, - sanitizer: markup.NewSanitizer(), + stack: []string{}, } } @@ -37,7 +35,7 @@ prefix := strings.Join(a.stack, "") // render current line with the existing prefix rendered := terminal.Render([]byte(prefix + line)) // sanitize - sanitized := a.sanitizer.SanitizeLogs(rendered) + sanitized := sanitizer.SanitizeLogs(rendered) // update the stack with sequences from current line for _, m := range sequenceRe.FindAllStringSubmatch(line, -1) { diff --git a/appview/pulls/compose.go b/appview/pulls/compose.go --- a/appview/pulls/compose.go +++ b/appview/pulls/compose.go @@ -17,7 +17,7 @@ "tangled.org/core/appview/db" "tangled.org/core/appview/models" "tangled.org/core/appview/oauth" "tangled.org/core/appview/pages" - "tangled.org/core/appview/pages/markup" + "tangled.org/core/appview/pages/markup/sanitizer" "tangled.org/core/appview/xrpcclient" "tangled.org/core/patchutil" "tangled.org/core/types" @@ -78,7 +78,6 @@ if title == "" { s.pages.Notice(w, "pull", "Title is required for git-diff patches.") return } - sanitizer := markup.NewSanitizer() if st := strings.TrimSpace(sanitizer.SanitizeDescription(title)); (st) == "" { s.pages.Notice(w, "pull", "Title is empty after HTML sanitization") return @@ -426,7 +425,7 @@ case pages.SourcePatch: if strings.TrimSpace(patch) == "" { return nil, nil, nil } - if verr := s.validator.ValidatePatch(&patch); verr != nil { + if verr := validatePatch(&patch); verr != nil { return nil, nil, fmt.Errorf("invalid patch: paste a valid git diff or format-patch") } comparison = parsePastedPatch(patch) diff --git a/appview/pulls/compose_helpers_test.go b/appview/pulls/compose_helpers_test.go --- a/appview/pulls/compose_helpers_test.go +++ b/appview/pulls/compose_helpers_test.go @@ -12,7 +12,6 @@ "github.com/go-git/go-git/v5/plumbing/object" "tangled.org/core/appview/models" "tangled.org/core/appview/pages" "tangled.org/core/appview/pages/repoinfo" - "tangled.org/core/appview/validator" "tangled.org/core/patchutil" "tangled.org/core/types" ) @@ -371,8 +370,7 @@ } func TestPrefetchComparisonPatch(t *testing.T) { s := &Pulls{ - validator: &validator.Validator{}, - logger: slog.New(slog.NewTextHandler(io.Discard, nil)), + logger: slog.New(slog.NewTextHandler(io.Discard, nil)), } cases := []struct { @@ -408,8 +406,7 @@ } func TestPrefetchComparisonValidPatch(t *testing.T) { s := &Pulls{ - validator: &validator.Validator{}, - logger: slog.New(slog.NewTextHandler(io.Discard, nil)), + logger: slog.New(slog.NewTextHandler(io.Discard, nil)), } patch := `diff --git a/a.txt b/a.txt index 0000000..1111111 100644 @@ -435,8 +432,7 @@ } func TestPrefetchComparisonMissingInputs(t *testing.T) { s := &Pulls{ - validator: &validator.Validator{}, - logger: slog.New(slog.NewTextHandler(io.Discard, nil)), + logger: slog.New(slog.NewTextHandler(io.Discard, nil)), } cases := []struct { diff --git a/appview/pulls/create.go b/appview/pulls/create.go --- a/appview/pulls/create.go +++ b/appview/pulls/create.go @@ -71,7 +71,7 @@ sourceRev := comparison.Rev2 patch := comparison.FormatPatchRaw combined := comparison.CombinedPatchRaw - if err := s.validator.ValidatePatch(&patch); err != nil { + if err := validatePatch(&patch); err != nil { s.logger.Error("failed to validate patch", "err", err) s.pages.Notice(w, "pull", "Invalid patch format. Please provide a valid diff.") return @@ -85,7 +85,7 @@ s.createPullRequest(w, r, repo, userDid, title, body, targetBranch, patch, combined, sourceRev, pullSource, isStacked, stackTitles, stackBodies) } func (s *Pulls) handlePatchBasedPull(w http.ResponseWriter, r *http.Request, repo *models.Repo, userDid syntax.DID, title, body, targetBranch, patch string, isStacked bool, stackTitles, stackBodies map[string]string) { - if err := s.validator.ValidatePatch(&patch); err != nil { + if err := validatePatch(&patch); err != nil { s.logger.Error("patch validation failed", "err", err) s.pages.Notice(w, "pull", "Invalid patch format. Please provide a valid diff.") return @@ -178,7 +178,7 @@ sourceRev := comparison.Rev2 patch := comparison.FormatPatchRaw combined := comparison.CombinedPatchRaw - if err := s.validator.ValidatePatch(&patch); err != nil { + if err := validatePatch(&patch); err != nil { s.logger.Error("failed to validate patch", "err", err) s.pages.Notice(w, "pull", "Invalid patch format. Please provide a valid diff.") return diff --git a/appview/pulls/labels.go b/appview/pulls/labels.go --- a/appview/pulls/labels.go +++ b/appview/pulls/labels.go @@ -140,7 +140,33 @@ valid := make([]models.LabelOp, 0, len(raw)) for _, op := range raw { def := defs[op.OperandKey] - if err := s.validator.ValidateLabelOp(ctx, def, repo, &op); err != nil { + + // validate permissions: only collaborators can apply labels currently + // + // TODO: introduce a repo:triage permission + ok, err := s.acl.HasRepoPermissionErr(ctx, repo, op.Did, "repo:push") + if err != nil { + l.Warn("invalid label op", "err", err, "subject", op.Subject, "key", op.OperandKey) + continue + } + if !ok { + l.Warn("forbidden label op", "subject", op.Subject, "key", op.OperandKey) + continue + } + + // resolve Handle to DID + if def.ValueType.IsString() && def.ValueType.IsDidFormat() { + val := syntax.AtIdentifier(op.OperandValue) + if val.IsHandle() { + ident, err := s.idResolver.Directory().Lookup(ctx, val) + if err != nil { + l.Warn("failed to resolve handle", "err", err, "subject", op.Subject, "key", op.OperandKey) + } + op.OperandValue = ident.DID.String() + } + } + + if err := def.ValidateOperandValue(&op); err != nil { l.Warn("invalid label op", "err", err, "subject", op.Subject, "key", op.OperandKey) continue } diff --git a/appview/pulls/pulls.go b/appview/pulls/pulls.go --- a/appview/pulls/pulls.go +++ b/appview/pulls/pulls.go @@ -6,6 +6,7 @@ "compress/gzip" "fmt" "io" "log/slog" + "strings" "tangled.org/core/appview/config" "tangled.org/core/appview/db" @@ -17,9 +18,9 @@ "tangled.org/core/appview/notify" "tangled.org/core/appview/oauth" "tangled.org/core/appview/pages" "tangled.org/core/appview/reporesolver" - "tangled.org/core/appview/validator" "tangled.org/core/idresolver" "tangled.org/core/ogre" + "tangled.org/core/patchutil" indigoxrpc "github.com/bluesky-social/indigo/xrpc" ) @@ -37,7 +38,6 @@ config *config.Config notifier notify.Notifier acl *knotacl.Service logger *slog.Logger - validator *validator.Validator indexer *pulls_indexer.Indexer ogreClient *ogre.Client } @@ -52,7 +52,6 @@ db *db.DB, config *config.Config, notifier notify.Notifier, acl *knotacl.Service, - validator *validator.Validator, indexer *pulls_indexer.Indexer, logger *slog.Logger, ) *Pulls { @@ -67,7 +66,6 @@ config: config, notifier: notifier, acl: acl, logger: logger, - validator: validator, indexer: indexer, ogreClient: ogre.NewClient(config.Ogre.Host), } @@ -90,3 +88,20 @@ return &b } func ptrPullState(s models.PullState) *models.PullState { return &s } + +func validatePatch(patch *string) error { + if patch == nil || *patch == "" { + return fmt.Errorf("patch is empty") + } + + // add newline if not present to diff style patches + if !patchutil.IsFormatPatch(*patch) && !strings.HasSuffix(*patch, "\n") { + *patch = *patch + "\n" + } + + if err := patchutil.IsPatchValid(*patch); err != nil { + return err + } + + return nil +} diff --git a/appview/pulls/resubmit.go b/appview/pulls/resubmit.go --- a/appview/pulls/resubmit.go +++ b/appview/pulls/resubmit.go @@ -274,7 +274,7 @@ s.resubmitStackedPullHelper(w, r, repo, userDid, pull, patch) return } - if err := s.validator.ValidatePatch(&patch); err != nil { + if err := validatePatch(&patch); err != nil { s.pages.Notice(w, "resubmit-error", err.Error()) return } diff --git a/appview/repo/repo.go b/appview/repo/repo.go --- a/appview/repo/repo.go +++ b/appview/repo/repo.go @@ -27,7 +27,6 @@ "tangled.org/core/appview/pages" "tangled.org/core/appview/pagination" "tangled.org/core/appview/reporesolver" "tangled.org/core/appview/sites" - "tangled.org/core/appview/validator" xrpcclient "tangled.org/core/appview/xrpcclient" "tangled.org/core/consts" "tangled.org/core/eventconsumer" @@ -59,7 +58,6 @@ acl *knotacl.Service notifier notify.Notifier logger *slog.Logger serviceAuth *serviceauth.ServiceAuth - validator *validator.Validator cfClient *cloudflare.Client ogreClient *ogre.Client codesearch *codesearch.CodeSearch @@ -77,7 +75,6 @@ notifier notify.Notifier, enforcer *rbac.Enforcer, acl *knotacl.Service, logger *slog.Logger, - validator *validator.Validator, cfClient *cloudflare.Client, codesearch *codesearch.CodeSearch, ) *Repo { @@ -93,7 +90,6 @@ notifier: notifier, enforcer: enforcer, acl: acl, logger: logger, - validator: validator, cfClient: cfClient, ogreClient: ogre.NewClient(config.Ogre.Host), codesearch: codesearch, @@ -255,7 +251,7 @@ Color: &color, Multiple: multiple, Created: time.Now(), } - if err := rp.validator.ValidateLabelDefinition(&label); err != nil { + if err := label.Validate(); err != nil { fail(err.Error(), err) return } diff --git a/appview/repo/settings.go b/appview/repo/settings.go --- a/appview/repo/settings.go +++ b/appview/repo/settings.go @@ -5,7 +5,9 @@ "context" "encoding/json" "fmt" "net/http" + "net/url" "path" + "regexp" "slices" "strings" "time" @@ -21,6 +23,7 @@ "tangled.org/core/appview/sites" xrpcclient "tangled.org/core/appview/xrpcclient" "tangled.org/core/consts" "tangled.org/core/orm" + "tangled.org/core/sets" "tangled.org/core/types" comatproto "github.com/bluesky-social/indigo/api/atproto" @@ -552,14 +555,15 @@ website = r.FormValue("website") topicStr = r.FormValue("topics") ) - err = rp.validator.ValidateURI(website) - if website != "" && err != nil { - l.Error("invalid uri", "err", err) - rp.pages.Notice(w, noticeId, err.Error()) - return + if website != "" { + if err := validateURI(website); err != nil { + l.Error("invalid uri", "err", err) + rp.pages.Notice(w, noticeId, err.Error()) + return + } } - topics, err := rp.validator.ValidateRepoTopicStr(topicStr) + topics, err := parseRepoTopicStr(topicStr) if err != nil { l.Error("invalid topics", "err", err) rp.pages.Notice(w, noticeId, err.Error()) @@ -619,3 +623,59 @@ } rp.pages.HxRefresh(w) } + +const ( + maxTopicLen = 50 + maxTopics = 20 +) + +var ( + topicRE = regexp.MustCompile(`\A[a-z0-9-]+\z`) +) + +// parseRepoTopicStr parses and validates whitespace-separated topic string. +// +// Rules: +// - topics are separated by whitespace +// - each topic may contain lowercase letters, digits, and hyphens only +// - each topic must be <= 50 characters long +// - no more than 20 topics allowed +// - duplicates are removed +func parseRepoTopicStr(topicStr string) ([]string, error) { + topicStr = strings.TrimSpace(topicStr) + if topicStr == "" { + return nil, nil + } + parts := strings.Fields(topicStr) + if len(parts) > maxTopics { + return nil, fmt.Errorf("too many topics: %d (maximum %d)", len(parts), maxTopics) + } + + topicSet := sets.New[string]() + + for _, t := range parts { + if topicSet.Contains(t) { + continue + } + if len(t) > maxTopicLen { + return nil, fmt.Errorf("topic '%s' is too long (maximum %d characters)", t, maxTopics) + } + if !topicRE.MatchString(t) { + return nil, fmt.Errorf("topic '%s' contains invalid characters (allowed: lowercase letters, digits, hyphens)", t) + } + topicSet.Insert(t) + } + return slices.Collect(topicSet.All()), nil +} + +// TODO(boltless): move this to models.Repo instead +func validateURI(uri string) error { + parsed, err := url.Parse(uri) + if err != nil { + return fmt.Errorf("invalid uri format") + } + if parsed.Scheme == "" { + return fmt.Errorf("uri scheme missing") + } + return nil +} diff --git a/appview/state/router.go b/appview/state/router.go --- a/appview/state/router.go +++ b/appview/state/router.go @@ -372,7 +372,6 @@ s.mentionsResolver, s.db, s.config, s.notifier, - s.validator, s.indexer.Issues, log.SubLogger(s.logger, "issues"), ) @@ -390,7 +389,6 @@ s.db, s.config, s.notifier, s.aclService, - s.validator, s.indexer.Pulls, log.SubLogger(s.logger, "pulls"), ) @@ -410,7 +408,6 @@ s.notifier, s.enforcer, s.aclService, log.SubLogger(s.logger, "repo"), - s.validator, s.cfClient, s.codesearch, ) @@ -438,8 +435,8 @@ ls := labels.New( s.oauth, s.pages, s.db, - s.validator, - s.enforcer, + s.idResolver.Directory(), + s.aclService, s.notifier, log.SubLogger(s.logger, "labels"), ) diff --git a/appview/state/state.go b/appview/state/state.go --- a/appview/state/state.go +++ b/appview/state/state.go @@ -35,7 +35,6 @@ "tangled.org/core/appview/pipelines" pipelinessh "tangled.org/core/appview/pipelines/ssh" "tangled.org/core/appview/reporesolver" "tangled.org/core/appview/repoverify" - "tangled.org/core/appview/validator" xrpcclient "tangled.org/core/appview/xrpcclient" "tangled.org/core/consts" "tangled.org/core/eventconsumer" @@ -75,7 +74,6 @@ knotstream *eventconsumer.Consumer spindlestream *eventconsumer.Consumer pipelineNotifier *pipelines.StatusNotifier logger *slog.Logger - validator *validator.Validator cfClient *cloudflare.Client codesearch *codesearch.CodeSearch } @@ -122,9 +120,6 @@ oauth, err := oauth.New(config, posthog, d, enforcer, aclService, res, log.SubLogger(logger, "oauth")) if err != nil { return nil, fmt.Errorf("failed to start oauth handler: %w", err) } - - validator := validator.New(d, res, aclService) - repoResolver := reporesolver.New(config, aclService, d, rdb) mentionsResolver := mentions.New(config, res, d, log.SubLogger(logger, "mentionsResolver")) @@ -196,7 +191,6 @@ IdResolver: res, Cache: rdb, Config: config, Logger: log.SubLogger(logger, "ingester"), - Validator: validator, MentionsResolver: mentionsResolver, Notifier: notifier, Verifier: repoverify.New(res, config.Core.Dev), @@ -250,7 +244,6 @@ knotstream: knotstream, spindlestream: spindlestream, pipelineNotifier: pipelineNotifier, logger: logger, - validator: validator, cfClient: cfClient, codesearch: &codesearch.CodeSearch{Host: config.CodeSearch.ZoektUrl}, } diff --git a/appview/validator/issue.go b/appview/validator/issue.go deleted file mode 100644 --- a/appview/validator/issue.go +++ /dev/null @@ -1,28 +0,0 @@ -package validator - -import ( - "fmt" - "strings" - - "tangled.org/core/appview/models" -) - -func (v *Validator) ValidateIssue(issue *models.Issue) error { - if issue.Title == "" { - return fmt.Errorf("issue title is empty") - } - - if issue.Body == "" { - return fmt.Errorf("issue body is empty") - } - - if st := strings.TrimSpace(v.sanitizer.SanitizeDescription(issue.Title)); st == "" { - return fmt.Errorf("title is empty after HTML sanitization") - } - - if sb := strings.TrimSpace(v.sanitizer.SanitizeDefault(issue.Body)); sb == "" { - return fmt.Errorf("body is empty after HTML sanitization") - } - - return nil -} diff --git a/appview/validator/label.go b/appview/validator/label.go deleted file mode 100644 --- a/appview/validator/label.go +++ /dev/null @@ -1,217 +0,0 @@ -package validator - -import ( - "context" - "fmt" - "regexp" - "slices" - "strings" - - "github.com/bluesky-social/indigo/atproto/syntax" - "tangled.org/core/api/tangled" - "tangled.org/core/appview/models" -) - -var ( - // Label name should be alphanumeric with hyphens/underscores, but not start/end with them - labelNameRegex = regexp.MustCompile(`^[a-zA-Z0-9]([a-zA-Z0-9_-]*[a-zA-Z0-9])?$`) - // Color should be a valid hex color - colorRegex = regexp.MustCompile(`^#[a-fA-F0-9]{6}$`) - // You can only label issues and pulls presently - validScopes = []string{tangled.RepoIssueNSID, tangled.RepoPullNSID} -) - -func (v *Validator) ValidateLabelDefinition(label *models.LabelDefinition) error { - if label.Name == "" { - return fmt.Errorf("label name is empty") - } - if len(label.Name) > 40 { - return fmt.Errorf("label name too long (max 40 graphemes)") - } - if len(label.Name) < 1 { - return fmt.Errorf("label name too short (min 1 grapheme)") - } - if !labelNameRegex.MatchString(label.Name) { - return fmt.Errorf("label name contains invalid characters (use only letters, numbers, hyphens, and underscores)") - } - - if !label.ValueType.IsConcreteType() { - return fmt.Errorf("invalid value type: %q (must be one of: null, boolean, integer, string)", label.ValueType.Type) - } - - // null type checks: cannot be enums, multiple or explicit format - if label.ValueType.IsNull() && label.ValueType.IsEnum() { - return fmt.Errorf("null type cannot be used in conjunction with enum type") - } - if label.ValueType.IsNull() && label.Multiple { - return fmt.Errorf("null type labels cannot be multiple") - } - if label.ValueType.IsNull() && !label.ValueType.IsAnyFormat() { - return fmt.Errorf("format cannot be used in conjunction with null type") - } - - // format checks: cannot be used with enum, or integers - if !label.ValueType.IsAnyFormat() && label.ValueType.IsEnum() { - return fmt.Errorf("enum types cannot be used in conjunction with format specification") - } - - if !label.ValueType.IsAnyFormat() && !label.ValueType.IsString() { - return fmt.Errorf("format specifications are only permitted on string types") - } - - // validate scope (nsid format) - if label.Scope == nil { - return fmt.Errorf("scope is required") - } - for _, s := range label.Scope { - if _, err := syntax.ParseNSID(s); err != nil { - return fmt.Errorf("failed to parse scope: %w", err) - } - if !slices.Contains(validScopes, s) { - return fmt.Errorf("invalid scope: scope must be present in %q", validScopes) - } - } - - // validate color if provided - if label.Color != nil { - color := strings.TrimSpace(*label.Color) - if color == "" { - // empty color is fine, set to nil - label.Color = nil - } else { - if !colorRegex.MatchString(color) { - return fmt.Errorf("color must be a valid hex color (e.g. #79FFE1 or #000)") - } - // expand 3-digit hex to 6-digit hex - if len(color) == 4 { // #ABC - color = fmt.Sprintf("#%c%c%c%c%c%c", color[1], color[1], color[2], color[2], color[3], color[3]) - } - // convert to uppercase for consistency - color = strings.ToUpper(color) - label.Color = &color - } - } - - return nil -} - -func (v *Validator) ValidateLabelOp(ctx context.Context, labelDef *models.LabelDefinition, repo *models.Repo, labelOp *models.LabelOp) error { - if labelDef == nil { - return fmt.Errorf("label definition is required") - } - if repo == nil { - return fmt.Errorf("repo is required") - } - if labelOp == nil { - return fmt.Errorf("label operation is required") - } - - expectedKey := labelDef.AtUri().String() - if labelOp.OperandKey != expectedKey { - return fmt.Errorf("operand key %q does not match label definition URI %q", labelOp.OperandKey, expectedKey) - } - - if labelOp.Operation != models.LabelOperationAdd && labelOp.Operation != models.LabelOperationDel { - return fmt.Errorf("invalid operation: %q (must be 'add' or 'del')", labelOp.Operation) - } - - if labelOp.Subject == "" { - return fmt.Errorf("subject URI is required") - } - if _, err := syntax.ParseATURI(string(labelOp.Subject)); err != nil { - return fmt.Errorf("invalid subject URI: %w", err) - } - - if err := v.validateOperandValue(labelDef, labelOp); err != nil { - return fmt.Errorf("invalid operand value: %w", err) - } - - // Validate performed time is not zero/invalid - if labelOp.PerformedAt.IsZero() { - return fmt.Errorf("performed_at timestamp is required") - } - - // validate permissions: only collaborators can apply labels currently - // - // TODO: introduce a repo:triage permission - ok, err := v.acl.HasRepoPermissionErr(ctx, repo, labelOp.Did, "repo:push") - if err != nil { - return err - } - if !ok { - return fmt.Errorf("unauthorized label operation") - } - - return nil -} - -func (v *Validator) validateOperandValue(labelDef *models.LabelDefinition, labelOp *models.LabelOp) error { - valueType := labelDef.ValueType - - // this is permitted, it "unsets" a label - if labelOp.OperandValue == "" { - labelOp.Operation = models.LabelOperationDel - return nil - } - - switch valueType.Type { - case models.ConcreteTypeNull: - // For null type, value should be empty - if labelOp.OperandValue != "null" { - return fmt.Errorf("null type requires empty value, got %q", labelOp.OperandValue) - } - - case models.ConcreteTypeString: - // For string type, validate enum constraints if present - if valueType.IsEnum() { - if !slices.Contains(valueType.Enum, labelOp.OperandValue) { - return fmt.Errorf("value %q is not in allowed enum values %v", labelOp.OperandValue, valueType.Enum) - } - } - - switch valueType.Format { - case models.ValueTypeFormatDid: - id, err := v.resolver.ResolveIdent(context.Background(), labelOp.OperandValue) - if err != nil { - return fmt.Errorf("failed to resolve did/handle: %w", err) - } - - labelOp.OperandValue = id.DID.String() - - case models.ValueTypeFormatAny, "": - default: - return fmt.Errorf("unsupported format constraint: %q", valueType.Format) - } - - case models.ConcreteTypeInt: - if labelOp.OperandValue == "" { - return fmt.Errorf("integer type requires non-empty value") - } - if _, err := fmt.Sscanf(labelOp.OperandValue, "%d", new(int)); err != nil { - return fmt.Errorf("value %q is not a valid integer", labelOp.OperandValue) - } - - if valueType.IsEnum() { - if !slices.Contains(valueType.Enum, labelOp.OperandValue) { - return fmt.Errorf("value %q is not in allowed enum values %v", labelOp.OperandValue, valueType.Enum) - } - } - - case models.ConcreteTypeBool: - if labelOp.OperandValue != "true" && labelOp.OperandValue != "false" { - return fmt.Errorf("boolean type requires value to be 'true' or 'false', got %q", labelOp.OperandValue) - } - - // validate enum constraints if present (though uncommon for booleans) - if valueType.IsEnum() { - if !slices.Contains(valueType.Enum, labelOp.OperandValue) { - return fmt.Errorf("value %q is not in allowed enum values %v", labelOp.OperandValue, valueType.Enum) - } - } - - default: - return fmt.Errorf("unsupported value type: %q", valueType.Type) - } - - return nil -} diff --git a/appview/validator/label_test.go b/appview/validator/label_test.go deleted file mode 100644 --- a/appview/validator/label_test.go +++ /dev/null @@ -1,87 +0,0 @@ -package validator - -import ( - "context" - "encoding/json" - "errors" - "io" - "log/slog" - "net/http" - "net/http/httptest" - "path/filepath" - "strings" - "testing" - "time" - - "tangled.org/core/api/tangled" - "tangled.org/core/appview/db" - "tangled.org/core/appview/knotacl" - "tangled.org/core/appview/models" - "tangled.org/core/consts" - "tangled.org/core/rbac" -) - -func unreachableListValidator(t *testing.T) (*Validator, string) { - t.Helper() - srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - switch { - case strings.HasSuffix(r.URL.Path, tangled.KnotVersionNSID): - json.NewEncoder(w).Encode(tangled.KnotVersion_Output{Version: "v1.15.0", Capabilities: []string{string(consts.CapKnotACL)}}) - case strings.HasSuffix(r.URL.Path, tangled.RepoListCollaboratorsNSID): - http.Error(w, "list down", http.StatusInternalServerError) - default: - http.NotFound(w, r) - } - })) - t.Cleanup(srv.Close) - host := strings.TrimPrefix(srv.URL, "http://") - - dir := t.TempDir() - enforcer, err := rbac.NewEnforcer(filepath.Join(dir, "rbac.db")) - if err != nil { - t.Fatalf("NewEnforcer: %v", err) - } - d, err := db.Make(context.Background(), filepath.Join(dir, "appview.db")) - if err != nil { - t.Fatalf("db.Make: %v", err) - } - svc := knotacl.NewService(enforcer, d, true, slog.New(slog.NewTextHandler(io.Discard, nil))) - return &Validator{acl: svc}, host -} - -func TestValidateLabelOp_MalformedRejectedBeforePermCheck(t *testing.T) { - v, host := unreachableListValidator(t) - def := &models.LabelDefinition{Did: "did:plc:akshay", Rkey: "deadbeef"} - repo := &models.Repo{Did: "did:plc:akshay", Knot: host, RepoDid: "did:plc:limpet"} - - op := &models.LabelOp{ - Did: "did:plc:scallop", - OperandKey: "does-not-match-the-def-aturi", - Operation: "garbage", - } - err := v.ValidateLabelOp(context.Background(), def, repo, op) - if errors.Is(err, knotacl.ErrKnotUnreachable) { - t.Fatalf("malformed op returned ErrKnotUnreachable; structural validation did not run before the perm check") - } - if err == nil || !strings.Contains(err.Error(), "operand key") { - t.Fatalf("want a structural operand-key error, got %v", err) - } -} - -func TestValidateLabelOp_WellFormedFailsOpenWhenKnotUnreachable(t *testing.T) { - v, host := unreachableListValidator(t) - def := &models.LabelDefinition{Did: "did:plc:akshay", Rkey: "deadbeef"} - repo := &models.Repo{Did: "did:plc:akshay", Knot: host, RepoDid: "did:plc:limpet"} - - op := &models.LabelOp{ - Did: "did:plc:scallop", - OperandKey: def.AtUri().String(), - Operation: models.LabelOperationAdd, - Subject: "at://did:plc:limpet/sh.tangled.repo.issue/abc123", - PerformedAt: time.Now(), - } - err := v.ValidateLabelOp(context.Background(), def, repo, op) - if !errors.Is(err, knotacl.ErrKnotUnreachable) { - t.Fatalf("well-formed op against an unreachable knot = %v, want ErrKnotUnreachable so the ingester fails open", err) - } -} diff --git a/appview/validator/patch.go b/appview/validator/patch.go deleted file mode 100644 --- a/appview/validator/patch.go +++ /dev/null @@ -1,25 +0,0 @@ -package validator - -import ( - "fmt" - "strings" - - "tangled.org/core/patchutil" -) - -func (v *Validator) ValidatePatch(patch *string) error { - if patch == nil || *patch == "" { - return fmt.Errorf("patch is empty") - } - - // add newline if not present to diff style patches - if !patchutil.IsFormatPatch(*patch) && !strings.HasSuffix(*patch, "\n") { - *patch = *patch + "\n" - } - - if err := patchutil.IsPatchValid(*patch); err != nil { - return err - } - - return nil -} diff --git a/appview/validator/pull.go b/appview/validator/pull.go deleted file mode 100644 --- a/appview/validator/pull.go +++ /dev/null @@ -1,68 +0,0 @@ -package validator - -import ( - "database/sql" - "fmt" - "strings" - - "tangled.org/core/appview/db" - "tangled.org/core/appview/models" - "tangled.org/core/orm" - "tangled.org/core/patchutil" -) - -func (v *Validator) ValidatePull(pull *models.Pull) error { - if len(pull.Submissions) == 0 { - return fmt.Errorf("pull must have at least one submission") - } - - latestSubmission := pull.LatestSubmission() - if latestSubmission == nil { - return fmt.Errorf("pull must have a valid latest submission") - } - - isFormatPatch := patchutil.IsFormatPatch(latestSubmission.Patch) - - // title and body can only be empty if the patch is a format-patch - if !isFormatPatch { - if pull.Title == "" { - return fmt.Errorf("pull title is empty (required for non-format-patch pulls)") - } - - if pull.Body == "" { - return fmt.Errorf("pull body is empty (required for non-format-patch pulls)") - } - - if st := strings.TrimSpace(v.sanitizer.SanitizeDescription(pull.Title)); st == "" { - return fmt.Errorf("title is empty after HTML sanitization") - } - - if sb := strings.TrimSpace(v.sanitizer.SanitizeDefault(pull.Body)); sb == "" { - return fmt.Errorf("body is empty after HTML sanitization") - } - } - - // the dependent_on should not form a DAG, aka, two PRs should not have the same dependent - if pull.DependentOn != nil { - dependentPull, err := db.GetPull( - v.db, - orm.FilterEq("dependent_on", pull.DependentOn.String()), - ) - - if err == sql.ErrNoRows { - return nil - } - - if err != nil { - return fmt.Errorf("failed to fetch pulls with same dependency: %w", err) - } - - if dependentPull.AtUri() == pull.AtUri() { - return nil - } - - return fmt.Errorf("another pull already depends on %s, which would form a DAG, this is presently disallowed", pull.DependentOn.String()) - } - - return nil -} diff --git a/appview/validator/repo_topics.go b/appview/validator/repo_topics.go deleted file mode 100644 --- a/appview/validator/repo_topics.go +++ /dev/null @@ -1,53 +0,0 @@ -package validator - -import ( - "fmt" - "maps" - "regexp" - "slices" - "strings" -) - -const ( - maxTopicLen = 50 - maxTopics = 20 -) - -var ( - topicRE = regexp.MustCompile(`\A[a-z0-9-]+\z`) -) - -// ValidateRepoTopicStr parses and validates whitespace-separated topic string. -// -// Rules: -// - topics are separated by whitespace -// - each topic may contain lowercase letters, digits, and hyphens only -// - each topic must be <= 50 characters long -// - no more than 20 topics allowed -// - duplicates are removed -func (v *Validator) ValidateRepoTopicStr(topicsStr string) ([]string, error) { - topicsStr = strings.TrimSpace(topicsStr) - if topicsStr == "" { - return nil, nil - } - parts := strings.Fields(topicsStr) - if len(parts) > maxTopics { - return nil, fmt.Errorf("too many topics: %d (maximum %d)", len(parts), maxTopics) - } - - topicSet := make(map[string]struct{}) - - for _, t := range parts { - if _, exists := topicSet[t]; exists { - continue - } - if len(t) > maxTopicLen { - return nil, fmt.Errorf("topic '%s' is too long (maximum %d characters)", t, maxTopics) - } - if !topicRE.MatchString(t) { - return nil, fmt.Errorf("topic '%s' contains invalid characters (allowed: lowercase letters, digits, hyphens)", t) - } - topicSet[t] = struct{}{} - } - return slices.Collect(maps.Keys(topicSet)), nil -} diff --git a/appview/validator/string.go b/appview/validator/string.go deleted file mode 100644 --- a/appview/validator/string.go +++ /dev/null @@ -1,27 +0,0 @@ -package validator - -import ( - "errors" - "fmt" - "unicode/utf8" - - "tangled.org/core/appview/models" -) - -func (v *Validator) ValidateString(s *models.String) error { - var err error - - if utf8.RuneCountInString(s.Filename) > 140 { - err = errors.Join(err, fmt.Errorf("filename too long")) - } - - if utf8.RuneCountInString(s.Description) > 280 { - err = errors.Join(err, fmt.Errorf("description too long")) - } - - if len(s.Contents) == 0 { - err = errors.Join(err, fmt.Errorf("contents is empty")) - } - - return err -} diff --git a/appview/validator/uri.go b/appview/validator/uri.go deleted file mode 100644 --- a/appview/validator/uri.go +++ /dev/null @@ -1,17 +0,0 @@ -package validator - -import ( - "fmt" - "net/url" -) - -func (v *Validator) ValidateURI(uri string) error { - parsed, err := url.Parse(uri) - if err != nil { - return fmt.Errorf("invalid uri format") - } - if parsed.Scheme == "" { - return fmt.Errorf("uri scheme missing") - } - return nil -} diff --git a/appview/validator/validator.go b/appview/validator/validator.go deleted file mode 100644 --- a/appview/validator/validator.go +++ /dev/null @@ -1,24 +0,0 @@ -package validator - -import ( - "tangled.org/core/appview/db" - "tangled.org/core/appview/knotacl" - "tangled.org/core/appview/pages/markup" - "tangled.org/core/idresolver" -) - -type Validator struct { - db *db.DB - sanitizer markup.Sanitizer - resolver *idresolver.Resolver - acl *knotacl.Service -} - -func New(db *db.DB, res *idresolver.Resolver, acl *knotacl.Service) *Validator { - return &Validator{ - db: db, - sanitizer: markup.NewSanitizer(), - resolver: res, - acl: acl, - } -}