Raised by the independent review of #174. Deliberately NOT milestoned 1.0.0 — it cannot fire today.
internal/database/model_target.go:37 gives Target a Webhook back-reference, and Webhook has Targets. That is a cycle. encoding/json does not detect cycles; it recurses until the stack dies.
It is harmless right now for two reasons that both happen to hold rather than being designed to: omitzero hides the back-reference while it stays zero, and nothing in the service marshals these models at all — /api/v1 is registered and empty. The first handler that marshals a preloadedWebhook or Target will recurse.
The reviewer described it as deliberate-looking, and it may well be — GORM needs the association for preloading. This issue is not asking to remove it, only to make the safety explicit rather than incidental.
Definition of done
Marshalling a preloaded Webhook (with Targets populated) and a preloaded Target (with Webhook populated) is either safe or impossible, and a test pins whichever it is.
If the answer is json:"-" on the back-reference, confirm GORM's preloading is unaffected — GORM does not use json tags for association mapping, but verify rather than assume, and check nothing round-trips these models through JSON in tests or fixtures.
Check the other associations in internal/database/ for the same shape while you are there, and list what you found.
Related context: #160 added json:"-" to Target.Config, APIKey.Key and Setting.Value for the adjacent reason — that TargetView is the masking barrier for the HTML path only, and the first handler to marshal a model directly would leak. This is the same "the models are not safe to marshal yet, and nothing marshals them" territory, so it may be worth doing both audits at once when someone picks up /api/v1.
Implementation requirements
Branch from next, PR based on next, single commit, title ending (closes #N).
Run make bootstrap in a fresh clone before gating — browser assets are fetched at build time, and make lint now needs Docker.
Gate on make check plus the Docker lint path with the cache defeated. All linting runs in Docker, never on the host.
Raised by the independent review of https://git.eeqj.de/sneak/webhooker/pulls/174. Deliberately NOT milestoned 1.0.0 — it cannot fire today.
`internal/database/model_target.go:37` gives `Target` a `Webhook` back-reference, and `Webhook` has `Targets`. That is a cycle. `encoding/json` does not detect cycles; it recurses until the stack dies.
It is harmless right now for two reasons that both happen to hold rather than being designed to: `omitzero` hides the back-reference while it stays zero, and nothing in the service marshals these models at all — `/api/v1` is registered and empty. The first handler that marshals a **preloaded** `Webhook` or `Target` will recurse.
The reviewer described it as deliberate-looking, and it may well be — GORM needs the association for preloading. This issue is not asking to remove it, only to make the safety explicit rather than incidental.
## Definition of done
- Marshalling a preloaded `Webhook` (with `Targets` populated) and a preloaded `Target` (with `Webhook` populated) is either safe or impossible, and a test pins whichever it is.
- If the answer is `json:"-"` on the back-reference, confirm GORM's preloading is unaffected — GORM does not use json tags for association mapping, but verify rather than assume, and check nothing round-trips these models through JSON in tests or fixtures.
- Check the other associations in `internal/database/` for the same shape while you are there, and list what you found.
Related context: https://git.eeqj.de/sneak/webhooker/issues/160 added `json:"-"` to `Target.Config`, `APIKey.Key` and `Setting.Value` for the adjacent reason — that `TargetView` is the masking barrier for the HTML path only, and the first handler to marshal a model directly would leak. This is the same "the models are not safe to marshal yet, and nothing marshals them" territory, so it may be worth doing both audits at once when someone picks up `/api/v1`.
## Implementation requirements
- Branch from `next`, PR based on `next`, single commit, title ending ` (closes #N)`.
- Do not modify `TODO.md` (see https://git.eeqj.de/sneak/webhooker/issues/112).
- Run `make bootstrap` in a fresh clone before gating — browser assets are fetched at build time, and `make lint` now needs Docker.
- Gate on `make check` plus the Docker lint path with the cache defeated. All linting runs in Docker, never on the host.
clawbot
self-assigned this 2026-08-18 01:10:13 +02:00
Plan. Put json:\"-\" on each child-to-parent reference in internal/database (the target's and the entrypoint's reference to their webhook, and any other of the same shape found there), so a preloaded model marshals without recursing. A test marshals a preloaded webhook with its targets and entrypoints, and a preloaded target with its webhook, and shows both succeed and carry no back-reference. An existing or added test shows preloading still fills these associations. The PR lists every association checked and what was done with it. Ignore the issue's old notes on make bootstrap.
Model: opus-5-5
Plan. Put `json:\"-\"` on each child-to-parent reference in `internal/database` (the target's and the entrypoint's reference to their webhook, and any other of the same shape found there), so a preloaded model marshals without recursing. A test marshals a preloaded webhook with its targets and entrypoints, and a preloaded target with its webhook, and shows both succeed and carry no back-reference. An existing or added test shows preloading still fills these associations. The PR lists every association checked and what was done with it. Ignore the issue's old notes on `make bootstrap`.
Model: opus-5-5
Done in #433. Every reference from a model to the record it belongs to (a target's or entrypoint's webhook, a webhook's or API key's user, an event's webhook and entrypoint, a delivery's event and target, a delivery result's delivery) is now left out of the JSON, so no two models can marshal each other in a loop. The references to child records stay. A new test preloads a webhook with its targets and entrypoints and a target with its webhook, checks preloading still fills each reference, and checks the JSON leaves it out. The PR lists every association checked.
Judgement call: an event's webhook and entrypoint close no loop on their own; they are left out too so the rule holds for every reference to a parent.
Model: opus-5-5
Done in https://git.eeqj.de/sneak/webhooker/pulls/433. Every reference from a model to the record it belongs to (a target's or entrypoint's webhook, a webhook's or API key's user, an event's webhook and entrypoint, a delivery's event and target, a delivery result's delivery) is now left out of the JSON, so no two models can marshal each other in a loop. The references to child records stay. A new test preloads a webhook with its targets and entrypoints and a target with its webhook, checks preloading still fills each reference, and checks the JSON leaves it out. The PR lists every association checked.
Judgement call: an event's webhook and entrypoint close no loop on their own; they are left out too so the rule holds for every reference to a parent.
Model: opus-5-5
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Raised by the independent review of #174. Deliberately NOT milestoned 1.0.0 — it cannot fire today.
internal/database/model_target.go:37givesTargetaWebhookback-reference, andWebhookhasTargets. That is a cycle.encoding/jsondoes not detect cycles; it recurses until the stack dies.It is harmless right now for two reasons that both happen to hold rather than being designed to:
omitzerohides the back-reference while it stays zero, and nothing in the service marshals these models at all —/api/v1is registered and empty. The first handler that marshals a preloadedWebhookorTargetwill recurse.The reviewer described it as deliberate-looking, and it may well be — GORM needs the association for preloading. This issue is not asking to remove it, only to make the safety explicit rather than incidental.
Definition of done
Webhook(withTargetspopulated) and a preloadedTarget(withWebhookpopulated) is either safe or impossible, and a test pins whichever it is.json:"-"on the back-reference, confirm GORM's preloading is unaffected — GORM does not use json tags for association mapping, but verify rather than assume, and check nothing round-trips these models through JSON in tests or fixtures.internal/database/for the same shape while you are there, and list what you found.Related context: #160 added
json:"-"toTarget.Config,APIKey.KeyandSetting.Valuefor the adjacent reason — thatTargetViewis the masking barrier for the HTML path only, and the first handler to marshal a model directly would leak. This is the same "the models are not safe to marshal yet, and nothing marshals them" territory, so it may be worth doing both audits at once when someone picks up/api/v1.Implementation requirements
next, PR based onnext, single commit, title ending(closes #N).TODO.md(see #112).make bootstrapin a fresh clone before gating — browser assets are fetched at build time, andmake lintnow needs Docker.make checkplus the Docker lint path with the cache defeated. All linting runs in Docker, never on the host.Plan. Put
json:\"-\"on each child-to-parent reference ininternal/database(the target's and the entrypoint's reference to their webhook, and any other of the same shape found there), so a preloaded model marshals without recursing. A test marshals a preloaded webhook with its targets and entrypoints, and a preloaded target with its webhook, and shows both succeed and carry no back-reference. An existing or added test shows preloading still fills these associations. The PR lists every association checked and what was done with it. Ignore the issue's old notes onmake bootstrap.Model: opus-5-5
Done in #433. Every reference from a model to the record it belongs to (a target's or entrypoint's webhook, a webhook's or API key's user, an event's webhook and entrypoint, a delivery's event and target, a delivery result's delivery) is now left out of the JSON, so no two models can marshal each other in a loop. The references to child records stay. A new test preloads a webhook with its targets and entrypoints and a target with its webhook, checks preloading still fills each reference, and checks the JSON leaves it out. The PR lists every association checked.
Judgement call: an event's webhook and entrypoint close no loop on their own; they are left out too so the rule holds for every reference to a parent.
Model: opus-5-5