Describe code in its own terms, not by an earlier build's data (closes #522)
check / check (push) Waiting to run
check / check (push) Waiting to run
Several comments justified code by what an earlier build would have left in a database or on disk. Each now says what the code does: reserveSQLiteFile sets the mode on every open whoever created the file, and expired treats a session lacking a deadline's timestamp as expired. The comments on retention, resubmitted events, archive pruning, target headers, the redactor and the delivery result view drop their mentions of older rows. parseMaxRetries loses its paragraph on targets saved before the ceiling existed, and neither BeforeSave nor retentionCutoff cites a migration. Comments only. Model: opus-5-5
This commit is contained in:
@@ -45,8 +45,7 @@ type Event struct {
|
|||||||
|
|
||||||
// ResubmittedFromID names the event this one was copied from by
|
// ResubmittedFromID names the event this one was copied from by
|
||||||
// an operator resubmit. It is nil for an event that arrived on
|
// an operator resubmit. It is nil for an event that arrived on
|
||||||
// the receiver, which is every event created before the column
|
// the receiver. It is not a foreign key: the source event can be
|
||||||
// existed. It is not a foreign key: the source event can be
|
|
||||||
// reaped by retention while its copies remain, and the id is
|
// reaped by retention while its copies remain, and the id is
|
||||||
// kept as the record of where the copy came from either way.
|
// kept as the record of where the copy came from either way.
|
||||||
ResubmittedFromID *string `gorm:"type:uuid;index:idx_events_resubmitted_from_id,priority:1;index:idx_events_entrypoint_id,priority:3" json:"resubmittedFromId,omitempty"`
|
ResubmittedFromID *string `gorm:"type:uuid;index:idx_events_resubmitted_from_id,priority:1;index:idx_events_entrypoint_id,priority:3" json:"resubmittedFromId,omitempty"`
|
||||||
|
|||||||
@@ -82,7 +82,7 @@ type Webhook struct {
|
|||||||
// value while building the insert statement, which runs after
|
// value while building the insert statement, which runs after
|
||||||
// BeforeSave; rewriting any later than this loses that race and the
|
// BeforeSave; rewriting any later than this loses that race and the
|
||||||
// row lands at 30 days. Living on the model also means a future call
|
// row lands at 30 days. Living on the model also means a future call
|
||||||
// site — a REST API, a fixture, a migration — cannot bypass it.
|
// site, such as a REST API or a fixture, cannot bypass it.
|
||||||
func (w *Webhook) BeforeSave(_ *gorm.DB) error {
|
func (w *Webhook) BeforeSave(_ *gorm.DB) error {
|
||||||
if w.RetentionDays <= 0 {
|
if w.RetentionDays <= 0 {
|
||||||
w.RetentionDays = RetentionForeverDays
|
w.RetentionDays = RetentionForeverDays
|
||||||
@@ -98,8 +98,8 @@ func (w *Webhook) BeforeSave(_ *gorm.DB) error {
|
|||||||
// are exempt from reaping.
|
// are exempt from reaping.
|
||||||
//
|
//
|
||||||
// It accepts the RetentionForeverDays sentinel written by BeforeSave
|
// It accepts the RetentionForeverDays sentinel written by BeforeSave
|
||||||
// and, defensively, the non-positive values that rows written before
|
// and, defensively, any non-positive value, which BeforeSave treats
|
||||||
// the sentinel existed may still carry.
|
// as a request to retain forever.
|
||||||
func retainsForever(retentionDays int) bool {
|
func retainsForever(retentionDays int) bool {
|
||||||
return retentionDays <= 0 ||
|
return retentionDays <= 0 ||
|
||||||
retentionDays >= RetentionForeverDays
|
retentionDays >= RetentionForeverDays
|
||||||
|
|||||||
@@ -253,7 +253,7 @@ func (r *RetentionReaper) reapWebhook(
|
|||||||
// row in the database: the sweep then deletes every event, delivery,
|
// row in the database: the sweep then deletes every event, delivery,
|
||||||
// and delivery result, including ones created seconds ago. Rejecting
|
// and delivery result, including ones created seconds ago. Rejecting
|
||||||
// out-of-range input at the form is the primary guard; saturating here
|
// out-of-range input at the form is the primary guard; saturating here
|
||||||
// means an old row, a migration, or a future call site cannot turn a
|
// means a value that reaches the row by any other path cannot turn a
|
||||||
// too-large retention into total data loss.
|
// too-large retention into total data loss.
|
||||||
func retentionCutoff(
|
func retentionCutoff(
|
||||||
now time.Time,
|
now time.Time,
|
||||||
|
|||||||
@@ -118,9 +118,8 @@ const SQLiteFilePerm fs.FileMode = 0o600
|
|||||||
// SQLite still reports the absence rather than this function
|
// SQLite still reports the absence rather than this function
|
||||||
// materializing an empty database the caller asked not to create.
|
// materializing an empty database the caller asked not to create.
|
||||||
//
|
//
|
||||||
// Chmod of a file that already exists is what tightens a data
|
// The mode is set on every open, whoever created the file, so a
|
||||||
// directory an earlier build left at 0644 — including a developer's
|
// database or sidecar found at a wider mode is tightened too.
|
||||||
// own scratch directory — without any migration machinery.
|
|
||||||
func reserveSQLiteFile(path string, create bool) error {
|
func reserveSQLiteFile(path string, create bool) error {
|
||||||
if create {
|
if create {
|
||||||
// gosec G304: the path is the database file the caller asked
|
// gosec G304: the path is the database file the caller asked
|
||||||
|
|||||||
@@ -201,8 +201,7 @@ func (s *ArchiveSweeper) sweep(ctx context.Context) {
|
|||||||
|
|
||||||
// sweepTarget prunes the archive of a single database target.
|
// sweepTarget prunes the archive of a single database target.
|
||||||
// A missing, empty, or "never" expiry parses as a zero duration
|
// A missing, empty, or "never" expiry parses as a zero duration
|
||||||
// and is skipped entirely, so those archives keep exactly the
|
// and is skipped entirely, so those archives are never pruned.
|
||||||
// behaviour they had before the sweep existed.
|
|
||||||
func (s *ArchiveSweeper) sweepTarget(target *database.Target) {
|
func (s *ArchiveSweeper) sweepTarget(target *database.Target) {
|
||||||
expiry, err := parseArchiveExpiry(target.Config)
|
expiry, err := parseArchiveExpiry(target.Config)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
|
|||||||
@@ -82,8 +82,8 @@ func isReservedTargetHeader(name string) bool {
|
|||||||
// one pair silently overwriting the other.
|
// one pair silently overwriting the other.
|
||||||
//
|
//
|
||||||
// An input with no pairs yields an empty map, which omitempty drops
|
// An input with no pairs yields an empty map, which omitempty drops
|
||||||
// from the stored config: a target configured with no headers keeps
|
// from the stored config, so a target configured with no headers
|
||||||
// the same config JSON it had before this field existed.
|
// stores no headers key at all.
|
||||||
func ParseTargetHeaders(raw string) (map[string]string, error) {
|
func ParseTargetHeaders(raw string) (map[string]string, error) {
|
||||||
headers := make(map[string]string)
|
headers := make(map[string]string)
|
||||||
|
|
||||||
|
|||||||
@@ -13,9 +13,9 @@ import (
|
|||||||
const RedactionMarker = "(redacted)"
|
const RedactionMarker = "(redacted)"
|
||||||
|
|
||||||
// Redactor removes one target's own credential material from
|
// Redactor removes one target's own credential material from
|
||||||
// text that target's remote peer chose: a delivery response
|
// a delivery response body, whose text the target's remote
|
||||||
// body, or a delivery error stored before the delivery path
|
// peer chose, and from a stored delivery error as a second
|
||||||
// learned to mask the URLs it embeds.
|
// line behind the delivery path's own URL masking.
|
||||||
//
|
//
|
||||||
// It removes byte-identical echoes of strings taken from the
|
// It removes byte-identical echoes of strings taken from the
|
||||||
// target's stored configuration, and nothing else. Anything
|
// target's stored configuration, and nothing else. Anything
|
||||||
|
|||||||
@@ -12,13 +12,12 @@ import (
|
|||||||
//
|
//
|
||||||
// The bound is enforced here and in SQL, because this page's
|
// The bound is enforced here and in SQL, because this page's
|
||||||
// memory profile must not depend on a constant in another
|
// memory profile must not depend on a constant in another
|
||||||
// package staying where it is, and because rows predating the
|
// package staying where it is, or on every stored row having
|
||||||
// delivery engine's own cap or restored from an archive are
|
// been written by the delivery engine under its own cap.
|
||||||
// not covered by it at all.
|
|
||||||
//
|
//
|
||||||
// It happens to equal that engine cap, so a row written by the
|
// It happens to equal that engine cap, so a row written by the
|
||||||
// current engine reaches this one exactly and is never cut
|
// engine reaches this one exactly and is never cut twice.
|
||||||
// twice. Nothing here may assume the two differ: see view.
|
// Nothing here may assume the two differ: see view.
|
||||||
const maxRenderedResponseBytes = 4096
|
const maxRenderedResponseBytes = 4096
|
||||||
|
|
||||||
// deliveryResultColumns is the delivery attempt projection.
|
// deliveryResultColumns is the delivery attempt projection.
|
||||||
@@ -40,9 +39,8 @@ const deliveryResultColumns = "delivery_id, attempt_num, created_at, " +
|
|||||||
//
|
//
|
||||||
// Both Error and ResponseBody have been through the target's
|
// Both Error and ResponseBody have been through the target's
|
||||||
// Redactor. The engine already masks the URL out of the
|
// Redactor. The engine already masks the URL out of the
|
||||||
// errors it stores, so for errors this is a second line
|
// errors it stores, so for errors this is a second line; for
|
||||||
// covering rows written before it did; for response bodies it
|
// response bodies it is the only line, and its reach is what
|
||||||
// is the only line, and its reach is what
|
|
||||||
// delivery.Redactor documents.
|
// delivery.Redactor documents.
|
||||||
type DeliveryResultView struct {
|
type DeliveryResultView struct {
|
||||||
AttemptNum int
|
AttemptNum int
|
||||||
|
|||||||
@@ -49,13 +49,6 @@ var (
|
|||||||
// working retry configuration over a typo. A default answers a
|
// working retry configuration over a typo. A default answers a
|
||||||
// question that was not asked; it never answers one that was asked
|
// question that was not asked; it never answers one that was asked
|
||||||
// badly.
|
// badly.
|
||||||
//
|
|
||||||
// A target stored with a count above the ceiling before this
|
|
||||||
// validation existed keeps rendering and keeps delivering — nothing
|
|
||||||
// clamps the row. Re-saving it from the edit form does have to bring
|
|
||||||
// it into range, because the form submits the pre-filled value back
|
|
||||||
// and accepting it would be the ceiling not applying to the edit
|
|
||||||
// path. The 400 names the ceiling, so the fix is one field.
|
|
||||||
func parseMaxRetries(raw string, fallback int) (int, error) {
|
func parseMaxRetries(raw string, fallback int) (int, error) {
|
||||||
raw = strings.TrimSpace(raw)
|
raw = strings.TrimSpace(raw)
|
||||||
if raw == "" {
|
if raw == "" {
|
||||||
|
|||||||
@@ -441,10 +441,9 @@ func (s *Session) Regenerate(
|
|||||||
//
|
//
|
||||||
// Whichever comes first ends the session.
|
// Whichever comes first ends the session.
|
||||||
//
|
//
|
||||||
// A session that claims to be authenticated but carries no
|
// A session that claims to be authenticated but lacks the timestamp
|
||||||
// timestamps predates this check; it is treated as expired so the
|
// a deadline is measured from is treated as expired, so the user
|
||||||
// user re-authenticates rather than being granted an unbounded
|
// re-authenticates rather than being granted an unbounded session.
|
||||||
// session.
|
|
||||||
func (s *Session) expired(sess *sessions.Session) bool {
|
func (s *Session) expired(sess *sessions.Session) bool {
|
||||||
now := s.now()
|
now := s.now()
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user