Skip to content

issue: small robustness fixes (health resp body leak, SNS err ordering, migration INSERT escaping, dynamodb swallowed err) #203

@Lutherwaves

Description

@Lutherwaves

Check Existing Issues

  • I have searched the existing issues and discussions.

Expected Behavior

A handful of small, independent correctness/robustness fixes across health, pubsub, and storage. Each is a few lines; grouped here as a checklist so they can land in one (or a few) PRs rather than spawning four micro-issues.

Actual Behavior

1. Health dependency check leaks the HTTP response body (health/health.go:28-34).

resp, err := http.Get(d)
if err != nil { ... }
if resp.StatusCode > 399 { ... }

resp.Body is never closed, leaking a connection/FD on every dependency probe. Health checks run repeatedly, so this accumulates. Needs defer resp.Body.Close() (guarded for the error case).

2. SNS publisher checks the config error after already using the config (pubsub/sns.go:23-40).

cfg, err := awsconfig.LoadDefaultConfig(context.TODO())
// ... cfg.Region / cfg.Credentials mutated and used ...
if err != nil { logger.Fatal(...) }   // checked too late

The err from LoadDefaultConfig should be checked before cfg is touched.

3. Migration INSERT uses unescaped string interpolation (storage/sql.go:168-171).

statement = fmt.Sprintf(`INSERT INTO migrations VALUES(%v, '%v', '%v', %v)`, id, name, desc, time.Now().UnixMilli())

Values aren't attacker-controlled (they come from migration filenames/descriptions), but a name or description containing an apostrophe breaks the INSERT. Should be parameterized.

4. DynamoDB List swallows the buildParams error (storage/dynamodb.go:258).

params, _ := s.buildParams(filter)

On a marshal failure the query still runs with nil/empty params, silently producing wrong results. The error should propagate.

Steps to Reproduce

  1. (feat: Package instantiation and multi-key filtering #1) Run a readiness probe with an HTTP dependency repeatedly; observe FD/connection growth.
  2. (fix(pgsql): Fix missing schema in migrations and leadership table operations #3) Run a migration whose description contains ' (e.g. add user's table); observe the INSERT fails.
  3. (Initial support for PubSub and DynamoDB updates #2/cicd: Remove requirement of release commit to release #4) Code-path review — error is checked after use / discarded.

Logs & Screenshots

n/a — inspection-level findings with exact file:line above.

Additional Information

Found during a general audit of the library. All four are low-risk, low-LOC fixes; bundling for a single robustness PR. Happy to split if maintainers prefer separate tracking.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workinggoPull requests that update go code

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions