fix: repair PR #263 blockers, drop unsafe stub commands, restore upstream deps - #1
Merged
isaquepinheiro merged 9 commits intoJul 22, 2026
Conversation
The committed sbom.cdx.json was not produced by this codebase: it lists gin-gonic/gin (never a dependency of boss) and a cobra version that does not match go.mod. Shipping a hand-written SBOM in a CRA-compliance change is indefensible, so it is removed. SBOMs belong in release artifacts. go.mod/go.sum are restored to upstream/main byte-for-byte. The downgrades (go 1.26->1.25, golangci-lint v2->v1, cobra 1.10.2->1.9.1, crypto 0.52->0.51) were unnecessary -- the branch builds clean on the upstream toolchain -- and the golangci-lint v1 binary cannot read this repo's version:"2" config, which is what turned the Lint job red.
boss sbom panicked on every invocation: runPkgSbom built an anonymous
struct and the generators asserted it back as map[string]interface{}.
Reproduced with a minimal project, then fixed by giving the manifest a
named type and passing it typed end to end.
Also corrected in the generated document:
- Versions now come from the lock file. boss.json holds constraints such
as "^3.0.0", which are not valid component versions; the lock has the
resolved ones. When an entry is missing the constraint is still emitted,
but the run warns and the component is marked boss:resolved=false.
- purl is pkg:github/<owner>/<repo>@<version>. "pkg:delphi" is not a
registered purl type, and boss dependencies are Git repositories.
- serialNumber is a real RFC 4122 v4 UUID from crypto/rand. The previous
timestamp arithmetic overflowed the final group past 12 hex digits and
placed the version nibble in the variant position, so every document
failed the CycloneDX serialNumber pattern.
- Components are emitted in sorted order; map iteration reordered the
SBOM on every run and churned the file in version control.
- No "hashes" entry is written. The digest boss keeps in the lock is a
directory fingerprint (utils.HashDir), not a cryptographic hash of a
distributed artifact, and must not be presented as one.
Removed pkg sign, pkg verify and scan. sign XORed a single byte and
wrote the certificate password's environment variable name in clear text
into the bundle; verify only checked that two marker strings were present
yet reported "Author signature verified successfully"; scan queried OSV
with ecosystem "Delphi", which OSV does not define, so every project
reported "Zero findings detected". Shipping those under a CRA-compliance
banner would state a guarantee the code does not provide. They can come
back in a dedicated change built on real cryptography and an advisory
source that actually covers Delphi.
Two further defects found while fixing the above:
- boss sbom wrote to ./dist instead of ./sbom. sbom and pack shared one
outputDir variable, and StringVar assigns the default at registration
time, so pack's default overwrote sbom's. Each command now owns its own.
- boss cra init generated "<name>.cdx.json" while boss cra looks for
sbom.cdx.json, so the wizard produced a file its own checker could not
see. Scoped names such as "acme/demo" also put a path separator in the
file name. Both now go through shared constants.
cra init no longer overwrites an existing SECURITY.md, the "100%" strings
in cra output are escaped (they printed as "100%!C(NOVERB)RA"), and files
holding generated output use the 0600/0750 modes already used elsewhere
in the repository.
boss login --token never existed. runPortalLogin was written but had no callers, and login.go was never touched, so the flag the help text and contribute's error message both told users to run failed with "unknown flag: --token". Without it no token could be stored, which left workspace clone, contribute and publish-sbom permanently answering "You must log in first". The flag now exists on the login command and routes to the portal flow; the git registry login is unchanged when it is absent. The client no longer rejects tokens by prefix. The portal owns token format, and a client-side check means every installed copy of boss breaks the day that format changes. An empty token is still refused locally. The config file that stores the token is written 0600 in a 0700 directory. It previously landed at 0644/0755, leaving an authentication token readable by every account on the machine -- and the change also walked back the permission tightening done upstream in a06efa1. Commands are grouped after config.RegisterCmd, not before. The grouping pass matched on cmd.Name(), but config registered the cache command afterwards, so cache kept an empty GroupID and cobra printed it in a stray "Additional Commands" block. Group titles now read as titles instead of "Available Commands (new):". contribute pushes with --force-with-lease. A plain --force combined with branch names drawn from only 10,000 random values could silently replace a contribution pushed earlier; branch names now come from crypto/rand. math/rand.Seed also went away -- it is a documented no-op since Go 1.24 and this repository's linter forbids math/rand outright. Restored what the feature branch had dropped from files it did not need to change: the appName/appDescription constants, defaultPackageVersion, projectTypeApp/projectTypePkg, the gochecknoglobals nolint markers on the cobra flag variables, and the staticcheck nolint on installer.GetDependency.
Mechanical fixes from the repository's own pre-commit hook: sentence-ending
periods on doc comments (godot), http.MethodPost instead of the "POST"
literal (usestdlibvars), strconv.FormatBool over fmt.Sprintf("%t"), and
fmt.Fprintf on the buffer instead of buf.WriteString(fmt.Sprintf(...)).
No behaviour change; build, vet and tests still pass.
golangci-lint --fix ran on Windows and edited three build-tagged _win.go files the branch has no reason to change. The Linux CI does not compile them, so upstream is green without these edits and they only widen the diff.
…ect writers
golangci-lint flagged this PR's additions to root.go/new.go for funlen,
gocognit, nestif, goconst, gosec and revive. The fixes are behaviour
preserving:
root.go
- Execute() was 108 lines (funlen limit 100). The command wiring and the
help-group pass moved into registerCommands/applyCommandGroups, and the
help/version fast-path detection into isHelpOrVersionInvocation. The
ordering guarantee that the comment already documented is kept: every
command, config.RegisterCmd included, is registered before the grouping
pass, so none of them ends up in cobra's "Additional Commands" block.
- Command names and group identifiers are now constants shared with the
command files, instead of literals repeated across the switch (goconst).
new.go
- doCreateProject had cognitive complexity 37 and a 16-deep nested block.
The per-IDE file writing moved into writeLazarusProjectFiles and
writeDelphiProjectFiles; the decision logic is untouched.
- Generated project files are written with 0600 instead of 0644 (gosec
G306), matching the mode already used everywhere else in the repo and
the 0750 MkdirAll convention adopted upstream in HashLoad#262.
- "src", "lazarus" and "delphi" became constants; packageJsonPath became
packageJSONPath (revive var-naming).
- The long <Import> line in dprojTemplate carries a //nolint:lll: wrapping
it would change the generated .dproj.
dependencies.go
- The --version flag name uses the shared flagNameVersion constant so the
literal no longer trips goconst.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… tighten modes
Two real defects the linters surfaced, plus the complexity/style debt of the
same files.
Defects
- runWorkspaceClone ended with os.Exit(1) while the manifest response body
was only closed by a defer, which os.Exit never runs (gocritic
exitAfterDefer). The HTTP call moved into fetchWorkspaceManifest, so the
body is closed before control returns to the function that may exit.
- Every response body Close and the directory handle in isDirPopulated had
their errors dropped silently (errcheck); they are now explicitly
discarded in a deferred closure.
Context propagation (noctx)
- The portal calls use http.NewRequestWithContext and every git/boss
subprocess uses exec.CommandContext, fed by cobra's cmd.Context(). No
behaviour changes today (the root command runs with a background
context), but the subprocesses and requests are now cancellable.
Complexity, without touching decision logic
- runWorkspaceClone (cognitive complexity 61) keeps its loop and its
success/skip/fail accounting; the per-repository work moved into
cloneWorkspaceRepo, createCodenameBranch and runBossInstall.
- runWorkspaceUpdate and runWorkspacePush had the same 4-level repository
discovery inlined twice; both now call discoverWorkspaceRepos, which
walks the directories in exactly the previous order.
- runWorkspaceStatus and injectDprojPaths delegate to findWorkspaceRootRepo,
collectDependencySearchPaths and mergeDprojSearchPaths.
- The status/ahead-behind git invocations share one gitCapture helper.
gosec
- MkdirAll 0755 -> 0750 and WriteFile 0644 -> 0600, following HashLoad#262 and the
modes already used across the repo. The .dproj rewrite keeps the mode of
the existing file, since os.WriteFile only applies perm on creation.
- G304/G703 on paths that come from the portal manifest, from a Glob inside
the cloned workspace or from an explicit --file/--spec flag, and G117 on
the config marshal (persisting the token locally is the point of the
file), are annotated with //nolint:gosec and a reason, as utils/hash.go
and git_native.go already do.
Style
- PortalBaseUrl/updatedXml/bossJsonPath/CloneUrl/SshUrl/PrUrl renamed to
the Go initialism spelling (revive); the JSON tags are untouched, so the
wire format with the portal is unchanged.
- Exported manifest types got doc comments; the status/HTTP if-else chains
became switches; long lines wrapped; the cra securityEmail package global
became a local flag variable passed to runCraInit.
- fmt.Printf/fmt.Print replaced by fmt.Fprint(f) on os.Stdout (forbidigo).
msg.Info was not an option: it appends a line break, and the CRA wizard
prompt has to stay on the same line as the answer.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…okups
- TestNewCommandRegistration reported a missing --type flag with t.Error
and then dereferenced the nil flag on the next line (staticcheck SA5011,
a guaranteed panic if the flag ever disappeared). It now uses t.Fatal.
- The four doCreateProject tests saved and restored the working directory
by hand. t.Chdir does it for them (usetesting), which also removes the
outer err variable that 23 govet shadow reports pointed at. The
remaining function-scope read errors were renamed so the inline
"if _, err := os.Stat(...)" checks shadow nothing.
- TestPubPascalCommands had cognitive complexity 22 from four hand-rolled
command lookups; they collapse into findCommand/assertSubcommands. The
assertions are the same.
- bossJsonPath -> bossJSONPath (revive var-naming).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
repo.Name comes from the portal's workspace manifest and was split on "/" with the second element taken unconditionally, in three places. A manifest entry without a slash panics the clone. The client does not control that data, so it now falls back to the whole name.
isaquepinheiro
merged commit Jul 22, 2026
87327f4
into
feature/pubpascal-cra-compliance
6 of 7 checks passed
This was referenced Jul 22, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Internal review PR. Target is
feature/pubpascal-cra-compliance, the head branch of upstream HashLoad#263 — merging this updates that PR, so it stays open until we approve it here.Addresses the two blockers found locally plus the maintainer's review (@snakeice, CHANGES_REQUESTED).
Blockers (both reproduced, both fixed)
boss sbompanicked on every run.runPkgSbombuilt an anonymous struct and the generators asserted it back asmap[string]interface{}. Reproduced against a minimal project — stack trace atpubpascal.go:854. Fixed by giving the manifest a named type.boss login --tokendid not exist.runPortalLoginhad zero callers andlogin.gowas never touched, so the flag the help text andcontributeboth told users to run failed withunknown flag. Nothing could store a token, soworkspace clone,contributeandpublish-sbomall answered "You must log in first". The flag is now wired.Removed:
pkg sign,pkg verify,scansignXORed one byte and wrote the certificate password's env var name in clear text into the bundle.verifychecked that two marker strings were present and reported "Author signature verified successfully".scanqueried OSV with ecosystem"Delphi", which OSV does not define, so every project reported "Zero findings detected".Under a CRA-compliance banner these state a guarantee the code does not provide. They come back in a dedicated change built on real cryptography and an advisory source that covers Delphi.
Also removed: the committed
sbom.cdx.jsonIt lists
gin-gonic/gin, never a dependency of boss, and a cobra version that does not matchgo.mod. It was not produced by this codebase.go.mod/go.sumrestored to upstreamByte-identical to
upstream/mainagain. The branch builds clean on Go 1.26, so the downgrades were unnecessary — and the golangci-lint v1 binary they pinned cannot read this repo'sversion: "2"config, which is what turned the Lint job red.Review items addressed
appName/appDescriptionconstants%in100%GroupIDswitch breaking silentlyconfig.RegisterCmdnow runs before the grouping pass;cachewas landing in a stray blockboss:resolved=falseon fallbackserialNumbernot RFC-4122crypto/randv4 UUIDscan/sign/verifypdv_validationgit push --force--force-with-lease; branch names fromcrypto/randPlus, found while fixing the above:
boss sbomwrote to./dist(sharedoutputDirvariable,StringVarassigns defaults at registration time);boss cra initgenerated a file name its own checker could not find;cra initoverwrote an existingSECURITY.md; the token config was written0644in a0755directory;math/rand.Seedis a no-op since Go 1.24 and is forbidden by this repo's linter; and the branch had dropped constants andnolintmarkers from files it did not need to change.Verification
go build ./...,go vet ./...andgo test ./...all clean.boss sbomandboss login --tokenexercised end to end against a scratch project. Command grouping confirmed in--help.Still open
Not in this PR: the
beevik/etreerefactor of the.dprojinjection (#10, duplicatesutils/librarypath), movingsetup.InitializeintoPersistentPreRun(#3), thecli/*subpackage split (#6),packmanifest defaults (#8), theboss pubpascalnamespacing (HashLoad#21), and the remaining lint/gosec backlog.