Skip to content

chore(deps): bump age to v1.3.2 - #100

Merged
steipete merged 1 commit into
mainfrom
chore/bump-age-v1.3.2
Aug 31, 2026
Merged

chore(deps): bump age to v1.3.2#100
steipete merged 1 commit into
mainfrom
chore/bump-age-v1.3.2

Conversation

@steipete

Copy link
Copy Markdown
Contributor

Update filippo.io/age from v1.3.1 to v1.3.2 for upstream fixes and input hardening, refresh the module checksums, and add a changelog entry. This adopts upstream's 2 MiB header and 1,024-recipient limits without changing crawlkit's backup APIs.

Validation: GOWORK=off go mod tidy, unchanged module files after a second tidy, go vet ./..., and go test -count=1 ./... with GOWORK=off; Codex autoreview clean through P3. Built crawlctl passed help, run, JSON status, and log checks. Real backup API runs wrote and restored encrypted JSONL snapshots and 200,000-byte files across age v1.3.1 → v1.3.2 and v1.3.2 → v1.3.1; wrong identities and truncated ciphertext were rejected. All runtime data used temporary fixtures.

@steipete
steipete requested a review from a team as a code owner August 31, 2026 06:16
@clawsweeper

clawsweeper Bot commented Aug 31, 2026

Copy link
Copy Markdown

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

@steipete
steipete merged commit e4d5cea into main Aug 31, 2026
8 checks passed
@steipete
steipete deleted the chore/bump-age-v1.3.2 branch August 31, 2026 06:19
@clawsweeper clawsweeper Bot added merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. P2 Normal priority bug or improvement with limited blast radius. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Aug 31, 2026
@clawsweeper

clawsweeper Bot commented Aug 31, 2026

Copy link
Copy Markdown

Codex review: needs real behavior proof before merge. Reviewed August 31, 2026, 2:19 AM ET / 06:19 UTC.

ClawSweeper review

What this changes

The PR updates the age encryption dependency to v1.3.2 and refreshes its resolved Go module checksums and unreleased changelog entry.

Merge readiness

Blocked until real behavior proof is added - 4 items remain

Keep open pending upgrade-safety coverage and real behavior proof: the narrow dependency update changes the encrypted-backup library’s stated input limits, while existing crawlkit validation accepts an unbounded recipient list.

Priority: P2
Reviewed head: 7479bb34e719d000d2b73b7ee3e0fe64ba136587
Owner decision: Required. See Decision needed.

Review scores

Measure Result What it means
Overall readiness 🧂 unranked krab (1/6) The dependency update is focused, but its upgrade boundary is unresolved and the claimed real behavior validation is not inspectable.
Proof confidence 🧂 unranked krab (1/6) Needs real behavior proof before merge: The changed production owner is the backup package’s direct age encryption/decryption path, but the PR body only asserts temporary-fixture backup and restore results without inspectable output or an artifact. Attach a redacted terminal trace or log from the real after-fix backup-and-restore scenarios, then update the PR body for automatic re-review or ask a maintainer to comment @clawsweeper re-review.
Patch quality 🦐 gold shrimp (3/6) 1 actionable review finding remain.

Verification

Check Result Evidence
Real behavior Needs proof Needs real behavior proof before merge: The changed production owner is the backup package’s direct age encryption/decryption path, but the PR body only asserts temporary-fixture backup and restore results without inspectable output or an artifact. Attach a redacted terminal trace or log from the real after-fix backup-and-restore scenarios, then update the PR body for automatic re-review or ask a maintainer to comment @clawsweeper re-review.
Evidence reviewed 7 items Introduced dependency change: The PR changes the direct age requirement from v1.3.1 to v1.3.2; the supplied introduced delta is complete and limited to the module files and changelog.
Encryption and restore dependency boundary: The production backup path delegates writing and reading encrypted shard data directly to age.
Recipient inputs have no crawlkit-side upper bound: parseRecipients accepts every non-empty valid recipient and passes the resulting slice to age.Encrypt; it does not preflight the PR-described 1,024-recipient limit.
Findings 1 actionable finding [P1] Cover the new encrypted-backup limits before upgrading
Security None None.

How this fits together

The shared backup package encrypts archive shards and files using age before they are stored in a local archive. Restore paths decrypt those artifacts, so dependency acceptance limits affect both backup creation and recovery.

flowchart LR
  A[Archive data and files] --> B[Backup writer]
  B --> C[Age encryption library]
  C --> D[Encrypted archive shards]
  D --> E[Backup restore]
  E --> F[Recovered local data]
Loading

Decision needed

Question Recommendation
Should crawlkit accept the age v1.3.2 recipient and header limits as an intentional upgrade-time rejection of unusually large encrypted-backup configurations and archives? Accept with explicit upgrade handling: Keep v1.3.2, add boundary coverage and a clear user-facing failure or recovery path for configurations and archives beyond its limits.

Why: The limits harden encrypted-input handling but can make previously accepted configuration or persisted ciphertext fail, and the desired upgrade behavior is a maintainer policy choice.

Before merge

  • Add real behavior proof - Needs real behavior proof before merge: The changed production owner is the backup package’s direct age encryption/decryption path, but the PR body only asserts temporary-fixture backup and restore results without inspectable output or an artifact. Attach a redacted terminal trace or log from the real after-fix backup-and-restore scenarios, then update the PR body for automatic re-review or ask a maintainer to comment @clawsweeper re-review.
  • Cover the new encrypted-backup limits before upgrading (P1) - parseRecipients accepts every non-empty recipient and delegates directly to age.Encrypt; this bump introduces the PR-described 1,024-recipient and 2 MiB-header limits. Existing large recipient configurations or archives can therefore fail after upgrade without a crawlkit preflight, recovery path, or boundary coverage. Add explicit handling and focused upgrade tests before merging.
  • Resolve merge risk (P1) - Existing configurations with more than 1,024 recipients, or archives whose encrypted headers exceed the new 2 MiB limit described by the PR, may begin failing only after upgrade; crawlkit currently has no preflight, migration, or documented recovery behavior for those cases.

Findings

  • [P1] Cover the new encrypted-backup limits before upgrading — go.mod:6
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Change surface 3 files affected; 10 added, 7 removed The patch contains no crawlkit source changes, so compatibility behavior comes entirely from the encrypted dependency upgrade.

Merge-risk options

Maintainer options:

  1. Cover the upgrade boundary before merge (recommended)
    Add focused tests and user-visible handling for the new recipient and header rejection paths, then provide redacted real backup/restore evidence.
  2. Accept the compatibility tradeoff
    Maintain the dependency bump while explicitly documenting that unusually large existing recipient sets or archive headers may be rejected after upgrade.

Technical review

Best possible solution:

Retain the upstream hardening only with an explicit upgrade decision, focused boundary/error coverage, clear recovery guidance for affected archives, and redacted evidence from a real backup-and-restore run.

Do we have a high-confidence way to reproduce the issue?

No high-confidence executed reproduction is available in this review. The case is source-reproducible at medium confidence: crawlkit admits all valid recipients and the PR states that the newly pinned dependency enforces a 1,024-recipient limit.

Is this the best way to solve the issue?

No—the version bump is not ready by itself because its intentional input hardening has an unaddressed upgrade boundary. A focused compatibility decision, coverage, and real-run evidence are the narrowest maintainable completion path.

Full review comments:

  • [P1] Cover the new encrypted-backup limits before upgrading — go.mod:6
    parseRecipients accepts every non-empty recipient and delegates directly to age.Encrypt; this bump introduces the PR-described 1,024-recipient and 2 MiB-header limits. Existing large recipient configurations or archives can therefore fail after upgrade without a crawlkit preflight, recovery path, or boundary coverage. Add explicit handling and focused upgrade tests before merging.
    Confidence: 0.87

Overall correctness: patch is incorrect
Overall confidence: 0.87

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 52f0fb12e4d2.

Labels

Label changes:

  • add P2: This is a bounded encryption-backup upgrade concern with potential impact on uncommon existing configurations rather than a reported live outage.
  • add merge-risk: 🚨 compatibility: The PR’s stated new age limits can reject previously accepted recipient configurations or persisted encrypted headers after upgrade.
  • add rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🦐 gold shrimp.
  • add status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The changed production owner is the backup package’s direct age encryption/decryption path, but the PR body only asserts temporary-fixture backup and restore results without inspectable output or an artifact. Attach a redacted terminal trace or log from the real after-fix backup-and-restore scenarios, then update the PR body for automatic re-review or ask a maintainer to comment @clawsweeper re-review.

Label justifications:

  • P2: This is a bounded encryption-backup upgrade concern with potential impact on uncommon existing configurations rather than a reported live outage.
  • merge-risk: 🚨 compatibility: The PR’s stated new age limits can reject previously accepted recipient configurations or persisted encrypted headers after upgrade.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🦐 gold shrimp.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The changed production owner is the backup package’s direct age encryption/decryption path, but the PR body only asserts temporary-fixture backup and restore results without inspectable output or an artifact. Attach a redacted terminal trace or log from the real after-fix backup-and-restore scenarios, then update the PR body for automatic re-review or ask a maintainer to comment @clawsweeper re-review.

Evidence

What I checked:

  • Introduced dependency change: The PR changes the direct age requirement from v1.3.1 to v1.3.2; the supplied introduced delta is complete and limited to the module files and changelog. (go.mod:6, 7479bb34e719)
  • Encryption and restore dependency boundary: The production backup path delegates writing and reading encrypted shard data directly to age. (backup/crypto.go:78, 7479bb34e719)
  • Recipient inputs have no crawlkit-side upper bound: parseRecipients accepts every non-empty valid recipient and passes the resulting slice to age.Encrypt; it does not preflight the PR-described 1,024-recipient limit. (backup/crypto.go:120, 7479bb34e719)
  • Existing test coverage is below the new boundary: The encrypted snapshot round-trip test uses one generated recipient, so it does not define intended behavior for high-recipient configurations or rejected legacy headers. (backup/backup_test.go:31, 7479bb34e719)
  • Feature-history routing: Local history identifies the shared encrypted-backup feature commit and subsequent backup hardening work; this supports routing but not a line-introduction claim. (backup/crypto.go:16, 55196d74e0e6)
  • Reviewer proof gap: The complete supplied PR body describes real temporary-fixture runs but contains no inspectable terminal output, redacted log, recording, or linked artifact showing those after-fix results. (7479bb34e719)

Likely related people:

  • steipete: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)
  • Vincent Koc: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

  • Add focused coverage and a clear failure or recovery path for the new recipient and header limits.
  • Attach redacted real backup-and-restore output, including the relevant upgrade compatibility scenario.

Rating scale

Score Internal tier Crab rank Meaning
6/6 S 🦀 challenger crab Exceptional readiness
5/6 A 🦞 diamond lobster Very strong readiness
4/6 B 🐚 platinum hermit Good normal PR; ordinary maintainer review
3/6 C 🦐 gold shrimp Useful, but confidence is limited
2/6 D 🦪 silver shellfish Proof or implementation needs work
1/6 F 🧂 unranked krab Not merge-ready
N/A NA 🌊 off-meta tidepool Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. P2 Normal priority bug or improvement with limited blast radius. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant