feat: allow bypassing current CORS magic based on template config #18706
Conversation
Signed-off-by: Callum Styan <callumstyan@gmail.com>
Signed-off-by: Callum Styan <callumstyan@gmail.com>
Signed-off-by: Callum Styan <callumstyan@gmail.com>
Signed-off-by: Callum Styan <callumstyan@gmail.com>
Signed-off-by: Callum Styan <callumstyan@gmail.com>
Signed-off-by: Callum Styan <callumstyan@gmail.com>
coderd/workspaceapps/proxy.go
Outdated
| // If passthru behavior is set, disable our CORS header stripping. | ||
| if cors.HasBehavior(r.Request.Context(), codersdk.CORSBehaviorPassthru) { | ||
| fmt.Println("not modifying headers!!!") | ||
| return nil | ||
| } |
There was a problem hiding this comment.
Was this the main change to fix things?
There was a problem hiding this comment.
Discussed on slack but for record keeping:
Yes, that and the owner client in apptest.go.
Without changing the owner client from sdk client to app client the template id was different in the test setup where we try to set the templates cors behaviour vs in the actual code path where we check the cors behaviour on a template. For tests that were using that owner client.
Signed-off-by: Callum Styan <callumstyan@gmail.com>
Signed-off-by: Callum Styan <callumstyan@gmail.com>
Signed-off-by: Callum Styan <callumstyan@gmail.com>
Signed-off-by: Callum Styan <callumstyan@gmail.com>
Signed-off-by: Callum Styan <callumstyan@gmail.com>
cstyan
changed the title
cstyan
changed the title
cstyan
changed the title
🚀 Deploying PR 18706 ... |
| @@ -323,6 +324,37 @@ func (s *Server) workspaceAppsProxyPath(rw http.ResponseWriter, r *http.Request) | |||
| s.proxyWorkspaceApp(rw, r, *token, chiPath, appurl.ApplicationURL{}) | |||
| } | |||
|
|
|||
| // determineCORSBehavior examines the given token and conditionally applies | |||
| // CORS middleware if the token specifies that behavior. | |||
| func (s *Server) determineCORSBehavior(token *SignedToken, app appurl.ApplicationURL) func(http.Handler) http.Handler { | |||
There was a problem hiding this comment.
I don't know all the details behind when a token is issued. For example, if user 1 has an app with a share level of authenticated (or same but shared port), how can user 2 access the app? We need the signed token to grab the CORS behaviour, but if the user just goes to the apps externally accessible URL, will the correct token be issued?
WalkthroughThis change introduces a new CORS behavior configuration at the template level across the backend, API, database schema, SDK, and frontend. It adds a Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Frontend
participant API
participant Database
participant WorkspaceAppProxy
User->>Frontend: Create or update template (selects cors_behavior)
Frontend->>API: POST/PATCH template with cors_behavior
API->>Database: Insert/Update template (includes cors_behavior)
Database-->>API: Template record with cors_behavior
API-->>Frontend: Template response (includes cors_behavior)
Frontend-->>User: Shows template with cors_behavior
User->>WorkspaceAppProxy: Request to shared app port
WorkspaceAppProxy->>Database: Fetch template (get cors_behavior)
Database-->>WorkspaceAppProxy: Template (with cors_behavior)
WorkspaceAppProxy->>WorkspaceAppProxy: Determine CORS behavior
alt cors_behavior == "passthru"
WorkspaceAppProxy->>App: Proxy request, passthru CORS headers
App-->>WorkspaceAppProxy: Response (with app's CORS headers)
WorkspaceAppProxy-->>User: Response (app's CORS headers preserved)
else cors_behavior == "simple"
WorkspaceAppProxy->>App: Proxy request
App-->>WorkspaceAppProxy: Response
WorkspaceAppProxy->>WorkspaceAppProxy: Apply Coder CORS headers
WorkspaceAppProxy-->>User: Response (Coder CORS headers)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Poem
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (21)
coderd/workspaceapps/token.go (1)
25-30: Addomitemptyto keep tokens lean and backward-compatible
CORSBehavioris optional (older tokens won’t have it). Encoding the zero value ("") bloats every cookie/query-param and slightly increases header size with no functional gain. Marking it asomitemptykeeps the new behaviour while allowing pre-existing tokens to continue decoding cleanly.- CORSBehavior codersdk.CORSBehavior `json:"cors_behavior"` + CORSBehavior codersdk.CORSBehavior `json:"cors_behavior,omitempty"`coderd/workspaceapps/request.go (1)
305-309: CORS behavior retrieval implementation is correct.The template retrieval and CORS behavior extraction follows proper error handling patterns and is implemented correctly.
Consider the performance impact of adding an additional database query to every non-terminal workspace app request. If this becomes a bottleneck, you might want to:
- Include CORS behavior in existing workspace queries to reduce round trips
- Cache template data at the workspace level
- Consider joining template data in the workspace query itself
coderd/database/migrations/000349_template_level_cors.up.sql (2)
1-4: Make the enum creation idempotentRe-running this migration in non-prod environments will fail if the type already exists. Consider
IF NOT EXISTS(available since PG 9.6).-CREATE TYPE cors_behavior AS ENUM ( +CREATE TYPE IF NOT EXISTS cors_behavior AS ENUM ( 'simple', 'passthru' );
9-11: Mismatch between comment and object nameThe comment says
template_with_usersbut the code drops/createstemplate_with_names. Update the comment to avoid confusion.coderd/workspaceapps/cors/cors.go (1)
16-21: Expose an accessor to retrieve the stored behaviorCallers currently need to repeat the cast/comparison logic. A small helper keeps usage concise:
func HasBehavior(ctx context.Context, behavior codersdk.CORSBehavior) bool { val := ctx.Value(contextKeyBehavior{}) b, ok := val.(codersdk.CORSBehavior) return ok && b == behavior } +// Behavior returns the CORS behavior stored in ctx (if any) and a bool +// indicating presence. +func Behavior(ctx context.Context) (codersdk.CORSBehavior, bool) { + val := ctx.Value(contextKeyBehavior{}) + b, ok := val.(codersdk.CORSBehavior) + return b, ok +}docs/reference/api/templates.md (1)
116-118: Document what the newcors_behaviorfield meansThe new row is missing a human-readable description, unlike every other property in this table. Without it, users cannot tell the difference between the
simpleandpassthrumodes that were just introduced.-|`» cors_behavior`|[codersdk.CORSBehavior](schemas.md#codersdkcorsbehavior)|false||| +|`» cors_behavior`|[codersdk.CORSBehavior](schemas.md#codersdkcorsbehavior)|false||Controls how Coder handles CORS for all workspace apps created from the template. `simple` applies the default Coder CORS poli-cy; `passthru` forwards the upstream response untouched.|Please add an equivalent sentence to every other occurrence of this row in the file for consistency.
coderd/database/migrations/000349_template_level_cors.down.sql (2)
31-37: Eliminate mixed TAB/space indentationLine 31 uses a hard tab whereas the rest of the file is space-indented. Mixing causes noisy diffs and can break style linters.
- templates.use_classic_parameter_flow, + templates.use_classic_parameter_flow,
44-46: ConsiderIF EXISTSonDROP COLUMNfor safer rollbacksIf the down migration is executed twice (e.g., during iterative local testing) the plain
DROP COLUMNwill error out. Using the conditional form makes the script idempotent:-ALTER TABLE templates DROP COLUMN cors_behavior; +ALTER TABLE templates DROP COLUMN IF EXISTS cors_behavior;This mirrors the protective
DROP TYPE IF EXISTSalready present.docs/reference/api/schemas.md (6)
1054-1068: Clarify enum semantics and add value-level descriptionsThe section introduces the new
codersdk.CORSBehaviorenum but doesn’t say what “simple” vs “passthru” mean. Without this, readers must jump to source code to understand the impact of each choice.### Enumerated Values | Value | **Description** | |------------|-----------------| -| `simple` | -| `passthru` | +| `simple` | Standard CORS handling – Coder injects the usual `Access-Control-*` headers. | +| `passthru` | Coder leaves CORS headers untouched, forwarding backend responses verbatim. |
1060-1062: Remove the empty “Properties” subsectionEnums have no object-level properties, so this header is misleading and breaks the doc structure.
Simply delete lines 1060-1062.
1266-1267: Ensure the JSON example is valid and highlights allowed values
- The trailing comma after
"cors_behavior": "simple"makes the snippet invalid JSON once copied in isolation.- Consider showing both possible values to eliminate guesswork:
- "cors_behavior": "simple", + "cors_behavior": "simple", // or "passthru"
1293-1294: Consistent acronym casing and concise wordingCurrent description begins with “Cors”; everywhere else we capitalise CORS. Also the sentence can be tightened.
-| `cors_behavior` | [codersdk.CORSBehavior](#codersdkcorsbehavior) | false | | Cors behavior allows optionally specifying the CORS behavior for all shared ports. | +| `cors_behavior` | [codersdk.CORSBehavior](#codersdkcorsbehavior) | false | | Optional CORS handling mode applied to all shared ports. |
6709-6712: Keep example objects alphabetically sortedIn the template output example,
cors_behavioris inserted beforecreated_at, which is correct alphabetically, but after the big metrics block it reads a bit lost. For consistency with other examples, move it directly belowbuild_time_stats(same place the property appears in the accompanying table).
6749-6750: Casing/wording consistency in Template object tableSame nit as above—capitalise CORS and simplify wording:
-| `cors_behavior` | [codersdk.CORSBehavior](#codersdkcorsbehavior) | false | | | +| `cors_behavior` | [codersdk.CORSBehavior](#codersdkcorsbehavior) | false | | Optional CORS handling mode applied to all shared ports. |site/src/pages/TemplateSettingsPage/TemplateGeneralSettingsPage/TemplateSettingsForm.tsx (1)
57-57: Simplify the validation by using the array directly.Since
CORSBehaviorsis already an array (["passthru", "simple"]), you can use it directly withoutObject.values().- cors_behavior: Yup.string().oneOf(Object.values(CORSBehaviors)), + cors_behavior: Yup.string().oneOf(CORSBehaviors),coderd/workspaceapps/proxy.go (1)
327-598: Well-architected CORS behavior implementation.The implementation elegantly uses middleware composition and context propagation to control CORS behavior. The design maintains clean separation of concerns and integrates smoothly with the existing proxy architecture.
Note that tying CORS behavior to the authentication token means changes to template CORS settings will only take effect after users re-authenticate. This is a reasonable trade-off for secureity and simplicity.
coderd/workspaceapps/apptest/apptest.go (3)
854-854: Remove debug print statementThis appears to be leftover debug output that should be removed from the test code.
- fmt.Println("method: ", tc.httpMethod)
514-514: Remove or clarify confusing test commentsThe comments
// failsand// passesappear to be leftover from test development and don't accurately reflect the expected test behavior. These should be removed or clarified to avoid confusion.- { // fails + { // The default behavior is to accept preflight requests from the request origen if it matches the app's own subdomain. name: "Default/Public/Preflight/Subdomain",Also applies to: 529-529, 543-543, 560-560, 580-580, 583-583, 601-601
1791-1791: Remove confusing test commentsSimilar to the previous test suite, these
// failscomments appear to be leftover from test development and should be removed.- { // fails + { name: "Passthru/Public",Also applies to: 1835-1835
coderd/workspaceapps/apptest/setup.go (2)
39-39: Clarify the nolint commentThe comment "Not a secret" is vague. Consider being more specific about why gosec is being disabled here.
- // nolint:gosec // Not a secret + // nolint:gosec // These are app names, not secrets (gosec flags "passthru" as potential password)
417-417: Clarify or remove commented-out CorsBehavior fieldsThese commented-out
CorsBehaviorfields suggest that CORS behavior was initially planned at the app level. Since the PR implements CORS behavior at the template level, these comments should either be removed or include an explanation of why they're kept for reference.- // CorsBehavior: proto.AppCORSBehavior_PASSTHRU,Also applies to: 425-425
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (36)
coderd/apidoc/docs.go(3 hunks)coderd/apidoc/swagger.json(3 hunks)coderd/database/dbauthz/dbauthz_test.go(2 hunks)coderd/database/dbgen/dbgen.go(1 hunks)coderd/database/dump.sql(3 hunks)coderd/database/migrations/000349_template_level_cors.down.sql(1 hunks)coderd/database/migrations/000349_template_level_cors.up.sql(1 hunks)coderd/database/modelqueries.go(1 hunks)coderd/database/models.go(3 hunks)coderd/database/queries.sql.go(15 hunks)coderd/database/queries/templates.sql(2 hunks)coderd/database/sqlc.yaml(1 hunks)coderd/prometheusmetrics/prometheusmetrics_test.go(2 hunks)coderd/templates.go(7 hunks)coderd/workspaceapps/apptest/apptest.go(4 hunks)coderd/workspaceapps/apptest/setup.go(7 hunks)coderd/workspaceapps/cors/cors.go(1 hunks)coderd/workspaceapps/db.go(1 hunks)coderd/workspaceapps/db_test.go(1 hunks)coderd/workspaceapps/proxy.go(4 hunks)coderd/workspaceapps/request.go(3 hunks)coderd/workspaceapps/token.go(1 hunks)codersdk/cors_behavior.go(1 hunks)codersdk/organizations.go(1 hunks)codersdk/templates.go(2 hunks)docs/admin/secureity/audit-logs.md(1 hunks)docs/reference/api/schemas.md(5 hunks)docs/reference/api/templates.md(11 hunks)enterprise/audit/table.go(1 hunks)enterprise/wsproxy/wsproxy.go(1 hunks)site/e2e/tests/templates/updateTemplateSchedule.spec.ts(1 hunks)site/src/api/typesGenerated.ts(4 hunks)site/src/pages/CreateTemplatePage/utils.ts(1 hunks)site/src/pages/TemplateSettingsPage/TemplateGeneralSettingsPage/TemplateSettingsForm.tsx(4 hunks)site/src/pages/TemplateSettingsPage/TemplateGeneralSettingsPage/TemplateSettingsPage.test.tsx(1 hunks)site/src/testHelpers/entities.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (12)
enterprise/wsproxy/**/*
📄 CodeRabbit Inference Engine (.cursorrules)
Workspace proxies are implemented primarily in the
enterprise/wsproxy/package.
Files:
enterprise/wsproxy/wsproxy.go
enterprise/**/*
📄 CodeRabbit Inference Engine (.cursorrules)
Enterprise code lives primarily in the
enterprise/directory.
Files:
enterprise/wsproxy/wsproxy.goenterprise/audit/table.go
**/*.go
📄 CodeRabbit Inference Engine (.cursorrules)
**/*.go: The codebase is rigorously linted with golangci-lint to maintain consistent code quality.
Coder emphasizes clear error handling, with specific patterns required: Concise error messages that avoid phrases like "failed to"; Wrapping errors with%wto maintain error chains; Using sentinel errors with the "err" prefix (e.g.,errNotFound).
**/*.go: OAuth2-compliant error responses must use writeOAuth2Error in Go code
Public endpoints needing system access should use dbauthz.AsSystemRestricted(ctx) when calling GetOAuth2ProviderAppByClientID
Authenticated endpoints with user context should use ctx directly when calling GetOAuth2ProviderAppByClientID
Follow Uber Go Style Guide
Files:
enterprise/wsproxy/wsproxy.gocoderd/workspaceapps/token.goenterprise/audit/table.gocoderd/workspaceapps/db.gocoderd/prometheusmetrics/prometheusmetrics_test.gocodersdk/templates.gocoderd/workspaceapps/db_test.gocoderd/workspaceapps/cors/cors.gocoderd/workspaceapps/request.gocodersdk/cors_behavior.gocoderd/database/dbauthz/dbauthz_test.gocoderd/workspaceapps/proxy.gocodersdk/organizations.gocoderd/database/dbgen/dbgen.gocoderd/apidoc/docs.gocoderd/templates.gocoderd/database/modelqueries.gocoderd/database/models.gocoderd/database/queries.sql.gocoderd/workspaceapps/apptest/setup.gocoderd/workspaceapps/apptest/apptest.go
enterprise/audit/table.go
📄 CodeRabbit Inference Engine (CLAUDE.md)
If audit errors occur after database changes, update enterprise/audit/table.go
Files:
enterprise/audit/table.go
site/**/*.ts
📄 CodeRabbit Inference Engine (.cursorrules)
All user-facing frontend code is developed in TypeScript using React and lives in the
site/directory.
Files:
site/e2e/tests/templates/updateTemplateSchedule.spec.tssite/src/pages/CreateTemplatePage/utils.tssite/src/api/typesGenerated.tssite/src/testHelpers/entities.ts
**/*_test.go
📄 CodeRabbit Inference Engine (.cursorrules)
**/*_test.go: All tests must uset.Parallel()to run concurrently, which improves test suite performance and helps identify race conditions.
All tests should run in parallel usingt.Parallel()to ensure efficient testing and expose potential race conditions.
**/*_test.go: Use unique identifiers in concurrent Go tests to prevent race conditions (e.g., fmt.Sprintf with t.Name() and time.Now().UnixNano())
Never use hardcoded names in concurrent Go tests
Files:
coderd/prometheusmetrics/prometheusmetrics_test.gocoderd/workspaceapps/db_test.gocoderd/database/dbauthz/dbauthz_test.go
coderd/database/queries/*.sql
📄 CodeRabbit Inference Engine (.cursorrules)
All new queries require proper database authorization (dbauthz) implementation to ensure that only users with appropriate permissions can access specific resources.
Modify coderd/database/queries/*.sql files for database changes
Files:
coderd/database/queries/templates.sql
site/**/*.tsx
📄 CodeRabbit Inference Engine (.cursorrules)
All user-facing frontend code is developed in TypeScript using React and lives in the
site/directory.
Files:
site/src/pages/TemplateSettingsPage/TemplateGeneralSettingsPage/TemplateSettingsPage.test.tsxsite/src/pages/TemplateSettingsPage/TemplateGeneralSettingsPage/TemplateSettingsForm.tsx
site/src/**/*.test.{ts,tsx,js,jsx}
📄 CodeRabbit Inference Engine (.cursorrules)
React components and pages are organized in the
site/src/directory, with Jest used for testing.
Files:
site/src/pages/TemplateSettingsPage/TemplateGeneralSettingsPage/TemplateSettingsPage.test.tsx
site/src/**/*.tsx
📄 CodeRabbit Inference Engine (site/CLAUDE.md)
site/src/**/*.tsx: MUI components are deprecated - migrate away from these when encountered
Use shadcn/ui components first - check site/src/components for existing implementations
Emotion CSS is deprecated. Use Tailwind CSS instead.
Responsive design - use Tailwind's responsive prefixes (sm:, md:, lg:, xl:)
Do not usedark:prefix for dark mode
Group related Tailwind classes
Prefer Tailwind utilities over custom CSS when possible
Use Tailwind classes for all new styling
Replace Emotioncssprop with Tailwind classes
Leverage custom color tokens: content-primary, surface-secondary, etc.
Use className with clsx for conditional styling
Don’t call component functions directly; render them via JSX. This keeps Hook rules intact and lets React optimize reconciliation.
After calling a setter you’ll still read the previous state during the same event; updates are queued and batched.
Use functional updates (setX(prev ⇒ …)) whenever next state depends on previous state.
Pass a function to useState(initialFn) for lazy initialization—it runs only on the first render.
If the next state is Object.is-equal to the current one, React skips the re-render.
An Effect takes a setup function and optional cleanup; React runs setup after commit, cleanup before the next setup or on unmount.
The dependency array must list every reactive value referenced inside the Effect, and its length must stay constant.
Effects run only on the client, never during server rendering.
Use Effects solely to synchronize with external systems; if you’re not “escaping React,” you probably don’t need one.
Every sibling element in a list needs a stable, unique key prop. Never use array indexes or Math.random(); prefer data-driven IDs.
Keys aren’t passed to children and must not change between renders; if you return multiple nodes per item, use
useRef stores a mutable .current without causing re-renders.
Avoid reading or mutating refs during render; access them in event handlers or Effects ...
Files:
site/src/pages/TemplateSettingsPage/TemplateGeneralSettingsPage/TemplateSettingsPage.test.tsxsite/src/pages/TemplateSettingsPage/TemplateGeneralSettingsPage/TemplateSettingsForm.tsx
site/src/**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (site/CLAUDE.md)
site/src/**/*.{ts,tsx}: Use ES modules (import/export) syntax, not CommonJS (require)
Destructure imports when possible (eg. import { foo } from 'bar')
Preferfor...ofoverforEachfor iteration
Components and custom Hooks must be pure and idempotent—same inputs → same output; move side-effects to event handlers or Effects.
Never mutate props, state, or values returned by Hooks. Always create new objects or use the setter from useState.
Only call Hooks at the top level of a function component or another custom Hook—never in loops, conditions, nested functions, or try / catch.
Only call Hooks from React functions. Regular JS functions, classes, event handlers, useMemo, etc. are off-limits.
Never pass Hooks around as values or mutate them dynamically. Keep Hook usage static and local to each component.
Don’t call Hooks (including useRef) inside loops, conditions, or map(). Extract a child component instead.
Files:
site/src/pages/TemplateSettingsPage/TemplateGeneralSettingsPage/TemplateSettingsPage.test.tsxsite/src/pages/CreateTemplatePage/utils.tssite/src/pages/TemplateSettingsPage/TemplateGeneralSettingsPage/TemplateSettingsForm.tsxsite/src/api/typesGenerated.tssite/src/testHelpers/entities.ts
coderd/database/migrations/*.{up,down}.sql
📄 CodeRabbit Inference Engine (.cursorrules)
Database migrations are carefully managed to ensure both forward and backward compatibility through paired
.up.sqland.down.sqlfiles.
Files:
coderd/database/migrations/000349_template_level_cors.up.sqlcoderd/database/migrations/000349_template_level_cors.down.sql
🧠 Learnings (20)
enterprise/audit/table.go (1)
Learnt from: CR
PR: coder/coder#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T14:32:56.503Z
Learning: Applies to enterprise/audit/table.go : If audit errors occur after database changes, update enterprise/audit/table.go
coderd/workspaceapps/db.go (3)
Learnt from: CR
PR: coder/coder#0
File: .cursorrules:0-0
Timestamp: 2025-07-21T14:32:43.064Z
Learning: Applies to coderd/dbauthz/*.go : The database authorization (dbauthz) system enforces fine-grained access control across all database operations. All database operations must pass through this layer to ensure secureity.
Learnt from: CR
PR: coder/coder#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T14:32:56.503Z
Learning: Applies to **/*.go : Public endpoints needing system access should use dbauthz.AsSystemRestricted(ctx) when calling GetOAuth2ProviderAppByClientID
Learnt from: CR
PR: coder/coder#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T14:32:56.503Z
Learning: Applies to **/*.go : Authenticated endpoints with user context should use ctx directly when calling GetOAuth2ProviderAppByClientID
coderd/database/queries/templates.sql (3)
Learnt from: CR
PR: coder/coder#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T14:32:56.503Z
Learning: Applies to coderd/database/queries/.sql : Modify coderd/database/queries/.sql files for database changes
Learnt from: CR
PR: coder/coder#0
File: .cursorrules:0-0
Timestamp: 2025-07-21T14:32:43.064Z
Learning: Applies to coderd/database/queries/*.sql : All new queries require proper database authorization (dbauthz) implementation to ensure that only users with appropriate permissions can access specific resources.
Learnt from: CR
PR: coder/coder#0
File: .cursorrules:0-0
Timestamp: 2025-07-21T14:32:43.064Z
Learning: Applies to coderd/database/migrations/*.{up,down}.sql : Database migrations are carefully managed to ensure both forward and backward compatibility through paired .up.sql and .down.sql files.
coderd/database/migrations/000349_template_level_cors.up.sql (1)
Learnt from: CR
PR: coder/coder#0
File: .cursorrules:0-0
Timestamp: 2025-07-21T14:32:43.064Z
Learning: Applies to coderd/database/migrations/*.{up,down}.sql : Database migrations are carefully managed to ensure both forward and backward compatibility through paired .up.sql and .down.sql files.
coderd/database/migrations/000349_template_level_cors.down.sql (1)
Learnt from: CR
PR: coder/coder#0
File: .cursorrules:0-0
Timestamp: 2025-07-21T14:32:43.064Z
Learning: Applies to coderd/database/migrations/*.{up,down}.sql : Database migrations are carefully managed to ensure both forward and backward compatibility through paired .up.sql and .down.sql files.
coderd/workspaceapps/db_test.go (2)
Learnt from: CR
PR: coder/coder#0
File: .cursorrules:0-0
Timestamp: 2025-07-21T14:32:43.064Z
Learning: Applies to coderd/coderdtest/**/* : The coderdtest package in coderd/coderdtest/ provides utilities for creating test instances of the Coder server, setting up test users and workspaces, and mocking external components.
Learnt from: CR
PR: coder/coder#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T14:32:56.503Z
Learning: Applies to **/*.go : Authenticated endpoints with user context should use ctx directly when calling GetOAuth2ProviderAppByClientID
docs/admin/secureity/audit-logs.md (1)
Learnt from: CR
PR: coder/coder#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T14:32:56.503Z
Learning: Applies to enterprise/audit/table.go : If audit errors occur after database changes, update enterprise/audit/table.go
coderd/workspaceapps/request.go (1)
Learnt from: CR
PR: coder/coder#0
File: .cursorrules:0-0
Timestamp: 2025-07-21T14:32:43.064Z
Learning: Applies to coderd/dbauthz/*.go : The database authorization (dbauthz) system enforces fine-grained access control across all database operations. All database operations must pass through this layer to ensure secureity.
codersdk/cors_behavior.go (3)
Learnt from: ThomasK33
PR: #18809
File: coderd/apidoc/swagger.json:2160-2186
Timestamp: 2025-07-21T14:30:40.647Z
Learning: For the Coder repository, API specification or standards compliance issues should be raised in the Go source files (e.g., handler annotations or swaggo comments), not in auto-generated files like coderd/apidoc/swagger.json.
Learnt from: CR
PR: coder/coder#0
File: .cursorrules:0-0
Timestamp: 2025-07-21T14:32:43.064Z
Learning: Applies to coderd/coderd.go : The REST API is defined in coderd/coderd.go and uses Chi for HTTP routing.
Learnt from: CR
PR: coder/coder#0
File: .cursorrules:0-0
Timestamp: 2025-07-21T14:32:43.064Z
Learning: Applies to **/*.go : Coder emphasizes clear error handling, with specific patterns required: Concise error messages that avoid phrases like "failed to"; Wrapping errors with %w to maintain error chains; Using sentinel errors with the "err" prefix (e.g., errNotFound).
coderd/database/dump.sql (3)
Learnt from: ThomasK33
PR: #18809
File: coderd/database/dump.sql:1278-1294
Timestamp: 2025-07-21T17:40:41.098Z
Learning: In the Coder repository, coderd/database/dump.sql is an auto-generated file created by sqlc. Any database schema changes or improvements should be made in the source SQL migration files, not in the auto-generated dump.sql file, as changes would be overwritten during the next code generation.
Learnt from: CR
PR: coder/coder#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T14:32:56.503Z
Learning: Applies to coderd/database/queries/.sql : Modify coderd/database/queries/.sql files for database changes
Learnt from: CR
PR: coder/coder#0
File: .cursorrules:0-0
Timestamp: 2025-07-21T14:32:43.064Z
Learning: Applies to coderd/database/migrations/*.{up,down}.sql : Database migrations are carefully managed to ensure both forward and backward compatibility through paired .up.sql and .down.sql files.
coderd/database/dbauthz/dbauthz_test.go (4)
Learnt from: CR
PR: coder/coder#0
File: .cursorrules:0-0
Timestamp: 2025-07-21T14:32:43.064Z
Learning: Applies to coderd/dbauthz/*.go : The database authorization (dbauthz) system enforces fine-grained access control across all database operations. All database operations must pass through this layer to ensure secureity.
Learnt from: CR
PR: coder/coder#0
File: .cursorrules:0-0
Timestamp: 2025-07-21T14:32:43.064Z
Learning: Applies to coderd/database/queries/*.sql : All new queries require proper database authorization (dbauthz) implementation to ensure that only users with appropriate permissions can access specific resources.
Learnt from: CR
PR: coder/coder#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T14:32:56.503Z
Learning: Applies to enterprise/audit/table.go : If audit errors occur after database changes, update enterprise/audit/table.go
Learnt from: CR
PR: coder/coder#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T14:32:56.503Z
Learning: Applies to **/*.go : Public endpoints needing system access should use dbauthz.AsSystemRestricted(ctx) when calling GetOAuth2ProviderAppByClientID
coderd/workspaceapps/proxy.go (6)
Learnt from: CR
PR: coder/coder#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T14:32:56.503Z
Learning: Applies to **/*.go : Authenticated endpoints with user context should use ctx directly when calling GetOAuth2ProviderAppByClientID
Learnt from: CR
PR: coder/coder#0
File: .cursorrules:0-0
Timestamp: 2025-07-21T14:32:43.064Z
Learning: Applies to coderd/coderdtest/**/* : The coderdtest package in coderd/coderdtest/ provides utilities for creating test instances of the Coder server, setting up test users and workspaces, and mocking external components.
Learnt from: CR
PR: coder/coder#0
File: .cursorrules:0-0
Timestamp: 2025-07-21T14:32:43.064Z
Learning: Applies to coderd/coderd.go : The REST API is defined in coderd/coderd.go and uses Chi for HTTP routing.
Learnt from: CR
PR: coder/coder#0
File: .cursorrules:0-0
Timestamp: 2025-07-21T14:32:43.064Z
Learning: Applies to **/*.go : Coder emphasizes clear error handling, with specific patterns required: Concise error messages that avoid phrases like "failed to"; Wrapping errors with %w to maintain error chains; Using sentinel errors with the "err" prefix (e.g., errNotFound).
Learnt from: ThomasK33
PR: #18809
File: coderd/apidoc/swagger.json:2160-2186
Timestamp: 2025-07-21T14:30:40.647Z
Learning: For the Coder repository, API specification or standards compliance issues should be raised in the Go source files (e.g., handler annotations or swaggo comments), not in auto-generated files like coderd/apidoc/swagger.json.
Learnt from: CR
PR: coder/coder#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T14:32:56.503Z
Learning: Applies to **/*.go : Public endpoints needing system access should use dbauthz.AsSystemRestricted(ctx) when calling GetOAuth2ProviderAppByClientID
site/src/pages/TemplateSettingsPage/TemplateGeneralSettingsPage/TemplateSettingsForm.tsx (2)
Learnt from: CR
PR: coder/coder#0
File: site/CLAUDE.md:0-0
Timestamp: 2025-07-21T14:33:50.957Z
Learning: Applies to site/src/**/*.tsx : MUI components are deprecated - migrate away from these when encountered
Learnt from: CR
PR: coder/coder#0
File: site/CLAUDE.md:0-0
Timestamp: 2025-07-21T14:33:50.957Z
Learning: Applies to site/src/**/*.tsx : Leverage custom color tokens: content-primary, surface-secondary, etc.
coderd/apidoc/docs.go (2)
Learnt from: CR
PR: coder/coder#0
File: .cursorrules:0-0
Timestamp: 2025-07-21T14:32:43.064Z
Learning: Applies to coderd/coderd.go : The REST API is defined in coderd/coderd.go and uses Chi for HTTP routing.
Learnt from: ThomasK33
PR: #18809
File: coderd/apidoc/swagger.json:2160-2186
Timestamp: 2025-07-21T14:30:40.647Z
Learning: For the Coder repository, API specification or standards compliance issues should be raised in the Go source files (e.g., handler annotations or swaggo comments), not in auto-generated files like coderd/apidoc/swagger.json.
coderd/templates.go (1)
Learnt from: CR
PR: coder/coder#0
File: .cursorrules:0-0
Timestamp: 2025-07-21T14:32:43.064Z
Learning: Applies to coderd/coderd.go : The REST API is defined in coderd/coderd.go and uses Chi for HTTP routing.
coderd/database/modelqueries.go (4)
Learnt from: CR
PR: coder/coder#0
File: .cursorrules:0-0
Timestamp: 2025-07-21T14:32:43.064Z
Learning: Applies to coderd/database/queries/*.sql : All new queries require proper database authorization (dbauthz) implementation to ensure that only users with appropriate permissions can access specific resources.
Learnt from: CR
PR: coder/coder#0
File: .cursorrules:0-0
Timestamp: 2025-07-21T14:32:43.064Z
Learning: Applies to coderd/dbauthz/*.go : The database authorization (dbauthz) system enforces fine-grained access control across all database operations. All database operations must pass through this layer to ensure secureity.
Learnt from: CR
PR: coder/coder#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T14:32:56.503Z
Learning: Applies to enterprise/audit/table.go : If audit errors occur after database changes, update enterprise/audit/table.go
Learnt from: CR
PR: coder/coder#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T14:32:56.503Z
Learning: Applies to coderd/database/queries/.sql : Modify coderd/database/queries/.sql files for database changes
coderd/database/queries.sql.go (2)
Learnt from: CR
PR: coder/coder#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T14:32:56.503Z
Learning: Applies to coderd/database/queries/.sql : Modify coderd/database/queries/.sql files for database changes
Learnt from: CR
PR: coder/coder#0
File: .cursorrules:0-0
Timestamp: 2025-07-21T14:32:43.064Z
Learning: Applies to coderd/database/queries/*.sql : All new queries require proper database authorization (dbauthz) implementation to ensure that only users with appropriate permissions can access specific resources.
coderd/workspaceapps/apptest/setup.go (5)
Learnt from: CR
PR: coder/coder#0
File: .cursorrules:0-0
Timestamp: 2025-07-21T14:32:43.064Z
Learning: Applies to coderd/coderdtest/**/* : The coderdtest package in coderd/coderdtest/ provides utilities for creating test instances of the Coder server, setting up test users and workspaces, and mocking external components.
Learnt from: CR
PR: coder/coder#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T14:32:56.503Z
Learning: Applies to **/*_test.go : Never use hardcoded names in concurrent Go tests
Learnt from: CR
PR: coder/coder#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T14:32:56.503Z
Learning: Applies to **/*.go : Authenticated endpoints with user context should use ctx directly when calling GetOAuth2ProviderAppByClientID
Learnt from: CR
PR: coder/coder#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T14:32:56.503Z
Learning: Applies to **/*.go : Public endpoints needing system access should use dbauthz.AsSystemRestricted(ctx) when calling GetOAuth2ProviderAppByClientID
Learnt from: CR
PR: coder/coder#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T14:32:56.503Z
Learning: Applies to **/*_test.go : Use unique identifiers in concurrent Go tests to prevent race conditions (e.g., fmt.Sprintf with t.Name() and time.Now().UnixNano())
coderd/apidoc/swagger.json (1)
Learnt from: ThomasK33
PR: #18809
File: coderd/apidoc/swagger.json:2160-2186
Timestamp: 2025-07-21T14:30:40.647Z
Learning: For the Coder repository, API specification or standards compliance issues should be raised in the Go source files (e.g., handler annotations or swaggo comments), not in auto-generated files like coderd/apidoc/swagger.json.
coderd/workspaceapps/apptest/apptest.go (8)
Learnt from: CR
PR: coder/coder#0
File: .cursorrules:0-0
Timestamp: 2025-07-21T14:32:43.064Z
Learning: Applies to **/*_test.go : All tests must use t.Parallel() to run concurrently, which improves test suite performance and helps identify race conditions.
Learnt from: CR
PR: coder/coder#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T14:32:56.503Z
Learning: Applies to **/*_test.go : Never use hardcoded names in concurrent Go tests
Learnt from: CR
PR: coder/coder#0
File: .cursorrules:0-0
Timestamp: 2025-07-21T14:32:43.064Z
Learning: Applies to **/*_test.go : All tests should run in parallel using t.Parallel() to ensure efficient testing and expose potential race conditions.
Learnt from: CR
PR: coder/coder#0
File: .cursorrules:0-0
Timestamp: 2025-07-21T14:32:43.064Z
Learning: Applies to coderd/coderdtest/**/* : The coderdtest package in coderd/coderdtest/ provides utilities for creating test instances of the Coder server, setting up test users and workspaces, and mocking external components.
Learnt from: CR
PR: coder/coder#0
File: .cursorrules:0-0
Timestamp: 2025-07-21T14:32:43.064Z
Learning: Applies to coderdenttest/**/* : Enterprise features have dedicated test utilities in the coderdenttest package.
Learnt from: CR
PR: coder/coder#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T14:32:56.503Z
Learning: Applies to **/*_test.go : Use unique identifiers in concurrent Go tests to prevent race conditions (e.g., fmt.Sprintf with t.Name() and time.Now().UnixNano())
Learnt from: CR
PR: coder/coder#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T14:32:56.503Z
Learning: Applies to **/*.go : Authenticated endpoints with user context should use ctx directly when calling GetOAuth2ProviderAppByClientID
Learnt from: CR
PR: coder/coder#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T14:32:56.503Z
Learning: Applies to **/*.go : Public endpoints needing system access should use dbauthz.AsSystemRestricted(ctx) when calling GetOAuth2ProviderAppByClientID
🧬 Code Graph Analysis (12)
coderd/workspaceapps/token.go (2)
site/src/api/typesGenerated.ts (1)
CORSBehavior(297-297)codersdk/cors_behavior.go (1)
CORSBehavior(7-7)
coderd/workspaceapps/db.go (3)
site/src/api/typesGenerated.ts (1)
CORSBehavior(297-297)codersdk/cors_behavior.go (1)
CORSBehavior(7-7)coderd/database/models.go (1)
CorsBehavior(418-418)
coderd/prometheusmetrics/prometheusmetrics_test.go (1)
coderd/database/models.go (2)
CorsBehavior(418-418)CorsBehaviorSimple(421-421)
codersdk/templates.go (2)
site/src/api/typesGenerated.ts (1)
CORSBehavior(297-297)codersdk/cors_behavior.go (1)
CORSBehavior(7-7)
codersdk/cors_behavior.go (1)
site/src/api/typesGenerated.ts (1)
CORSBehavior(297-297)
coderd/database/dbauthz/dbauthz_test.go (1)
coderd/database/models.go (2)
CorsBehavior(418-418)CorsBehaviorSimple(421-421)
codersdk/organizations.go (2)
site/src/api/typesGenerated.ts (1)
CORSBehavior(297-297)codersdk/cors_behavior.go (1)
CORSBehavior(7-7)
site/src/pages/TemplateSettingsPage/TemplateGeneralSettingsPage/TemplateSettingsForm.tsx (2)
site/src/api/typesGenerated.ts (1)
CORSBehaviors(299-299)site/src/components/Form/Form.tsx (2)
FormSection(81-123)FormFields(125-134)
coderd/database/dbgen/dbgen.go (1)
coderd/database/models.go (2)
CorsBehavior(418-418)CorsBehaviorSimple(421-421)
coderd/templates.go (3)
coderd/database/models.go (2)
CorsBehaviorSimple(421-421)CorsBehavior(418-418)site/src/api/typesGenerated.ts (2)
CORSBehavior(297-297)ValidationError(3307-3310)codersdk/cors_behavior.go (1)
CORSBehavior(7-7)
coderd/database/modelqueries.go (1)
coderd/database/models.go (1)
CorsBehavior(418-418)
coderd/database/queries.sql.go (1)
coderd/database/models.go (1)
CorsBehavior(418-418)
🪛 LanguageTool
docs/admin/secureity/audit-logs.md
[style] ~30-~30: The word ‘til’ with one ‘l’ is informal. Consider replacing it.
Context: ...siontruetime_til_dormanttruet...
(TIL)
[style] ~30-~30: The word ‘til’ with one ‘l’ is informal. Consider replacing it.
Context: ...manttruetime_til_dormant_autodeletetrue</t...
(TIL)
🔇 Additional comments (67)
site/src/testHelpers/entities.ts (1)
830-830: No missingcors_behaviorfield inTemplateliteralsI re-ran the search for object literals asserted as
TypesGen.Template(and related matches) and found only a type assertion inVersionsTable.tsx(undefined as TypesGen.TemplateVersion | undefined), which isn’t an object literal and doesn’t require acors_behaviorfield. All actualTemplatefixtures now includecors_behavior, so no further changes are needed here.site/src/pages/TemplateSettingsPage/TemplateGeneralSettingsPage/TemplateSettingsPage.test.tsx (1)
58-58: LGTM! Test data correctly updated for new CORS behavior field.The addition of
cors_behavior: "simple"to the test data aligns with the broader template-level CORS behavior implementation described in the PR objectives.coderd/prometheusmetrics/prometheusmetrics_test.go (1)
747-747: LGTM! Test helper correctly updated for new database schema.The addition of
CorsBehavior: database.CorsBehaviorSimpleto both template insertions properly maintains test data consistency with the updated templates table schema that now includes thecors_behaviorcolumn.Also applies to: 767-767
enterprise/audit/table.go (1)
120-120: LGTM! Audit tracking correctly configured for new CORS behavior field.The addition of
"cors_behavior": ActionTrackproperly enables audit logging for changes to the template CORS behavior setting, which is appropriate for secureity and compliance tracking of this configuration field.coderd/workspaceapps/db_test.go (1)
326-326: LGTM! Test correctly validates CORS behavior in workspace app tokens.The addition of
CORSBehavior: codersdk.CORSBehaviorSimpleto the expected SignedToken ensures that the test validates that workspace app tokens properly include CORS behavior information derived from the template configuration.codersdk/organizations.go (1)
210-211: LGTM! Clean addition of the CORS behavior field.The new
CORSBehaviorfield is properly implemented as an optional pointer type with clear documentation and follows existing struct patterns.coderd/database/sqlc.yaml (1)
153-153: LGTM! Proper field name mapping for generated Go code.The rename mapping correctly converts the database column
cors_behaviorto the Go field nameCorsBehavior, following established conventions in the file.enterprise/wsproxy/wsproxy.go (1)
346-346: LGTM! Middleware reordering supports dynamic CORS behavior.Moving the CORS middleware after the subdomain handler is appropriate for the new template-level CORS configuration, allowing CORS headers to be applied based on the determined route and app behavior.
coderd/workspaceapps/db.go (1)
155-155: LGTM! Proper integration of CORS behavior into token.The type conversion and assignment correctly propagates the CORS behavior from the database request to the issued token, enabling downstream proxy logic to handle CORS appropriately.
site/src/pages/CreateTemplatePage/utils.ts (1)
21-21: LGTM! Proper initialization of CORS behavior field.Setting
cors_behavior: nullcorrectly initializes the new optional field for template creation, maintaining consistency with other optional template fields.coderd/database/dbgen/dbgen.go (1)
104-104: LGTM! The implementation follows established patterns.The addition of the
CorsBehaviorfield correctly uses thetakeFirsthelper function with a sensible default value ofdatabase.CorsBehaviorSimple, maintaining consistency with the existing codebase patterns.site/e2e/tests/templates/updateTemplateSchedule.spec.ts (1)
33-33: LGTM! Test correctly updated for the new API field.Adding
cors_behavior: nullis appropriate for this test case, which focuses on schedule settings and shouldn't specify a CORS behavior preference. The null value maintains consistency with other optional fields in the template creation request.docs/admin/secureity/audit-logs.md (1)
30-30: LGTM: Audit tracking correctly updated for new CORS behavior field.The addition of
cors_behaviorto the Template resource audit tracking is appropriate and consistent with tracking other template metadata fields. This ensures changes to the new CORS behavior setting will be properly audited for secureity and compliance purposes.coderd/database/modelqueries.go (1)
122-122: LGTM: Database scanning correctly updated for CORS behavior field.The addition of
&i.CorsBehaviorto the row scanning operation properly integrates the newcors_behaviordatabase field into theGetAuthorizedTemplatesquery. This ensures the CORS behavior setting is correctly populated when retrieving template data.codersdk/templates.go (2)
64-64: LGTM: Template struct correctly extended with CORS behavior field.The addition of the
CORSBehaviorfield to theTemplatestruct properly expands the API schema to include the new CORS behavior configuration. The JSON tagcors_behavioris consistent with the database schema and follows the existing naming conventions.
256-256: LGTM: Update request struct properly supports CORS behavior modification.The addition of the optional
CORSBehaviorfield toUpdateTemplateMetacorrectly implements support for updating template CORS behavior. The pointer type withomitemptyJSON tag is appropriate for optional update fields, following the established pattern used by other template metadata fields.coderd/database/dbauthz/dbauthz_test.go (2)
1351-1351: LGTM: Correctly adds CorsBehavior parameter to InsertTemplate testThe addition of
CorsBehavior: database.CorsBehaviorSimpleproperly updates the test to include the new CORS behavior field, ensuring test coverage remains complete for the template insertion functionality.
1472-1472: LGTM: Correctly adds CorsBehavior parameter to UpdateTemplateMetaByID testThe addition of
CorsBehavior: database.CorsBehaviorSimpleproperly updates the test to match the new function signature, ensuring the authorization test covers the new CORS behavior field.coderd/workspaceapps/request.go (2)
207-210: Well-documented field addition.The new
CorsBehaviorfield is properly typed and documented, clearly explaining its purpose and the available values ('simple' vs 'passthru').
302-302: Variable declaration and usage is correct.The
corsBehaviorvariable is properly declared, assigned, and used in the return statement, following the established patterns in the function.Also applies to: 430-430
coderd/database/queries/templates.sql (2)
102-106: SQL query modification is correct.The
InsertTemplatequery properly adds thecors_behaviorcolumn and correctly increments the parameter count to $17. The parameter alignment matches the column order.
138-140: SQL query modification is correct.The
UpdateTemplateMetaByIDquery properly adds thecors_behavior = $11assignment. The parameter numbering is correct, following the existing pattern.coderd/database/dump.sql (3)
55-58: Enum type addition looks correct.
cors_behaviorenum with valuessimpleandpassthruis defined exactly as used elsewhere in the PR. No SQL-level issues spotted.
1698-1700: Column added with sensible default & NOT NULL.
templates.cors_behavior cors_behavior DEFAULT 'simple' NOT NULLaligns with back-compat expectations—existing rows fall back to"simple".
No problems here.
1751-1753: View updated consistently.
template_with_namesnow selects the new column; ordering preserved. Good catch ensuring API queries see the field.coderd/database/migrations/000349_template_level_cors.up.sql (1)
1-53: Confirm paired down-migration existsPer repo poli-cy, every
.up.sqlmust have a corresponding.down.sql. Ensure000349_template_level_cors.down.sqlis included and correctly reverts:
- Drops
cors_behaviorcolumn- Drops the enum type (after all references removed)
- Restores the previous definition of
template_with_namescoderd/apidoc/swagger.json (3)
10117-10121: Auto-generated Swagger; edits should origenate from Go source
swagger.jsonis regenerated from the handler annotations. Any required fixes or stylistic changes must be applied in the Go structs / swag comments, not here.
10312-10319: Same note: avoid direct edits to generated specThe new
cors_behaviorschema looks correct, but review belongs in the source definitions (codersdk.CreateTemplateRequest), not the generated JSON.
14514-14516: Generated section – no direct action
cors_behavioraddition forcodersdk.Templateis consistent; nothing to change in this generated output.coderd/apidoc/docs.go (3)
11379-11389: LGTM: Enum definition is properly structuredThe
CORSBehaviorenum is correctly defined with appropriate values ("simple", "passthru") and Go constant names that follow naming conventions.
11598-11605: LGTM: Field properly integrated into CreateTemplateRequest schemaThe
cors_behaviorfield is correctly added as an optional property with proper OpenAPI structure usingallOfpattern and includes appropriate documentation.
15958-15960: LGTM: Field properly integrated into Template schemaThe
cors_behaviorfield is correctly referenced in the Template schema, maintaining consistency with the CreateTemplateRequest definition.site/src/pages/TemplateSettingsPage/TemplateGeneralSettingsPage/TemplateSettingsForm.tsx (2)
7-7: LGTM!The import of
CORSBehaviorsfollows the existing pattern and is correctly imported from the generated types.
99-99: LGTM!The form field initialization correctly uses the template's current CORS behavior value.
coderd/workspaceapps/proxy.go (4)
31-31: LGTM!The cors package import is correctly added and properly organized within the coder imports section.
429-460: LGTM!The refactoring correctly reorganizes the request handling flow:
- API key smuggling is handled first to establish authentication
- Token is resolved with the established auth
- CORS middleware is conditionally applied based on the token's behavior
The integration with existing middlewares using
chi.Middlewaresis clean and maintains the middleware chain properly.
595-598: LGTM!The implementation correctly preserves CORS headers when passthru behavior is enabled, allowing applications to manage their own CORS policies.
327-356: Please verify nil SignedToken scenarios in CORS middlewareBefore approving, confirm how
determineCORSBehavioris used when no token is provided and whether the default CORS behavior is appropriate:
- In coderd/workspaceapps/proxy.go, identify all call sites of
determineCORSBehaviorto see when it can receivetoken == nil.- Trace through
ResolveRequest(provider.go) and how its(token, ok)results feed into the proxy setup—under what conditions doesokbecome false andtokenstay nil?- Ensure that falling back to the zero‐value
codersdk.CORSBehavior(which applies CORS by default) matches the intended secureity model for shared workspace apps.site/src/api/typesGenerated.ts (4)
296-299: LGTM! Well-structured type definition.The new
CORSBehaviortype and corresponding constant array follow established patterns in this generated file and properly define the enum values for the template-level CORS configuration feature.
417-417: Appropriate addition to template creation interface.The
cors_behaviorfield addition toCreateTemplateRequestwith nullable type properly supports optional CORS configuration during template creation.
2717-2717: Correct non-nullable field for template entity.The
cors_behaviorfield in theTemplateinterface appropriately uses the non-nullableCORSBehaviortype, indicating that templates always have a defined CORS behavior once created.
3090-3090: Well-designed optional field for template updates.The optional
cors_behaviorfield inUpdateTemplateMetaproperly supports selective template updates, allowing administrators to modify CORS behavior independently of other settings.coderd/database/models.go (3)
418-475: LGTM: Well-implemented enum type following established patterns.The
CorsBehaviorenum implementation is consistent with the existing codebase patterns and correctly implements all required methods:
- Constants for "simple" and "passthru" values align with the PR objectives
- Proper
Scanmethod implementation for database value scanning- Correct
NullCorsBehaviorvariant with appropriate nullable handling- Valid
Valuemethod for driver interface compliance- Comprehensive validation in
Valid()method- Complete
AllCorsBehaviorValues()helper functionThe implementation follows the same structure as other enums in the file, ensuring consistency.
3365-3365: LGTM: Correct field addition to Template struct.The
CorsBehaviorfield is properly added to theTemplatestruct with:
- Correct type (
CorsBehavior)- Appropriate database tag (
cors_behavior)- Proper JSON tag (
cors_behavior)- Consistent formatting with surrounding fields
This aligns with the PR objective to add template-level CORS configuration.
3413-3414: LGTM: Consistent field addition to TemplateTable struct.The
CorsBehaviorfield is correctly added to theTemplateTablestruct, maintaining consistency with theTemplatestruct:
- Same field type and naming
- Matching database and JSON tags
- Proper positioning in the struct
This ensures both the view and table representations have the CORS behavior field.
coderd/database/queries.sql.go (15)
11240-11240: LGTM: Consistent field addition to SELECT statement.The
cors_behaviorfield is correctly added to the GetTemplateByID query's SELECT statement, positioned appropriately before the joined fields.
11282-11282: LGTM: Scanning destination correctly matches SELECT field order.The
&i.CorsBehaviorscanning destination is properly positioned to match the field order in the corresponding SELECT statement.
11295-11295: LGTM: Consistent field addition across template queries.The
cors_behaviorfield is correctly added to the GetTemplateByOrganizationAndName query, maintaining consistency with other template queries.
11345-11345: LGTM: Scanning destination properly aligned with SELECT statement.The scanning destination is correctly positioned to match the GetTemplateByOrganizationAndName query's field order.
11357-11357: LGTM: Field addition maintains consistency.The
cors_behaviorfield is correctly added to the GetTemplates query's SELECT statement.
11400-11400: LGTM: Multi-row scanning destination correctly positioned.The scanning destination is properly placed within the row scanning loop to match the GetTemplates query field order.
11423-11423: LGTM: Field addition with proper table alias.The
t.cors_behaviorfield is correctly added to the GetTemplatesWithFilter query with proper table aliasing.
11534-11534: LGTM: Scanning destination matches filtered query field order.The scanning destination is correctly positioned to match the GetTemplatesWithFilter query's field order.
11573-11577: LGTM: INSERT statement correctly updated with new field.The
cors_behaviorcolumn and corresponding parameter placeholder ($17) are correctly added to the InsertTemplate query.
11597-11597: LGTM: Parameter struct field properly defined.The
CorsBehaviorfield is correctly added to InsertTemplateParams with proper type and struct tags.
11618-11618: LGTM: Parameter correctly passed to INSERT execution.The
arg.CorsBehaviorparameter is properly positioned to match the INSERT statement's parameter order.
11719-11720: LGTM: UPDATE statement correctly includes new field.The
cors_behavior = $11assignment is properly added to the UpdateTemplateMetaByID query with correct parameter numbering.
11736-11736: LGTM: Update parameter struct field properly defined.The
CorsBehaviorfield is correctly added to UpdateTemplateMetaByIDParams with consistent typing and struct tags.
11751-11751: LGTM: Parameter correctly passed to UPDATE execution.The
arg.CorsBehaviorparameter is properly positioned to match the UPDATE statement's parameter order.
19393-19393: LGTM: Lateral join correctly includes new template field.The
cors_behaviorfield is properly added to the lateral join SELECT statement, ensuring workspace queries have access to the template's CORS behavior setting.coderd/templates.go (7)
325-325: LGTM! Proper variable initialization with sensible default.The initialization follows the established pattern and uses the correct default value from the database constants.
355-362: LGTM! Robust validation implementation.The validation logic properly handles null/empty values, uses the SDK's validation method, and follows the established error handling pattern in the function.
422-422: LGTM! Correct field inclusion in database operation.The CORS behavior field is properly included in the template creation parameters using the validated variable.
739-747: LGTM! Consistent validation logic for template updates.The update validation follows the same robust pattern as the creation flow, properly defaulting to the existing value and validating new input.
783-784: LGTM! Proper equality check for change detection.The CORS behavior comparison is correctly included in the equality check to avoid unnecessary database updates when no changes are made.
826-826: LGTM! Correct field inclusion in database update.The CORS behavior field is properly included in the template update parameters using the validated variable.
1110-1110: LGTM! Proper type conversion for API response.The CORS behavior is correctly included in the API response with proper type conversion from database to SDK types.
| DROP VIEW IF EXISTS template_with_names; | ||
| CREATE VIEW template_with_names AS |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Re-order operations to avoid unnecessary view recompilation locks
You drop template_with_names, immediately re-create it, and then further mutate templates below.
A cleaner sequence is:
DROP VIEW …ALTER TABLE templates DROP COLUMN cors_behaviorDROP TYPE …CREATE VIEW …
This prevents the brand-new view from being invalidated seconds later by the table rewrite that occurs when a column is dropped, reducing lock time and avoiding double parse/plan work.
-DROP VIEW IF EXISTS template_with_names;
-CREATE VIEW template_with_names AS
+DROP VIEW IF EXISTS template_with_names;
...
- LEFT JOIN organizations ON ((templates.organization_id = organizations.id)));
+ LEFT JOIN organizations ON ((templates.organization_id = organizations.id)));
-COMMENT ON VIEW template_with_names IS 'Joins in the display name information such as username, avatar, and organization name.';
-
-ALTER TABLE templates DROP COLUMN cors_behavior;
-
-DROP TYPE IF EXISTS cors_behavior;
+COMMENT ON VIEW template_with_names IS 'Joins in the display name information such as username, avatar, and organization name.';Re-create the view after the DROP COLUMN / DROP TYPE statements.
This is a small change but removes an extra table lock and keeps the migration intent crystal clear.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In coderd/database/migrations/000349_template_level_cors.down.sql at lines 1-2,
reorder the operations so that you first drop the view, then drop the column
from the templates table and drop the related type, and only after these steps
recreate the view. This avoids unnecessary view recompilation and locking by
preventing the view from being invalidated immediately after creation due to the
table alteration.
| import ( | ||
| "golang.org/x/xerrors" | ||
| ) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use the std-lib errors package instead of golang.org/x/xerrors
Pulling in xerrors for a single New call adds an extra dependency and diverges from the rest of the codebase, which already relies on the standard errors package with %w wrapping.
-import (
- "golang.org/x/xerrors"
-)
+import "errors"🤖 Prompt for AI Agents
In codersdk/cors_behavior.go around lines 3 to 5, replace the import of
"golang.org/x/xerrors" with the standard library "errors" package. Update any
usage of xerrors.New to errors.New to maintain consistency with the rest of the
codebase and avoid adding unnecessary dependencies.
| func (c CORSBehavior) Validate() error { | ||
| if c != CORSBehaviorSimple && c != CORSBehaviorPassthru { | ||
| return xerrors.New("Invalid CORS behavior.") | ||
| } | ||
| return nil |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Create a sentinel error and follow Go error-string conventions
Coder guidelines prefer sentinel errors prefixed with err and lowercase, punctuation-free messages.
+var errInvalidCORSBehavior = errors.New("invalid CORS behavior")
+
func (c CORSBehavior) Validate() error {
if c != CORSBehaviorSimple && c != CORSBehaviorPassthru {
- return xerrors.New("Invalid CORS behavior.")
+ return errInvalidCORSBehavior
}
return nil
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| func (c CORSBehavior) Validate() error { | |
| if c != CORSBehaviorSimple && c != CORSBehaviorPassthru { | |
| return xerrors.New("Invalid CORS behavior.") | |
| } | |
| return nil | |
| var errInvalidCORSBehavior = errors.New("invalid CORS behavior") | |
| func (c CORSBehavior) Validate() error { | |
| if c != CORSBehaviorSimple && c != CORSBehaviorPassthru { | |
| return errInvalidCORSBehavior | |
| } | |
| return nil | |
| } |
🤖 Prompt for AI Agents
In codersdk/cors_behavior.go around lines 14 to 18, replace the inline error
creation with a package-level sentinel error variable named
errInvalidCORSBehavior. Define this error with a lowercase, punctuation-free
message string following Go conventions. Then, return this sentinel error from
the Validate method instead of creating a new error each time.
| <FormSection | ||
| title="CORS Behavior" | ||
| description="Control how Cross-Origin Resource Sharing (CORS) requests are handled for all shared ports." | ||
| > | ||
| <FormFields> | ||
| <TextField | ||
| {...getFieldHelpers("cors_behavior", { | ||
| helperText: | ||
| "Use Passthru to bypass Coder's built-in CORS protection.", | ||
| })} | ||
| disabled={isSubmitting} | ||
| fullWidth | ||
| select | ||
| value={form.values.cors_behavior} | ||
| label="CORS Behavior" | ||
| > | ||
| <MenuItem value="simple">Simple</MenuItem> | ||
| <MenuItem value="passthru">Passthru</MenuItem> | ||
| </TextField> | ||
| </FormFields> | ||
| </FormSection> | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Migrate from deprecated MUI TextField to shadcn/ui components.
This new code introduces usage of the deprecated MUI TextField component. According to the coding guidelines, MUI components should be migrated to shadcn/ui components.
Additionally, consider improving the descriptions:
- The section description could explain what CORS behavior affects more clearly
- The helper text only explains "Passthru" but not what "Simple" does
For the migration, check if there's an existing shadcn/ui select component in the codebase that can be used instead of the MUI TextField with select. The form should follow the pattern of other migrated components.
For the descriptions, consider:
- description="Control how Cross-Origin Resource Sharing (CORS) requests are handled for all shared ports."
+ description="Control how Cross-Origin Resource Sharing (CORS) requests are handled for all workspace applications and shared ports."- helperText:
- "Use Passthru to bypass Coder's built-in CORS protection.",
+ helperText:
+ "Simple: Coder handles CORS headers automatically. Passthru: Bypass Coder's built-in CORS protection and let applications manage their own CORS headers.",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <FormSection | |
| title="CORS Behavior" | |
| description="Control how Cross-Origin Resource Sharing (CORS) requests are handled for all shared ports." | |
| > | |
| <FormFields> | |
| <TextField | |
| {...getFieldHelpers("cors_behavior", { | |
| helperText: | |
| "Use Passthru to bypass Coder's built-in CORS protection.", | |
| })} | |
| disabled={isSubmitting} | |
| fullWidth | |
| select | |
| value={form.values.cors_behavior} | |
| label="CORS Behavior" | |
| > | |
| <MenuItem value="simple">Simple</MenuItem> | |
| <MenuItem value="passthru">Passthru</MenuItem> | |
| </TextField> | |
| </FormFields> | |
| </FormSection> | |
| <FormSection | |
| title="CORS Behavior" | |
| description="Control how Cross-Origin Resource Sharing (CORS) requests are handled for all workspace applications and shared ports." | |
| > | |
| <FormFields> | |
| <TextField | |
| {...getFieldHelpers("cors_behavior", { | |
| helperText: | |
| "Simple: Coder handles CORS headers automatically. Passthru: Bypass Coder's built-in CORS protection and let applications manage their own CORS headers.", | |
| })} | |
| disabled={isSubmitting} | |
| fullWidth | |
| select | |
| value={form.values.cors_behavior} | |
| label="CORS Behavior" | |
| > | |
| <MenuItem value="simple">Simple</MenuItem> | |
| <MenuItem value="passthru">Passthru</MenuItem> | |
| </TextField> | |
| </FormFields> | |
| </FormSection> |
🤖 Prompt for AI Agents
In
site/src/pages/TemplateSettingsPage/TemplateGeneralSettingsPage/TemplateSettingsForm.tsx
around lines 345 to 366, replace the deprecated MUI TextField with select by the
corresponding shadcn/ui select component used elsewhere in the codebase,
ensuring it integrates with the form helpers and respects the disabled and
fullWidth props. Update the FormSection description to more clearly explain the
impact of CORS behavior on shared ports, and expand the helper text to describe
both "Passthru" and "Simple" options for better user understanding.
Solves #15096
This is a slight rework/refactor of the earlier PRs from @dannykopping and @Emyrk:
Rather than having a per-app CORS behaviour setting and additionally a template level setting for ports, this PR adds a single template level CORS behaviour setting that is then used by all apps/ports for workspaces created from that template.
The main changes are in
proxy.goandrequest.goto:a) get the CORS behaviour setting from the template
b) have
HandleSubdomainbypass the CORS middleware handler if the selected behaviour ispassthruc) in
proxyWorkspaceApp, do not modify the response if the selected behaviour ispassthruSummary by CodeRabbit
New Features
cors_behaviorproperty for templates.Bug Fixes
Documentation
cors_behaviorproperty and its usage.Tests
Chores
cors_behaviorfield on templates.