Skip to content

[codex] fix runtime hardening gaps#599

Open
cssbruno wants to merge 1 commit into
mainfrom
fix/validated-runtime-hardening
Open

[codex] fix runtime hardening gaps#599
cssbruno wants to merge 1 commit into
mainfrom
fix/validated-runtime-hardening

Conversation

@cssbruno

Copy link
Copy Markdown
Owner

Summary

  • Enforce default-deny for guardless fragments and command/query contract adapters while preserving JSON errors for contract endpoints.
  • Harden request parsing, env defaults/loading, rate-limit handler delegation, guard error responses, panic boundaries, and response-writer optional HTTP capabilities.
  • Add runtime safeguards for file outbox record limits/dead-letter dedupe/redaction, contract registry nil/zero-value use, tracing sink isolation, worker error redaction, and PBKDF2 iteration bounds.

Why

The runtime/generated-output audit found several gaps where stale or malformed generated state could bypass intended defaults, leak internal details, or wedge runtime workers/exporters. This change makes those paths fail closed and adds focused regression coverage.

Validation

  • go test ./internal/appgen ./runtime/app ./runtime/guard ./runtime/response ./runtime/ratelimit ./runtime/trace ./runtime/contracts ./runtime/contracts/fileoutbox ./addons/auth
  • go test ./...
  • scripts/test-go-modules.sh
  • go build ./cmd/gowdk

@cssbruno cssbruno marked this pull request as ready for review June 20, 2026 23:41

@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: b31d06e866

ℹ️ 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 on lines +354 to +356
if endpointDeniedByOmission(exposure.Guards) {
continue
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Keep form imports for denied contract decoders

When the only form usage is a guardless bound command/query contract with input fields, this continue suppresses the gowdkform import, but contractDecoderDecls still emits decodeContract... functions for executable exposures and those declarations reference gowdkform.DecodeExpected. The generated app then fails to compile even though the handler now returns 403; either skip those decoder declarations too or keep the import whenever a decoder is emitted.

Useful? React with 👍 / 👎.

Comment on lines +41 to +42
if endpointDeniedByOmission(exposure.Guards) {
return denyByOmissionJSONStmts()

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 Deny guardless fallback contract handlers too

This default-deny check only runs for executable contract exposures. A guardless routable contract whose binding falls back to fallbackContractHandlerDecl (for example, missing backend registration or unsupported signature) still returns the fallback JSON instead of the new 403, leaving stale or partially generated contract endpoints reachable by omission while actions/APIs/fragments fail closed. Apply the same guard omission check before emitting the fallback handler.

Useful? React with 👍 / 👎.

Comment thread runtime/trace/span.go
Comment on lines +91 to +95
go func() {
ctx, cancel := context.WithTimeout(context.Background(), defaultSinkTimeout)
defer cancel()
if err := sink.RecordSpan(ctx, snapshot); err != nil {
logSinkFailure(err)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Recover panicking trace sinks in the async exporter

Now that span export runs in its own goroutine, any panic from a custom Sink.RecordSpan or ExporterSink implementation is no longer contained by the request handler or net/http recovery; an unrecovered panic in this goroutine terminates the entire generated app. Wrap the sink call with a defer recover path before logging/reporting the sink failure.

Useful? React with 👍 / 👎.

Comment thread runtime/trace/span.go
if err == nil || SinkLogger == nil {
return
}
SinkLogger("gowdk trace: sink failed: " + err.Error())

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 Redact trace sink errors before logging

When an exporter fails with an error that includes connection strings, tokens, or authorization headers, the new default SinkLogger writes err.Error() verbatim. That creates a new credential-leak path specifically on tracing backend failures, while the other hardened runtime logs redact secrets first; redact the error string before passing it to SinkLogger.

Useful? React with 👍 / 👎.

Comment on lines +65 to +66
if endpointDeniedByOmission(fragment.Guards) {
return denyByOmissionStmts()

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Avoid stale imports for denied fragments

For a guardless fragment that would otherwise use the static fallback, this new early return removes the only generated gowdkpartial.Fragment reference, but fragmentsUseStaticFallback still causes gowdkpartial to be imported. The same pattern can leave a bound fragment's backend import unused, so generated apps with only such denied fragments fail to compile; skip denied fragments in the import predicates or avoid emitting their unused dependencies.

Useful? React with 👍 / 👎.

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