Repository review & feedback

[elrido]

Salut Julian,

Long overdue, I've finally reviewed your project, as discussed a few weeks ago. I'm not too much a Gopher, more of a Rustacian, so I can't comment much on the Go code itself, though I did see that you are using standard tooling in your CI workflow such as gofmt, go lint, go vet, govulncheck, among others.

All the useful and recommended markdown files are there, including README, LICENSE, SECURITY and CHANGELOG. I would not recommending to change the license and IANAL, but I understand that AGPL specifically addresses concerns regarding third parties turning your software into a SaaS service, without publishing any of their modifications. As far as I understand this tool, it relies on syncthing protocol without any modifications, so on your end a GPL would have been "libre" enough for a client. But AGPL doesn't hurt you in that regard, as long as your not striving for RMS-purist levels of free software standards.

I've only some very minor comments on the github action workflows:

https://github.com/julian-corbet/dotkeeper/blob/dce1591597664b0768e9cc2be640b34afaa8876a/.github/workflows/release.yml#L8-L9

Since this is a public repo, you can actually restrict the permissions granted to the GITHUB_TOKEN entirely, as all of the contents are read-only anonymously:

permissions: {}

This only becomes an issue if you are using temporary private repos, for example when collaborating on security fixes.

Only learned of this recently, and given that all workflows already use minimal permissions, it may not be necessary, but it seems you can ask the checkout action to not keep the token used around. Maybe more of a defense in depth measure, in your case.

I'm not quite sure if this step is necessary, except if that user does show up in the coverage profile you upload as an artifact:

https://github.com/julian-corbet/dotkeeper/blob/dce1591597664b0768e9cc2be640b34afaa8876a/.github/workflows/ci.yml#L26-L29

As you offer a Makefile, you may want to add a "help" target that outputs each targets help text that you add after two hashes:

# Run tests with coverage profile, then summarise. Writes:
#   coverage.out — raw profile for tooling (go tool cover, codecov, etc.)
#   coverage.html — human-readable browser view
cover: ## Run tests with coverage profile, then summarise.
[...]

help: ## Displays these usage instructions.
	@echo "Usage: make <target(s)>"
	@echo
	@echo "Specify one or multiple of the following targets and they will be processed in the given order:"
	@awk 'BEGIN {FS = ":.*?## "} /^[a-zA-Z_-]+:.*?## / {printf "%-16s%s\n", $$1, $$2}' $(MAKEFILE_LIST)

You already offer an Alpine Linux package (.apk) on each release. Given the release cadence, it may be easiest for you to get it published yourself directly into alpine testing and once established into community (so it will reach stable releases, too). You would need a user account on https://gitlab.alpinelinux.org/ and a fork of the aports repo into your personal account there.

• example minimal go project you could use as a reference
• wiki page on developing alpine packages - you need an alpine edge environment to follow that guide. It can be a container, as long as you either persist the keys in a volume or regenerate them every time.
• specifically, once you have created your APKBUILD spec file, you can use abuild checksum to update the checksum in it and abuild -r will then test-build a package, before you commit the changes and push them to a new branch.
• it may be worth doing this manually until you feel confident on all steps, before automating it in a gitlab workflow like you do for the AUR packaging

Let me know if this was the type of input you were looking for or if there are any specific areas you'd like me to take a closer look

[julian-corbet]

Salut @elrido — thanks for taking the time! I worked through your suggestions and acted on all five over the past day:

1. permissions: {} on workflows — chore: workflow hardening + Makefile help target (#82 feedback) #88. Both ci.yml and release.yml now declare zero scopes on GITHUB_TOKEN. Release upload uses PACKAGES_TOKEN (a user PAT) so the workflow-level lockdown is no constraint.

2. persist-credentials: false on checkout — chore: workflow hardening + Makefile help target (#82 feedback) #88. Defense in depth applied to every actions/checkout invocation across both workflows.

3. "Configure git identity for tests" step — investigated; it's required because several integration tests run git commit in t.TempDir() and refuse without a configured user. Coverage profile doesn't leak the identity (it's sampling data, not log output). chore: workflow hardening + Makefile help target (#82 feedback) #88 added an inline comment explaining the reasoning.

4. Makefile help target — chore: workflow hardening + Makefile help target (#82 feedback) #88. Self-documenting ## description per target; help is now the default goal. make alone prints the usage.

5. Alpine aports/community — chore(alpine): first-class APKBUILD + GitHub Actions native build (#82) #89 + follow-up fixes fix(alpine): skip abuild strip — Go binary already stripped #90, fix(alpine): skip tracedeps — apk 4.x renamed --who-owns #91, fix(alpine): trust disposable signing key during index step #92. Added alpine/APKBUILD + a dedicated alpine.yml workflow that runs abuild natively in alpine:edge. As of v1.2.6 the release page carries the native .apk artifacts (dotkeeper_v1.2.6_alpine_x86_64.apk and _aarch64.apk). Three iterative fixes were needed because alpine:edge has both API churn (apk-tools 4.x renamed info --who-owns) and cross-build quirks (abuild looks for the host's native strip even when CARCH differs). Maintainer docs for upstream submission to gitlab.alpinelinux.org/alpine/aports are in alpine/README.md.

Really appreciated the depth of the review — items 1–4 turned into small but real quality bumps, and item 5 has dotkeeper genuinely Alpine-native now. The submission to aports/community itself is the manual final step (needs my account); the package is ready to go whenever I find the time to push the MR.