Skip to content

Use json.dumps for redis.publish payloads in worker.py #111

@JohnRDOrazio

Description

@JohnRDOrazio

Problem

ontokit/worker.py builds Redis pubsub payloads via f-string-formatted JSON, e.g.:

await redis.publish(
    LINT_UPDATES_CHANNEL,
    f'{{"type": "lint_failed", "project_id": "{project_id}", '
    f'"run_id": "{run.id}", "error": "{str(e)}"}}',
)

This is fragile and, in at least one case, incorrect:

  • The lint_failed / normalization_failed / remote_check_failed handlers interpolate str(e) directly. Exception messages routinely contain ", \, or newlines (e.g. 'foo' has no attribute "bar"), which produces invalid JSON and breaks any subscriber that does json.loads(payload).
  • Other handlers only interpolate UUIDs and ints today, so they're safe by accident but fragile to future changes.

Affected lines

Confirmed instances in ontokit/worker.py (line numbers as of 37078ef):

  • L253 — lint_started
  • L322–323 — lint_complete
  • L345–346 — lint_failedbuggy: str(e) may contain unescaped quotes/newlines
  • L392 — normalization_status
  • L452 — normalization_started
  • L494 — normalization_complete
  • L513 — normalization_failedalso interpolates str(e)
  • L553 — normalization_status (per-project loop)
  • L1018 — remote_check_started
  • L1108 — remote_check_complete
  • L1145 — remote_check_failedalso interpolates str(e)

Proposed fix

Replace the f-string payloads with json.dumps({...}). json is already imported at the top of the file. Example:

await redis.publish(
    LINT_UPDATES_CHANNEL,
    json.dumps({
        "type": "lint_failed",
        "project_id": project_id,
        "run_id": str(run.id),
        "error": str(e),
    }),
)

This guarantees correct escaping for arbitrary error strings and removes the maintenance footgun for any future field that might contain user-controlled data.

Out of scope

  • Changing the channel names or message schemas
  • Adding new fields
  • Touching subscribers (existing JSON parsers will continue to work)

Origin

Surfaced as L4 in the peer review of #94. Deferred from #103 because every instance predates that PR; opening here so it doesn't get lost.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions