Skip to content

fix: force secure response cookies#142

Closed
cssbruno wants to merge 2 commits into
mainfrom
fix/secure-response-cookies
Closed

fix: force secure response cookies#142
cssbruno wants to merge 2 commits into
mainfrom
fix/secure-response-cookies

Conversation

@cssbruno

@cssbruno cssbruno commented Jun 9, 2026

Copy link
Copy Markdown
Owner

Summary

  • Force Secure=true on cookies written through runtime/response.WriteHTTP.
  • Preserve existing cookie attributes and avoid mutating the response input cookie slice.
  • Add regression coverage for the CodeQL cookie Secure finding.

This addresses the CodeQL alert for Cookie Secure attribute is not set to true at runtime/response.WriteHTTP.

Verification

  • I ran the relevant tests, lint, and build commands.
  • I ran scripts/test-go-modules.sh when Go code or compiler behavior changed.
  • I ran go build ./cmd/gowdk when CLI, compiler, runtime, addon, or release behavior changed.
  • I ran node --check editors/vscode/extension.js when editor files changed.
  • I updated docs for behavior, setup, or architecture changes.
  • I added or updated tests for changed behavior.

Additional validation:

  • go test ./runtime/response
  • git diff --check
  • go test ./...

LLM Assistance

  • LLM session summary: Codex fixed the CodeQL response cookie Secure finding with a scoped runtime writer change and regression test.
  • Human-reviewed assumptions: Pending maintainer review before merge.
  • Follow-up work: Confirm the code scanning alert closes after GitHub reruns CodeQL.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 8784aa4c20

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread runtime/response/response.go Outdated
func WriteHTTP(writer http.ResponseWriter, result Response) error {
status := statusOrDefault(result)
for _, cookie := range result.Cookies {
cookie.Secure = true

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve callers' HTTP-development cookie policy

This unconditional overwrite breaks callers that intentionally set Secure: false for local HTTP flows. For example, the login example defaults GOWDK_COOKIE_SECURE to false in examples/login/src/features/auth/auth.go and the documented make serve flow opens http://127.0.0.1:8090/; with this line, response.WithCookie emits a Secure session cookie anyway, so browsers/cookie jars will not send it back over the documented HTTP URL and the login redirect lands without an authenticated session. Please keep the caller's cookie.Secure value or make the secure policy configurable where WriteHTTP is used.

Useful? React with 👍 / 👎.

Set cookies to be secure when writing HTTP responses.
@cssbruno cssbruno closed this Jun 9, 2026
@cssbruno cssbruno deleted the fix/secure-response-cookies branch June 9, 2026 18:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant