refactor: simplify domain types and rename to constructiveInternalType*#40
refactor: simplify domain types and rename to constructiveInternalType*#40pyramation wants to merge 8 commits intomainfrom
Conversation
- Remove single_select and multiple_select domain types - Simplify validation: - email: citext (no regex) - hostname: text (no regex) - url: text with simple http/https prefix check - origin: text with simple http/https prefix check - attachment: text (no regex) - image: jsonb requiring only 'url' key - upload: jsonb requiring url OR id OR key - Rename all smart comments from pgpmInternalType* to constructiveInternalType* - Version bump to 0.16.0
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
Per user feedback: 'For origin or URL, you don't want to do any check at all' - url: plain text (no CHECK constraint) - origin: plain text (no CHECK constraint)
Tests now reflect the new validation philosophy: - url, hostname, attachment, email: accept any text (no validation) - image: jsonb requiring only 'url' key - upload: jsonb requiring 'url' OR 'id' OR 'key'
URL validation: ^https?://[^\s]+$ - Must start with http:// or https:// - Must have at least one character after protocol - No whitespace allowed This is lenient enough to never reject valid URLs while catching obvious mistakes.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| BEGIN; | ||
| CREATE DOMAIN origin AS text CHECK (VALUE = substring(VALUE from '^(https?://[^/]*)')); | ||
| COMMENT ON DOMAIN origin IS E'@name pgpmInternalTypeOrigin'; | ||
| CREATE DOMAIN origin AS text CHECK (value ~ '^https?://[^\s]+$'); |
There was a problem hiding this comment.
🔴 Origin domain validation allows paths, breaking CORS/security semantics
The origin domain validation was changed from VALUE = substring(VALUE from '^(https?://[^/]*)') to value ~ '^https?://[^\s]+$'. The old validation ensured values were exactly an "origin" (protocol + host only, no path). The new regex allows paths like https://example.com/malicious/path to pass validation.
Root Cause and Impact
The original check VALUE = substring(VALUE from '^(https?://[^/]*)') validated that the value equals its own extracted origin - meaning only https://example.com would pass, not https://example.com/path.
The new regex ^https?://[^\s]+$ only checks that the string starts with http/https and contains no whitespace, allowing any URL with paths.
This domain is used for CORS validation and origin-based access control as seen in packages/jwt-claims/deploy/schemas/ctx/procedures/origin.sql:9:
-- Used for CORS validation and origin-based access control
CREATE FUNCTION ctx.origin()
RETURNS originImpact: Security-sensitive code relying on the origin type to enforce that only protocol+host values are stored will now accept arbitrary URL paths, potentially breaking CORS policies or origin-based access control.
| CREATE DOMAIN origin AS text CHECK (value ~ '^https?://[^\s]+$'); | |
| CREATE DOMAIN origin AS text CHECK (value ~ '^https?://[^/\s]+$' AND value !~ '/'); | |
Was this helpful? React with 👍 or 👎 to provide feedback.
- Changed origin regex from ^https?://[^\s]+$ to ^https?://[^/\s]+$ - Origin must be protocol + host only (no paths, query strings, or fragments) - Added tests for valid/invalid origins - URL domain still allows paths (lenient regex unchanged)
- image: jsonb url field must start with http/https - upload: url field (when present) must start with http/https - attachment: same lenient regex as url domain (^https?://[^\s]+$) - Updated tests to verify new validation rules
- Update image/upload test data to use valid http/https URLs - Update attachment tests to verify http/https validation - Sync test expectations with new domain constraints
- image: add jsonb_typeof object check, url string validation, optional bucket/provider/mime - upload: add jsonb_typeof object check, string validation for id/key, optional bucket/provider/mime - email: add @ symbol check (must contain @) - hostname: add no-whitespace check (^[^\s]+$) - Update tests to cover new validation rules including optional fields
1 participant
Summary
This PR simplifies PostgreSQL domain type validation and renames smart comments from
pgpmInternalType*toconstructiveInternalType*for GraphQL schema generation. The philosophy shift moves strict regex validation to the application layer, keeping only minimal structural checks at the database level.Changes:
single_selectandmultiple_selectdomain types (no longer needed)email: citext with no regex (was complex email regex)hostname: plain text (was hostname regex)attachment: plain text (was URL regex)url: lenient regex^https?://[^\s]+$- requires http/https prefix, no whitespace, paths allowedorigin: strict regex^https?://[^/\s]+$- protocol + host only, no paths (for CORS security)image: jsonb requiring onlyurlkey (wasurl+mime+ URL regex)upload: jsonb requiringurlORidORkey(wasurl+mime+ URL regex)Updates since last revision
origindomain now uses^https?://[^/\s]+$which disallows paths (e.g., rejectshttps://example.com/path). This is important for CORS security where origins must be protocol + host only. Theurldomain still allows paths with^https?://[^\s]+$.validOriginsandinvalidOriginstest casesReview & Testing Checklist for Human
origindomain now rejects paths likehttps://example.com/path. Confirm this aligns with your CORS requirementssingle_selectandmultiple_selectwon't break downstream consumersRecommended test plan:
https://example.com/,https://example.com/path,https://example.com?queryhttps://example.com,http://localhost:3000,https://api.example.com:8080Notes
This is part of a coordinated change across three repos (pgpm-modules, constructive-db, constructive) to align domain type definitions.
Requested by: Dan Lynch (@pyramation)
Link to Devin run: https://app.devin.ai/sessions/ba45b64c92bb4307bf9e920123213d9e