detached 4d9eaac

#8 chore(hexcheck): bump to v0.4.0 and drop unused ignore directives

state
merged · 4d9eaac · fast-forward
branch
chore/hexcheck-v0.4.0 → main
opened
bnema ·
updated

Bump hexcheck to v0.4.0 and make hexcheck -fail-on warn pass. Comment-only changes in adapters.

  • Remove 89 ignore directives that v0.4.0 reports as unused.
  • Annotate the 16 new findings with accurate reasons.
  • Rewrite wrong reasons. 14 functions hold business rules marked 'pending move to domain/usecase' (revision resolution, commit budget, diff budget, workflow guard, commitsAhead, banner, mrPage, repoRecord, CreateRun, RotateOIDCKey, issuesOn, issueSubmittedPage); they move in follow-up MRs.

Ignore directives: 129 -> 57.

CommitSubjectAuthorDateSignatureStatus
4d9eaacfix(usecase): drop the unused repository read before the merge lockbriceGPG Verified
05fc636refactor: reuse the domain settings predicates and tighten the moved rules after reviewbriceGPG Verified
558568cfix(web,rpc): handle an API or web server built without merge requests or OIDC keysbriceGPG Verified
5344be5merge: hexmove/workflow into chore/hexcheck-v0.4.0briceGPG Verified
9f9eee5merge: hexmove/history into chore/hexcheck-v0.4.0briceGPG Verified
c902d2fmerge: hexmove/policies into chore/hexcheck-v0.4.0briceGPG Verified
6b4272dmerge: hexmove/budgets into chore/hexcheck-v0.4.0briceGPG Verified
fe7ab7dmerge: hexmove/revision into chore/hexcheck-v0.4.0briceGPG Verified
a1ec1afrefactor(domain): own the workflow-change rule for job tokens in WorkflowChangeOfbriceGPG Verified
6080cberefactor(usecase): own branch comparison, push summaries and unread commits in MergeRequestsbriceGPG Verified
c0f84a6refactor(usecase): own repo defaults, disabled workflows, OIDC rotation and issue visibility rulesbriceGPG Verified
0424a8arefactor(domain): own commit-object and diff read budgetsbriceGPG Verified
ac21956chore(hexcheck): bump to v0.4.0 and drop unused ignore directivesbriceGPG Verified
e188800refactor(domain): own the revision resolution order in Revision.CandidatesbriceGPG Verified

Checks 8

102 files · +2491 -1408
7/8
M docs/merge-requests.md +13 -0
M go.mod +1 -1
M go.sum +2 -2
M internal/adapters/cilogs/cilogs.go +2 -0
M internal/adapters/cisummaries/cisummaries.go +2 -0
M internal/adapters/ermitegit/commitobjects.go +12 -11
M internal/adapters/ermitegit/convert.go +25 -36
M internal/adapters/ermitegit/ermitegit.go +1 -1
M internal/adapters/gitcli/archive.go +0 -2
M internal/adapters/gitcli/changed.go +0 -4
M internal/adapters/gitcli/commitobjects.go +11 -15
M internal/adapters/gitcli/diff.go +15 -47
M internal/adapters/gitcli/export_test.go +0 -6
M internal/adapters/gitcli/merge.go +0 -16
M internal/adapters/gitcli/pull.go +0 -2
A internal/adapters/gitcli/push_trees.go +400 -0
R internal/adapters/gitcli/workflow_guard_test.go → internal/adapters/gitcli/push_trees_test.go +158 -217 renamed, 53% similar
M internal/adapters/gitcli/read.go +31 -60
M internal/adapters/gitcli/repos.go +1 -3
M internal/adapters/gitcli/stats.go +0 -6
D internal/adapters/gitcli/workflow_guard.go +0 -438
M internal/adapters/githttp/handler.go +4 -4
M internal/adapters/githttp/hooks.go +0 -2
M internal/adapters/kvstore/ci.go +11 -18
M internal/adapters/kvstore/ci_artifact.go +3 -5
M internal/adapters/kvstore/ci_concurrency.go +0 -4
@@ -103,7 +103,6 @@ func encodeConcurrencyRecord(r domain.CIConcurrencyRecord) []byte {
103103 return mustMarshal(pb)
104104 }
105105
106-//hexcheck:ignore suspicious-business-logic-in-adapter maps the stored record to the domain record field by field: no rule is decided here
107106 func decodeConcurrencyRecord(val []byte) (domain.CIConcurrencyRecord, error) {
108107 var pb storagepb.CIConcurrencyRecord
109108 if err := proto.Unmarshal(val, &pb); err != nil {
@@ -146,7 +145,6 @@ func encodeConcurrencyMark(m domain.CIConcurrencyMark) []byte {
146145 return mustMarshal(pb)
147146 }
148147
149-//hexcheck:ignore suspicious-business-logic-in-adapter maps the stored mark to the domain mark field by field: no rule is decided here
150148 func decodeConcurrencyMark(val []byte) (domain.CIConcurrencyMark, error) {
151149 var pb storagepb.CIConcurrencyMark
152150 if err := proto.Unmarshal(val, &pb); err != nil {
@@ -257,8 +255,6 @@ func (c *CI) UpdateConcurrency(ctx context.Context, repo domain.RepoName, group
257255 }
258256
259257 // ListConcurrency returns every stored group with its marks.
260-//
261-//hexcheck:ignore suspicious-business-logic-in-adapter lists and decodes records and marks: no rule is decided here
262258 func (c *CI) ListConcurrency(ctx context.Context) ([]ports.CIConcurrencyGroup, error) {
263259 type id struct{ repo, group string }
264260 snap := c.db.Snapshot(ctx)
@@ -103,7 +103,6 @@ func encodeConcurrencyRecord(r domain.CIConcurrencyRecord) []byte {
103 return mustMarshal(pb)103 return mustMarshal(pb)
104 }104 }
105 105
106-//hexcheck:ignore suspicious-business-logic-in-adapter maps the stored record to the domain record field by field: no rule is decided here
107 func decodeConcurrencyRecord(val []byte) (domain.CIConcurrencyRecord, error) {106 func decodeConcurrencyRecord(val []byte) (domain.CIConcurrencyRecord, error) {
108 var pb storagepb.CIConcurrencyRecord107 var pb storagepb.CIConcurrencyRecord
109 if err := proto.Unmarshal(val, &pb); err != nil {108 if err := proto.Unmarshal(val, &pb); err != nil {
@@ -146,7 +145,6 @@ func encodeConcurrencyMark(m domain.CIConcurrencyMark) []byte {
146 return mustMarshal(pb)145 return mustMarshal(pb)
147 }146 }
148 147
149-//hexcheck:ignore suspicious-business-logic-in-adapter maps the stored mark to the domain mark field by field: no rule is decided here
150 func decodeConcurrencyMark(val []byte) (domain.CIConcurrencyMark, error) {148 func decodeConcurrencyMark(val []byte) (domain.CIConcurrencyMark, error) {
151 var pb storagepb.CIConcurrencyMark149 var pb storagepb.CIConcurrencyMark
152 if err := proto.Unmarshal(val, &pb); err != nil {150 if err := proto.Unmarshal(val, &pb); err != nil {
@@ -257,8 +255,6 @@ func (c *CI) UpdateConcurrency(ctx context.Context, repo domain.RepoName, group
257 }255 }
258 256
259 // ListConcurrency returns every stored group with its marks.257 // ListConcurrency returns every stored group with its marks.
260-//
261-//hexcheck:ignore suspicious-business-logic-in-adapter lists and decodes records and marks: no rule is decided here
262 func (c *CI) ListConcurrency(ctx context.Context) ([]ports.CIConcurrencyGroup, error) {258 func (c *CI) ListConcurrency(ctx context.Context) ([]ports.CIConcurrencyGroup, error) {
263 type id struct{ repo, group string }259 type id struct{ repo, group string }
264 snap := c.db.Snapshot(ctx)260 snap := c.db.Snapshot(ctx)
M internal/adapters/kvstore/ci_settings.go +6 -4
@@ -85,6 +85,8 @@ func settingsKeys(ctx context.Context, snap *kv.Snapshot, repo domain.RepoName)
8585
8686 // BindSecretsKey stores the key check when there is none and compares it
8787 // otherwise.
88+//
89+//hexcheck:ignore suspicious-business-logic-in-adapter initializes or compares the persisted encryption-key check with compare-and-swap retries
8890 func (s *Settings) BindSecretsKey(ctx context.Context, check []byte) error {
8991 key := []byte(secretsKeyCheck)
9092 for range updateAttempts {
@@ -111,6 +113,8 @@ func (s *Settings) BindSecretsKey(ctx context.Context, check []byte) error {
111113
112114 // stage makes the batch depend on the scope: the repository key is guarded
113115 // and, for an environment, its record is required at the version read.
116+//
117+//hexcheck:ignore suspicious-business-logic-in-adapter validates a settings scope and adds repository and environment existence/version preconditions to a storage batch
114118 func (s *Settings) stage(ctx context.Context, b *kv.Batch, scope domain.SettingsScope) error {
115119 if !scope.Valid() {
116120 return fmt.Errorf("%w: scope", domain.ErrInvalid)
@@ -203,6 +207,8 @@ func (s *Settings) GetSealedSecret(ctx context.Context, scope domain.SettingsSco
203207 }
204208
205209 // existsIn is exists on a snapshot, so a listing and its check agree.
210+//
211+//hexcheck:ignore suspicious-business-logic-in-adapter validates a settings scope and checks repository and environment keys in the same snapshot as a listing
206212 func (s *Settings) existsIn(ctx context.Context, snap *kv.Snapshot, scope domain.SettingsScope) error {
207213 if !scope.Valid() {
208214 return fmt.Errorf("%w: scope", domain.ErrInvalid)
@@ -309,8 +315,6 @@ func decodeEnvironment(val []byte) (domain.Environment, error) {
309315 }
310316
311317 // CreateEnvironment implements ports.CISettingsStore.
312-//
313-//hexcheck:ignore suspicious-business-logic-in-adapter create-if-absent under a version precondition: a storage concern, no policy
314318 func (s *Settings) CreateEnvironment(ctx context.Context, repo domain.RepoName, name string, at time.Time) (domain.Environment, error) {
315319 key := envKey(repo, name)
316320 for range updateAttempts {
@@ -353,8 +357,6 @@ func (s *Settings) ListEnvironments(ctx context.Context, repo domain.RepoName) (
353357 }
354358
355359 // DeleteEnvironment implements ports.CISettingsStore.
356-//
357-//hexcheck:ignore suspicious-business-logic-in-adapter removes the keys of an environment in one batch: a storage concern, no policy
358360 func (s *Settings) DeleteEnvironment(ctx context.Context, repo domain.RepoName, name string) error {
359361 key := envKey(repo, name)
360362 scope := domain.EnvironmentScope(repo, name)
@@ -85,6 +85,8 @@ func settingsKeys(ctx context.Context, snap *kv.Snapshot, repo domain.RepoName)
85 85
86 // BindSecretsKey stores the key check when there is none and compares it86 // BindSecretsKey stores the key check when there is none and compares it
87 // otherwise.87 // otherwise.
88+//
89+//hexcheck:ignore suspicious-business-logic-in-adapter initializes or compares the persisted encryption-key check with compare-and-swap retries
88 func (s *Settings) BindSecretsKey(ctx context.Context, check []byte) error {90 func (s *Settings) BindSecretsKey(ctx context.Context, check []byte) error {
89 key := []byte(secretsKeyCheck)91 key := []byte(secretsKeyCheck)
90 for range updateAttempts {92 for range updateAttempts {
@@ -111,6 +113,8 @@ func (s *Settings) BindSecretsKey(ctx context.Context, check []byte) error {
111 113
112 // stage makes the batch depend on the scope: the repository key is guarded114 // stage makes the batch depend on the scope: the repository key is guarded
113 // and, for an environment, its record is required at the version read.115 // and, for an environment, its record is required at the version read.
116+//
117+//hexcheck:ignore suspicious-business-logic-in-adapter validates a settings scope and adds repository and environment existence/version preconditions to a storage batch
114 func (s *Settings) stage(ctx context.Context, b *kv.Batch, scope domain.SettingsScope) error {118 func (s *Settings) stage(ctx context.Context, b *kv.Batch, scope domain.SettingsScope) error {
115 if !scope.Valid() {119 if !scope.Valid() {
116 return fmt.Errorf("%w: scope", domain.ErrInvalid)120 return fmt.Errorf("%w: scope", domain.ErrInvalid)
@@ -203,6 +207,8 @@ func (s *Settings) GetSealedSecret(ctx context.Context, scope domain.SettingsSco
203 }207 }
204 208
205 // existsIn is exists on a snapshot, so a listing and its check agree.209 // existsIn is exists on a snapshot, so a listing and its check agree.
210+//
211+//hexcheck:ignore suspicious-business-logic-in-adapter validates a settings scope and checks repository and environment keys in the same snapshot as a listing
206 func (s *Settings) existsIn(ctx context.Context, snap *kv.Snapshot, scope domain.SettingsScope) error {212 func (s *Settings) existsIn(ctx context.Context, snap *kv.Snapshot, scope domain.SettingsScope) error {
207 if !scope.Valid() {213 if !scope.Valid() {
208 return fmt.Errorf("%w: scope", domain.ErrInvalid)214 return fmt.Errorf("%w: scope", domain.ErrInvalid)
@@ -309,8 +315,6 @@ func decodeEnvironment(val []byte) (domain.Environment, error) {
309 }315 }
310 316
311 // CreateEnvironment implements ports.CISettingsStore.317 // CreateEnvironment implements ports.CISettingsStore.
312-//
313-//hexcheck:ignore suspicious-business-logic-in-adapter create-if-absent under a version precondition: a storage concern, no policy
314 func (s *Settings) CreateEnvironment(ctx context.Context, repo domain.RepoName, name string, at time.Time) (domain.Environment, error) {318 func (s *Settings) CreateEnvironment(ctx context.Context, repo domain.RepoName, name string, at time.Time) (domain.Environment, error) {
315 key := envKey(repo, name)319 key := envKey(repo, name)
316 for range updateAttempts {320 for range updateAttempts {
@@ -353,8 +357,6 @@ func (s *Settings) ListEnvironments(ctx context.Context, repo domain.RepoName) (
353 }357 }
354 358
355 // DeleteEnvironment implements ports.CISettingsStore.359 // DeleteEnvironment implements ports.CISettingsStore.
356-//
357-//hexcheck:ignore suspicious-business-logic-in-adapter removes the keys of an environment in one batch: a storage concern, no policy
358 func (s *Settings) DeleteEnvironment(ctx context.Context, repo domain.RepoName, name string) error {360 func (s *Settings) DeleteEnvironment(ctx context.Context, repo domain.RepoName, name string) error {
359 key := envKey(repo, name)361 key := envKey(repo, name)
360 scope := domain.EnvironmentScope(repo, name)362 scope := domain.EnvironmentScope(repo, name)
M internal/adapters/kvstore/issues.go +0 -8
@@ -84,8 +84,6 @@ func issueKeys(ctx context.Context, snap *kv.Snapshot, repo domain.RepoName) [][
8484 // Create takes the next number of the repository from mrseq/<repo>, the
8585 // sequence of the merge requests, and stores the issue under it; see
8686 // createNumbered. A missing repository gives domain.ErrNotFound.
87-//
88-//hexcheck:ignore suspicious-business-logic-in-adapter takes the next number under a version precondition: a storage concern, no policy
8987 func (i *Issues) Create(ctx context.Context, issue domain.Issue) (domain.Issue, error) {
9088 err := createNumbered(ctx, i.db, i.locks, issue.Repo, func(b *kv.Batch, n int64) error {
9189 issue.Number = n
@@ -118,8 +116,6 @@ func (i *Issues) Get(ctx context.Context, repo domain.RepoName, number int64) (d
118116
119117 // List returns a page of the issues of the repository, highest number first. The
120118 // page token is the last number of the previous page.
121-//
122-//hexcheck:ignore suspicious-business-logic-in-adapter pages through the keys of a prefix: a storage concern, no policy
123119 func (i *Issues) List(ctx context.Context, repo domain.RepoName, params ports.ListIssuesParams) (ports.IssuePage, error) {
124120 before := int64(0) // 0: from the highest
125121 if params.PageToken != "" {
@@ -225,8 +221,6 @@ func (i *Issues) ListOpenAll(ctx context.Context) ([]domain.Issue, error) {
225221 // Update replaces the issue with a version precondition and moves its index key
226222 // in the same batch. The repository must exist (see guardRepoKey), so that a
227223 // concurrent delete of the repository leaves no index key behind.
228-//
229-//hexcheck:ignore suspicious-business-logic-in-adapter port method name only
230224 func (i *Issues) Update(ctx context.Context, issue domain.Issue) error {
231225 val, err := encodeIssue(issue)
232226 if err != nil {
@@ -330,7 +324,6 @@ func encodeIssue(issue domain.Issue) ([]byte, error) {
330324 }), nil
331325 }
332326
333-//hexcheck:ignore suspicious-business-logic-in-adapter schema decoding, not a policy
334327 func decodeIssue(repo domain.RepoName, item kv.Item) (domain.Issue, error) {
335328 var pb storagepb.Issue
336329 if err := proto.Unmarshal(item.Value, &pb); err != nil {
@@ -373,7 +366,6 @@ func encodeIssueComment(c domain.IssueComment) ([]byte, error) {
373366 }), nil
374367 }
375368
376-//hexcheck:ignore suspicious-business-logic-in-adapter schema decoding, not a policy
377369 func decodeIssueComment(item kv.Item) (domain.IssueComment, error) {
378370 var pb storagepb.IssueComment
379371 if err := proto.Unmarshal(item.Value, &pb); err != nil {
@@ -84,8 +84,6 @@ func issueKeys(ctx context.Context, snap *kv.Snapshot, repo domain.RepoName) [][
84 // Create takes the next number of the repository from mrseq/<repo>, the84 // Create takes the next number of the repository from mrseq/<repo>, the
85 // sequence of the merge requests, and stores the issue under it; see85 // sequence of the merge requests, and stores the issue under it; see
86 // createNumbered. A missing repository gives domain.ErrNotFound.86 // createNumbered. A missing repository gives domain.ErrNotFound.
87-//
88-//hexcheck:ignore suspicious-business-logic-in-adapter takes the next number under a version precondition: a storage concern, no policy
89 func (i *Issues) Create(ctx context.Context, issue domain.Issue) (domain.Issue, error) {87 func (i *Issues) Create(ctx context.Context, issue domain.Issue) (domain.Issue, error) {
90 err := createNumbered(ctx, i.db, i.locks, issue.Repo, func(b *kv.Batch, n int64) error {88 err := createNumbered(ctx, i.db, i.locks, issue.Repo, func(b *kv.Batch, n int64) error {
91 issue.Number = n89 issue.Number = n
@@ -118,8 +116,6 @@ func (i *Issues) Get(ctx context.Context, repo domain.RepoName, number int64) (d
118 116
119 // List returns a page of the issues of the repository, highest number first. The117 // List returns a page of the issues of the repository, highest number first. The
120 // page token is the last number of the previous page.118 // page token is the last number of the previous page.
121-//
122-//hexcheck:ignore suspicious-business-logic-in-adapter pages through the keys of a prefix: a storage concern, no policy
123 func (i *Issues) List(ctx context.Context, repo domain.RepoName, params ports.ListIssuesParams) (ports.IssuePage, error) {119 func (i *Issues) List(ctx context.Context, repo domain.RepoName, params ports.ListIssuesParams) (ports.IssuePage, error) {
124 before := int64(0) // 0: from the highest120 before := int64(0) // 0: from the highest
125 if params.PageToken != "" {121 if params.PageToken != "" {
@@ -225,8 +221,6 @@ func (i *Issues) ListOpenAll(ctx context.Context) ([]domain.Issue, error) {
225 // Update replaces the issue with a version precondition and moves its index key221 // Update replaces the issue with a version precondition and moves its index key
226 // in the same batch. The repository must exist (see guardRepoKey), so that a222 // in the same batch. The repository must exist (see guardRepoKey), so that a
227 // concurrent delete of the repository leaves no index key behind.223 // concurrent delete of the repository leaves no index key behind.
228-//
229-//hexcheck:ignore suspicious-business-logic-in-adapter port method name only
230 func (i *Issues) Update(ctx context.Context, issue domain.Issue) error {224 func (i *Issues) Update(ctx context.Context, issue domain.Issue) error {
231 val, err := encodeIssue(issue)225 val, err := encodeIssue(issue)
232 if err != nil {226 if err != nil {
@@ -330,7 +324,6 @@ func encodeIssue(issue domain.Issue) ([]byte, error) {
330 }), nil324 }), nil
331 }325 }
332 326
333-//hexcheck:ignore suspicious-business-logic-in-adapter schema decoding, not a policy
334 func decodeIssue(repo domain.RepoName, item kv.Item) (domain.Issue, error) {327 func decodeIssue(repo domain.RepoName, item kv.Item) (domain.Issue, error) {
335 var pb storagepb.Issue328 var pb storagepb.Issue
336 if err := proto.Unmarshal(item.Value, &pb); err != nil {329 if err := proto.Unmarshal(item.Value, &pb); err != nil {
@@ -373,7 +366,6 @@ func encodeIssueComment(c domain.IssueComment) ([]byte, error) {
373 }), nil366 }), nil
374 }367 }
375 368
376-//hexcheck:ignore suspicious-business-logic-in-adapter schema decoding, not a policy
377 func decodeIssueComment(item kv.Item) (domain.IssueComment, error) {369 func decodeIssueComment(item kv.Item) (domain.IssueComment, error) {
378 var pb storagepb.IssueComment370 var pb storagepb.IssueComment
379 if err := proto.Unmarshal(item.Value, &pb); err != nil {371 if err := proto.Unmarshal(item.Value, &pb); err != nil {
M internal/adapters/kvstore/issues_pending.go +0 -5
@@ -94,8 +94,6 @@ func (s *PendingIssues) Get(ctx context.Context, repo domain.RepoName, id domain
9494 // creation time then repository then id. There are at most
9595 // domain.MaxPendingIssues of them (a little more under a race), so they are
9696 // read and sorted. The page token is the position of the last item returned.
97-//
98-//hexcheck:ignore suspicious-business-logic-in-adapter pages through the keys of a prefix: a storage concern, no policy
9997 func (s *PendingIssues) List(ctx context.Context, repo domain.RepoName, page ports.ListPendingParams) (ports.PendingIssuePage, error) {
10098 prefix := pendingPrefix
10199 if !repo.IsZero() {
@@ -216,8 +214,6 @@ func (s *PendingIssues) Count(ctx context.Context) (int, error) {
216214 // Approve numbers the pending issue from mrseq/<repo> (see createNumbered),
217215 // writes the open issue and its index key and deletes the pending record, in
218216 // one batch that requires the pending record to be unchanged.
219-//
220-//hexcheck:ignore suspicious-business-logic-in-adapter only sets the number under a version precondition; the issue itself is built by domain.ApprovedIssue
221217 func (s *PendingIssues) Approve(ctx context.Context, repo domain.RepoName, id domain.PendingIssueID, now time.Time) (domain.Issue, error) {
222218 var number int64
223219 err := createNumbered(ctx, s.db, s.locks, repo, func(b *kv.Batch, n int64) error {
@@ -291,7 +287,6 @@ func encodePending(p domain.PendingIssue) ([]byte, error) {
291287 }), nil
292288 }
293289
294-//hexcheck:ignore suspicious-business-logic-in-adapter schema decoding, not a policy
295290 func decodePending(repo domain.RepoName, id domain.PendingIssueID, item kv.Item) (domain.PendingIssue, error) {
296291 var pb storagepb.PendingIssue
297292 if err := proto.Unmarshal(item.Value, &pb); err != nil {
@@ -94,8 +94,6 @@ func (s *PendingIssues) Get(ctx context.Context, repo domain.RepoName, id domain
94 // creation time then repository then id. There are at most94 // creation time then repository then id. There are at most
95 // domain.MaxPendingIssues of them (a little more under a race), so they are95 // domain.MaxPendingIssues of them (a little more under a race), so they are
96 // read and sorted. The page token is the position of the last item returned.96 // read and sorted. The page token is the position of the last item returned.
97-//
98-//hexcheck:ignore suspicious-business-logic-in-adapter pages through the keys of a prefix: a storage concern, no policy
99 func (s *PendingIssues) List(ctx context.Context, repo domain.RepoName, page ports.ListPendingParams) (ports.PendingIssuePage, error) {97 func (s *PendingIssues) List(ctx context.Context, repo domain.RepoName, page ports.ListPendingParams) (ports.PendingIssuePage, error) {
100 prefix := pendingPrefix98 prefix := pendingPrefix
101 if !repo.IsZero() {99 if !repo.IsZero() {
@@ -216,8 +214,6 @@ func (s *PendingIssues) Count(ctx context.Context) (int, error) {
216 // Approve numbers the pending issue from mrseq/<repo> (see createNumbered),214 // Approve numbers the pending issue from mrseq/<repo> (see createNumbered),
217 // writes the open issue and its index key and deletes the pending record, in215 // writes the open issue and its index key and deletes the pending record, in
218 // one batch that requires the pending record to be unchanged.216 // one batch that requires the pending record to be unchanged.
219-//
220-//hexcheck:ignore suspicious-business-logic-in-adapter only sets the number under a version precondition; the issue itself is built by domain.ApprovedIssue
221 func (s *PendingIssues) Approve(ctx context.Context, repo domain.RepoName, id domain.PendingIssueID, now time.Time) (domain.Issue, error) {217 func (s *PendingIssues) Approve(ctx context.Context, repo domain.RepoName, id domain.PendingIssueID, now time.Time) (domain.Issue, error) {
222 var number int64218 var number int64
223 err := createNumbered(ctx, s.db, s.locks, repo, func(b *kv.Batch, n int64) error {219 err := createNumbered(ctx, s.db, s.locks, repo, func(b *kv.Batch, n int64) error {
@@ -291,7 +287,6 @@ func encodePending(p domain.PendingIssue) ([]byte, error) {
291 }), nil287 }), nil
292 }288 }
293 289
294-//hexcheck:ignore suspicious-business-logic-in-adapter schema decoding, not a policy
295 func decodePending(repo domain.RepoName, id domain.PendingIssueID, item kv.Item) (domain.PendingIssue, error) {290 func decodePending(repo domain.RepoName, id domain.PendingIssueID, item kv.Item) (domain.PendingIssue, error) {
296 var pb storagepb.PendingIssue291 var pb storagepb.PendingIssue
297 if err := proto.Unmarshal(item.Value, &pb); err != nil {292 if err := proto.Unmarshal(item.Value, &pb); err != nil {
M internal/adapters/kvstore/mergerequests.go +3 -9
@@ -96,6 +96,8 @@ func indexKey(prefix string, repo domain.RepoName, branch string, n int64) []byt
9696 // when old is not known, as for a new request or a comment: the keys of mr are
9797 // then put or deleted, whatever they were). Deleting a key that is not there is
9898 // not an error.
99+//
100+//hexcheck:ignore suspicious-business-logic-in-adapter maintains source and target indexes to match the stored merge request open state and branch names
99101 func indexOpen(b *kv.Batch, old *domain.MergeRequest, mr domain.MergeRequest) {
100102 if old != nil && old.IsOpen() {
101103 if !mr.IsOpen() || old.Source != mr.Source {
@@ -204,8 +206,6 @@ func (m *MergeRequests) guardRepo(ctx context.Context, b *kv.Batch, repo domain.
204206 // rewrites it (see guardRepo), so that a repository deleted meanwhile is not
205207 // left with a stray merge request (a push recording pushed_at makes it retry). A missing repository gives
206208 // domain.ErrNotFound.
207-//
208-//hexcheck:ignore suspicious-business-logic-in-adapter takes the next number under a version precondition: a storage concern, no policy
209209 func (m *MergeRequests) Create(ctx context.Context, mr domain.MergeRequest) (domain.MergeRequest, error) {
210210 err := createNumbered(ctx, m.db, m.locks, mr.Repo, func(b *kv.Batch, n int64) error {
211211 mr.Number = n
@@ -451,7 +451,7 @@ func (m *MergeRequests) CountOpen(ctx context.Context, repo domain.RepoName) (in
451451
452452 // Update replaces the merge request with a version precondition.
453453 //
454-//hexcheck:ignore suspicious-business-logic-in-adapter port method name only
454+//hexcheck:ignore suspicious-business-logic-in-adapter replaces a versioned record and maintains open-request indexes and the derived mergeability record
455455 func (m *MergeRequests) Update(ctx context.Context, mr domain.MergeRequest) error {
456456 val, err := encodeMergeRequest(mr)
457457 if err != nil {
@@ -564,8 +564,6 @@ func (m *MergeRequests) GetRead(ctx context.Context, repo domain.RepoName, numbe
564564
565565 // SetRead writes mrread/<repo>/<number>. The repository must exist (see
566566 // guardRepo): domain.ErrNotFound otherwise.
567-//
568-//hexcheck:ignore suspicious-business-logic-in-adapter retries a write that lost a race with the repository key: a storage concern, no policy
569567 func (m *MergeRequests) SetRead(ctx context.Context, repo domain.RepoName, number int64, head domain.ObjectID) error {
570568 if head.IsZero() {
571569 return fmt.Errorf("%w: empty read position", domain.ErrInvalidObjectID)
@@ -748,7 +746,6 @@ func encodeMergeRequest(mr domain.MergeRequest) ([]byte, error) {
748746 }), nil
749747 }
750748
751-//hexcheck:ignore suspicious-business-logic-in-adapter schema decoding, not a policy
752749 func decodeMergeRequest(repo domain.RepoName, item kv.Item) (domain.MergeRequest, error) {
753750 var pb storagepb.MergeRequest
754751 if err := proto.Unmarshal(item.Value, &pb); err != nil {
@@ -815,7 +812,6 @@ func encodeComment(c domain.Comment) ([]byte, error) {
815812 }), nil
816813 }
817814
818-//hexcheck:ignore suspicious-business-logic-in-adapter schema decoding, not a policy
819815 func decodeComment(item kv.Item) (domain.Comment, error) {
820816 var pb storagepb.Comment
821817 if err := proto.Unmarshal(item.Value, &pb); err != nil {
@@ -931,8 +927,6 @@ func encodeSettings(s domain.RepoSettings) (*storagepb.RepoSettings, error) {
931927
932928 // decodeSettings returns the defaults for a repository stored before merge
933929 // requests existed, and for settings left with no mode that this build knows.
934-//
935-//hexcheck:ignore suspicious-business-logic-in-adapter schema decoding with the documented defaults
936930 func decodeSettings(pb *storagepb.RepoSettings) domain.RepoSettings {
937931 if pb == nil {
938932 return domain.DefaultRepoSettings()
@@ -96,6 +96,8 @@ func indexKey(prefix string, repo domain.RepoName, branch string, n int64) []byt
96 // when old is not known, as for a new request or a comment: the keys of mr are96 // when old is not known, as for a new request or a comment: the keys of mr are
97 // then put or deleted, whatever they were). Deleting a key that is not there is97 // then put or deleted, whatever they were). Deleting a key that is not there is
98 // not an error.98 // not an error.
99+//
100+//hexcheck:ignore suspicious-business-logic-in-adapter maintains source and target indexes to match the stored merge request open state and branch names
99 func indexOpen(b *kv.Batch, old *domain.MergeRequest, mr domain.MergeRequest) {101 func indexOpen(b *kv.Batch, old *domain.MergeRequest, mr domain.MergeRequest) {
100 if old != nil && old.IsOpen() {102 if old != nil && old.IsOpen() {
101 if !mr.IsOpen() || old.Source != mr.Source {103 if !mr.IsOpen() || old.Source != mr.Source {
@@ -204,8 +206,6 @@ func (m *MergeRequests) guardRepo(ctx context.Context, b *kv.Batch, repo domain.
204 // rewrites it (see guardRepo), so that a repository deleted meanwhile is not206 // rewrites it (see guardRepo), so that a repository deleted meanwhile is not
205 // left with a stray merge request (a push recording pushed_at makes it retry). A missing repository gives207 // left with a stray merge request (a push recording pushed_at makes it retry). A missing repository gives
206 // domain.ErrNotFound.208 // domain.ErrNotFound.
207-//
208-//hexcheck:ignore suspicious-business-logic-in-adapter takes the next number under a version precondition: a storage concern, no policy
209 func (m *MergeRequests) Create(ctx context.Context, mr domain.MergeRequest) (domain.MergeRequest, error) {209 func (m *MergeRequests) Create(ctx context.Context, mr domain.MergeRequest) (domain.MergeRequest, error) {
210 err := createNumbered(ctx, m.db, m.locks, mr.Repo, func(b *kv.Batch, n int64) error {210 err := createNumbered(ctx, m.db, m.locks, mr.Repo, func(b *kv.Batch, n int64) error {
211 mr.Number = n211 mr.Number = n
@@ -451,7 +451,7 @@ func (m *MergeRequests) CountOpen(ctx context.Context, repo domain.RepoName) (in
451 451
452 // Update replaces the merge request with a version precondition.452 // Update replaces the merge request with a version precondition.
453 //453 //
454-//hexcheck:ignore suspicious-business-logic-in-adapter port method name only454+//hexcheck:ignore suspicious-business-logic-in-adapter replaces a versioned record and maintains open-request indexes and the derived mergeability record
455 func (m *MergeRequests) Update(ctx context.Context, mr domain.MergeRequest) error {455 func (m *MergeRequests) Update(ctx context.Context, mr domain.MergeRequest) error {
456 val, err := encodeMergeRequest(mr)456 val, err := encodeMergeRequest(mr)
457 if err != nil {457 if err != nil {
@@ -564,8 +564,6 @@ func (m *MergeRequests) GetRead(ctx context.Context, repo domain.RepoName, numbe
564 564
565 // SetRead writes mrread/<repo>/<number>. The repository must exist (see565 // SetRead writes mrread/<repo>/<number>. The repository must exist (see
566 // guardRepo): domain.ErrNotFound otherwise.566 // guardRepo): domain.ErrNotFound otherwise.
567-//
568-//hexcheck:ignore suspicious-business-logic-in-adapter retries a write that lost a race with the repository key: a storage concern, no policy
569 func (m *MergeRequests) SetRead(ctx context.Context, repo domain.RepoName, number int64, head domain.ObjectID) error {567 func (m *MergeRequests) SetRead(ctx context.Context, repo domain.RepoName, number int64, head domain.ObjectID) error {
570 if head.IsZero() {568 if head.IsZero() {
571 return fmt.Errorf("%w: empty read position", domain.ErrInvalidObjectID)569 return fmt.Errorf("%w: empty read position", domain.ErrInvalidObjectID)
@@ -748,7 +746,6 @@ func encodeMergeRequest(mr domain.MergeRequest) ([]byte, error) {
748 }), nil746 }), nil
749 }747 }
750 748
751-//hexcheck:ignore suspicious-business-logic-in-adapter schema decoding, not a policy
752 func decodeMergeRequest(repo domain.RepoName, item kv.Item) (domain.MergeRequest, error) {749 func decodeMergeRequest(repo domain.RepoName, item kv.Item) (domain.MergeRequest, error) {
753 var pb storagepb.MergeRequest750 var pb storagepb.MergeRequest
754 if err := proto.Unmarshal(item.Value, &pb); err != nil {751 if err := proto.Unmarshal(item.Value, &pb); err != nil {
@@ -815,7 +812,6 @@ func encodeComment(c domain.Comment) ([]byte, error) {
815 }), nil812 }), nil
816 }813 }
817 814
818-//hexcheck:ignore suspicious-business-logic-in-adapter schema decoding, not a policy
819 func decodeComment(item kv.Item) (domain.Comment, error) {815 func decodeComment(item kv.Item) (domain.Comment, error) {
820 var pb storagepb.Comment816 var pb storagepb.Comment
821 if err := proto.Unmarshal(item.Value, &pb); err != nil {817 if err := proto.Unmarshal(item.Value, &pb); err != nil {
@@ -931,8 +927,6 @@ func encodeSettings(s domain.RepoSettings) (*storagepb.RepoSettings, error) {
931 927
932 // decodeSettings returns the defaults for a repository stored before merge928 // decodeSettings returns the defaults for a repository stored before merge
933 // requests existed, and for settings left with no mode that this build knows.929 // requests existed, and for settings left with no mode that this build knows.
934-//
935-//hexcheck:ignore suspicious-business-logic-in-adapter schema decoding with the documented defaults
936 func decodeSettings(pb *storagepb.RepoSettings) domain.RepoSettings {930 func decodeSettings(pb *storagepb.RepoSettings) domain.RepoSettings {
937 if pb == nil {931 if pb == nil {
938 return domain.DefaultRepoSettings()932 return domain.DefaultRepoSettings()
M internal/adapters/kvstore/passkeys.go +0 -2
@@ -134,8 +134,6 @@ func (s *Passkeys) Create(ctx context.Context, p domain.Passkey) error {
134134 }
135135
136136 // Update replaces a stored passkey, or returns domain.ErrNotFound.
137-//
138-//hexcheck:ignore suspicious-business-logic-in-adapter version-checked replace, not a policy
139137 func (s *Passkeys) Update(ctx context.Context, p domain.Passkey) error {
140138 val, err := encodePasskey(p)
141139 if err != nil {
@@ -134,8 +134,6 @@ func (s *Passkeys) Create(ctx context.Context, p domain.Passkey) error {
134 }134 }
135 135
136 // Update replaces a stored passkey, or returns domain.ErrNotFound.136 // Update replaces a stored passkey, or returns domain.ErrNotFound.
137-//
138-//hexcheck:ignore suspicious-business-logic-in-adapter version-checked replace, not a policy
139 func (s *Passkeys) Update(ctx context.Context, p domain.Passkey) error {137 func (s *Passkeys) Update(ctx context.Context, p domain.Passkey) error {
140 val, err := encodePasskey(p)138 val, err := encodePasskey(p)
141 if err != nil {139 if err != nil {
M internal/adapters/kvstore/repos.go +2 -10
@@ -67,8 +67,6 @@ func (s *Store) List(ctx context.Context) ([]domain.Repo, error) {
6767 const repoMutationTries = 8
6868
6969 // Mutate applies a retry-safe mutation under the version it read.
70-//
71-//hexcheck:ignore suspicious-business-logic-in-adapter optimistic concurrency is a storage concern
7270 func (s *Store) Mutate(ctx context.Context, name domain.RepoName, fn func(*domain.Repo) (bool, error)) (domain.Repo, error) {
7371 key := repoKey(name)
7472 for range repoMutationTries {
@@ -158,8 +156,6 @@ const deleteTries = 32
158156 // got in after the listing makes the batch fail and the keys are listed again,
159157 // and a write after the batch finds no repository: no key outlives the
160158 // repository to attach itself to a repository created again.
161-//
162-//hexcheck:ignore suspicious-business-logic-in-adapter chunked delete under a version precondition: a storage concern, no policy
163159 func (s *Store) Delete(ctx context.Context, name domain.RepoName) error {
164160 for range deleteTries {
165161 snap := s.db.Snapshot(ctx)
@@ -211,12 +207,8 @@ func encodeRepo(r domain.Repo) ([]byte, error) {
211207 if err != nil {
212208 return nil, err
213209 }
214- settings := r.Settings
215- if settings.IsZero() {
216- // Settings are always stored explicitly, so that a later change of
217- // the defaults never alters a stored repository.
218- settings = domain.DefaultRepoSettings()
219- }
210+ // Store defaults explicitly so future default changes do not alter stored repositories.
211+ settings := r.EffectiveSettings()
220212 pbSettings, err := encodeSettings(settings)
221213 if err != nil {
222214 return nil, err
@@ -67,8 +67,6 @@ func (s *Store) List(ctx context.Context) ([]domain.Repo, error) {
67 const repoMutationTries = 867 const repoMutationTries = 8
68 68
69 // Mutate applies a retry-safe mutation under the version it read.69 // Mutate applies a retry-safe mutation under the version it read.
70-//
71-//hexcheck:ignore suspicious-business-logic-in-adapter optimistic concurrency is a storage concern
72 func (s *Store) Mutate(ctx context.Context, name domain.RepoName, fn func(*domain.Repo) (bool, error)) (domain.Repo, error) {70 func (s *Store) Mutate(ctx context.Context, name domain.RepoName, fn func(*domain.Repo) (bool, error)) (domain.Repo, error) {
73 key := repoKey(name)71 key := repoKey(name)
74 for range repoMutationTries {72 for range repoMutationTries {
@@ -158,8 +156,6 @@ const deleteTries = 32
158 // got in after the listing makes the batch fail and the keys are listed again,156 // got in after the listing makes the batch fail and the keys are listed again,
159 // and a write after the batch finds no repository: no key outlives the157 // and a write after the batch finds no repository: no key outlives the
160 // repository to attach itself to a repository created again.158 // repository to attach itself to a repository created again.
161-//
162-//hexcheck:ignore suspicious-business-logic-in-adapter chunked delete under a version precondition: a storage concern, no policy
163 func (s *Store) Delete(ctx context.Context, name domain.RepoName) error {159 func (s *Store) Delete(ctx context.Context, name domain.RepoName) error {
164 for range deleteTries {160 for range deleteTries {
165 snap := s.db.Snapshot(ctx)161 snap := s.db.Snapshot(ctx)
@@ -211,12 +207,8 @@ func encodeRepo(r domain.Repo) ([]byte, error) {
211 if err != nil {207 if err != nil {
212 return nil, err208 return nil, err
213 }209 }
214- settings := r.Settings210+ // Store defaults explicitly so future default changes do not alter stored repositories.
215- if settings.IsZero() {211+ settings := r.EffectiveSettings()
216- // Settings are always stored explicitly, so that a later change of
217- // the defaults never alters a stored repository.
218- settings = domain.DefaultRepoSettings()
219- }
220 pbSettings, err := encodeSettings(settings)212 pbSettings, err := encodeSettings(settings)
221 if err != nil {213 if err != nil {
222 return nil, err214 return nil, err
M internal/adapters/kvstore/store.go +0 -2
@@ -186,8 +186,6 @@ var preIssueScopes = []domain.Scope{
186186 // for, so it gets each of them if missing (this is no escalation). Every other
187187 // token is left as it is. The batch requires each token to be at the version it
188188 // was read at.
189-//
190-//hexcheck:ignore suspicious-business-logic-in-adapter one-time storage upgrade of the stored tokens: a storage concern, no policy
191189 func (s *Store) upgradeTokens(ctx context.Context, b *kv.Batch) error {
192190 for key, item := range s.db.Snapshot(ctx).Prefix(ctx, []byte(tokenPrefix)) {
193191 pb := &storagepb.Token{}
@@ -186,8 +186,6 @@ var preIssueScopes = []domain.Scope{
186 // for, so it gets each of them if missing (this is no escalation). Every other186 // for, so it gets each of them if missing (this is no escalation). Every other
187 // token is left as it is. The batch requires each token to be at the version it187 // token is left as it is. The batch requires each token to be at the version it
188 // was read at.188 // was read at.
189-//
190-//hexcheck:ignore suspicious-business-logic-in-adapter one-time storage upgrade of the stored tokens: a storage concern, no policy
191 func (s *Store) upgradeTokens(ctx context.Context, b *kv.Batch) error {189 func (s *Store) upgradeTokens(ctx context.Context, b *kv.Batch) error {
192 for key, item := range s.db.Snapshot(ctx).Prefix(ctx, []byte(tokenPrefix)) {190 for key, item := range s.db.Snapshot(ctx).Prefix(ctx, []byte(tokenPrefix)) {
193 pb := &storagepb.Token{}191 pb := &storagepb.Token{}
M internal/adapters/kvstore/tokens.go +0 -6
@@ -153,8 +153,6 @@ func (s *Tokens) Revoke(ctx context.Context, id string) error {
153153
154154 // Touch sets last_used_at when the given time is later than the stored one,
155155 // so that racing writers never move it back.
156-//
157-//hexcheck:ignore suspicious-business-logic-in-adapter monotonic write guard, not a policy
158156 func (s *Tokens) Touch(ctx context.Context, id string, at time.Time) error {
159157 if !validTokenID(id) {
160158 return domain.ErrNotFound
@@ -194,8 +192,6 @@ func (s *Tokens) Touch(ctx context.Context, id string, at time.Time) error {
194192 // encodeToken refuses what the use case already refused (scopes, branch
195193 // prefixes): a malformed prefix must never reach the store, where it would
196194 // limit a token wrongly or not at all.
197-//
198-//hexcheck:ignore suspicious-business-logic-in-adapter schema encoding: it re-checks the syntax the use case enforced, it decides no policy
199195 func encodeToken(t domain.Token) ([]byte, error) {
200196 if !validTokenID(t.ID) {
201197 return nil, fmt.Errorf("%w: invalid id %q", domain.ErrInvalidToken, t.ID)
@@ -237,8 +233,6 @@ func encodeToken(t domain.Token) ([]byte, error) {
237233
238234 // decodeToken drops scopes it does not know (written by a newer ermite): a
239235 // token never gains rights by being read by an older build.
240-//
241-//hexcheck:ignore suspicious-business-logic-in-adapter schema decoding, not a policy
242236 func decodeToken(val []byte) (domain.Token, error) {
243237 var pb storagepb.Token
244238 if err := proto.Unmarshal(val, &pb); err != nil {
@@ -153,8 +153,6 @@ func (s *Tokens) Revoke(ctx context.Context, id string) error {
153 153
154 // Touch sets last_used_at when the given time is later than the stored one,154 // Touch sets last_used_at when the given time is later than the stored one,
155 // so that racing writers never move it back.155 // so that racing writers never move it back.
156-//
157-//hexcheck:ignore suspicious-business-logic-in-adapter monotonic write guard, not a policy
158 func (s *Tokens) Touch(ctx context.Context, id string, at time.Time) error {156 func (s *Tokens) Touch(ctx context.Context, id string, at time.Time) error {
159 if !validTokenID(id) {157 if !validTokenID(id) {
160 return domain.ErrNotFound158 return domain.ErrNotFound
@@ -194,8 +192,6 @@ func (s *Tokens) Touch(ctx context.Context, id string, at time.Time) error {
194 // encodeToken refuses what the use case already refused (scopes, branch192 // encodeToken refuses what the use case already refused (scopes, branch
195 // prefixes): a malformed prefix must never reach the store, where it would193 // prefixes): a malformed prefix must never reach the store, where it would
196 // limit a token wrongly or not at all.194 // limit a token wrongly or not at all.
197-//
198-//hexcheck:ignore suspicious-business-logic-in-adapter schema encoding: it re-checks the syntax the use case enforced, it decides no policy
199 func encodeToken(t domain.Token) ([]byte, error) {195 func encodeToken(t domain.Token) ([]byte, error) {
200 if !validTokenID(t.ID) {196 if !validTokenID(t.ID) {
201 return nil, fmt.Errorf("%w: invalid id %q", domain.ErrInvalidToken, t.ID)197 return nil, fmt.Errorf("%w: invalid id %q", domain.ErrInvalidToken, t.ID)
@@ -237,8 +233,6 @@ func encodeToken(t domain.Token) ([]byte, error) {
237 233
238 // decodeToken drops scopes it does not know (written by a newer ermite): a234 // decodeToken drops scopes it does not know (written by a newer ermite): a
239 // token never gains rights by being read by an older build.235 // token never gains rights by being read by an older build.
240-//
241-//hexcheck:ignore suspicious-business-logic-in-adapter schema decoding, not a policy
242 func decodeToken(val []byte) (domain.Token, error) {236 func decodeToken(val []byte) (domain.Token, error) {
243 var pb storagepb.Token237 var pb storagepb.Token
244 if err := proto.Unmarshal(val, &pb); err != nil {238 if err := proto.Unmarshal(val, &pb); err != nil {
M internal/adapters/rpc/api.go +2 -4
@@ -87,9 +87,7 @@ type Config struct {
8787 Secrets ports.SecretService
8888 Variables ports.VariableService
8989 Environments ports.EnvironmentService
90- // OIDCKeys rotates the key of the ID tokens of CI jobs; nil when the server
91- // issues none (GetVersion then does not list "ci.oidc"). Same rule as
92- // Sessions about typed nil pointers.
90+ // OIDCKeys manages ID-token signing keys, including when OIDC is disabled.
9391 OIDCKeys ports.OIDCKeyService
9492 // JobArtifacts and Artifacts serve JobArtifactService and ArtifactService,
9593 // and GetVersion lists "ci.artifacts", when both are set and Runner is.
@@ -138,7 +136,7 @@ func New(cfg Config) *API {
138136 if artifacts {
139137 features = append(features, FeatureCIArtifacts)
140138 }
141- if cfg.OIDCKeys != nil {
139+ if cfg.OIDCKeys != nil && cfg.OIDCKeys.Enabled() {
142140 features = append(features, FeatureCIOIDC)
143141 }
144142 if cfg.TrustedKeys != nil {
@@ -87,9 +87,7 @@ type Config struct {
87 Secrets ports.SecretService87 Secrets ports.SecretService
88 Variables ports.VariableService88 Variables ports.VariableService
89 Environments ports.EnvironmentService89 Environments ports.EnvironmentService
90- // OIDCKeys rotates the key of the ID tokens of CI jobs; nil when the server90+ // OIDCKeys manages ID-token signing keys, including when OIDC is disabled.
91- // issues none (GetVersion then does not list "ci.oidc"). Same rule as
92- // Sessions about typed nil pointers.
93 OIDCKeys ports.OIDCKeyService91 OIDCKeys ports.OIDCKeyService
94 // JobArtifacts and Artifacts serve JobArtifactService and ArtifactService,92 // JobArtifacts and Artifacts serve JobArtifactService and ArtifactService,
95 // and GetVersion lists "ci.artifacts", when both are set and Runner is.93 // and GetVersion lists "ci.artifacts", when both are set and Runner is.
@@ -138,7 +136,7 @@ func New(cfg Config) *API {
138 if artifacts {136 if artifacts {
139 features = append(features, FeatureCIArtifacts)137 features = append(features, FeatureCIArtifacts)
140 }138 }
141- if cfg.OIDCKeys != nil {139+ if cfg.OIDCKeys != nil && cfg.OIDCKeys.Enabled() {
142 features = append(features, FeatureCIOIDC)140 features = append(features, FeatureCIOIDC)
143 }141 }
144 if cfg.TrustedKeys != nil {142 if cfg.TrustedKeys != nil {
M internal/adapters/rpc/convert.go +1 -4
@@ -58,10 +58,7 @@ func visibilityFromPB(v pb.Visibility) (domain.Visibility, error) {
5858 }
5959
6060 func repoToPB(v ports.RepoView) *pb.Repo {
61- settings := v.Settings
62- if settings.IsZero() {
63- settings = domain.DefaultRepoSettings()
64- }
61+ settings := v.EffectiveSettings()
6562 return &pb.Repo{
6663 Name: v.Name.String(),
6764 Description: v.Description,
@@ -58,10 +58,7 @@ func visibilityFromPB(v pb.Visibility) (domain.Visibility, error) {
58 }58 }
59 59
60 func repoToPB(v ports.RepoView) *pb.Repo {60 func repoToPB(v ports.RepoView) *pb.Repo {
61- settings := v.Settings61+ settings := v.EffectiveSettings()
62- if settings.IsZero() {
63- settings = domain.DefaultRepoSettings()
64- }
65 return &pb.Repo{62 return &pb.Repo{
66 Name: v.Name.String(),63 Name: v.Name.String(),
67 Description: v.Description,64 Description: v.Description,
M internal/adapters/rpc/git.go +0 -1
@@ -187,7 +187,6 @@ func (s *gitService) GetCommit(ctx context.Context, req *connect.Request[pb.GetC
187187 return connect.NewResponse(commitDetailToPB(detail)), nil
188188 }
189189
190-//hexcheck:ignore suspicious-business-logic-in-adapter port method name only
191190 func (s *gitService) ResolveRevision(ctx context.Context, req *connect.Request[pb.ResolveRevisionRequest]) (*connect.Response[pb.ResolveRevisionResponse], error) {
192191 name, rev, err := target(req.Msg.GetRepo(), req.Msg.GetRevision())
193192 if err != nil {
@@ -187,7 +187,6 @@ func (s *gitService) GetCommit(ctx context.Context, req *connect.Request[pb.GetC
187 return connect.NewResponse(commitDetailToPB(detail)), nil187 return connect.NewResponse(commitDetailToPB(detail)), nil
188 }188 }
189 189
190-//hexcheck:ignore suspicious-business-logic-in-adapter port method name only
191 func (s *gitService) ResolveRevision(ctx context.Context, req *connect.Request[pb.ResolveRevisionRequest]) (*connect.Response[pb.ResolveRevisionResponse], error) {190 func (s *gitService) ResolveRevision(ctx context.Context, req *connect.Request[pb.ResolveRevisionRequest]) (*connect.Response[pb.ResolveRevisionResponse], error) {
192 name, rev, err := target(req.Msg.GetRepo(), req.Msg.GetRevision())191 name, rev, err := target(req.Msg.GetRepo(), req.Msg.GetRevision())
193 if err != nil {192 if err != nil {
M internal/adapters/rpc/mergerequest.go +0 -2
@@ -212,8 +212,6 @@ func commentID(s string) int64 {
212212 }
213213
214214 // headID parses a commit id of a request; the empty string is the zero id.
215-//
216-//hexcheck:ignore suspicious-business-logic-in-adapter parses a request field into the domain id and names the field in the error; no rule is decided here
217215 func headID(s string) (domain.ObjectID, error) {
218216 if s == "" {
219217 return domain.ObjectID{}, nil
@@ -212,8 +212,6 @@ func commentID(s string) int64 {
212 }212 }
213 213
214 // headID parses a commit id of a request; the empty string is the zero id.214 // headID parses a commit id of a request; the empty string is the zero id.
215-//
216-//hexcheck:ignore suspicious-business-logic-in-adapter parses a request field into the domain id and names the field in the error; no rule is decided here
217 func headID(s string) (domain.ObjectID, error) {215 func headID(s string) (domain.ObjectID, error) {
218 if s == "" {216 if s == "" {
219 return domain.ObjectID{}, nil217 return domain.ObjectID{}, nil
M internal/adapters/rpc/repo.go +0 -2
@@ -83,8 +83,6 @@ func (s *repoService) UpdateRepo(ctx context.Context, req *connect.Request[pb.Up
8383 // "settings.allowed_merge_modes" and "settings.failing_checks_block", and
8484 // nothing else; the optional issues field stands outside the mask, and alone
8585 // it is a valid update.
86-//
87-//hexcheck:ignore suspicious-business-logic-in-adapter maps the field mask of the contract to the optional fields of the use case; no rule is decided here
8886 func updateParams(msg *pb.UpdateRepoRequest) (ports.UpdateRepoParams, error) {
8987 var params ports.UpdateRepoParams
9088 paths := msg.GetUpdateMask().GetPaths()
@@ -83,8 +83,6 @@ func (s *repoService) UpdateRepo(ctx context.Context, req *connect.Request[pb.Up
83 // "settings.allowed_merge_modes" and "settings.failing_checks_block", and83 // "settings.allowed_merge_modes" and "settings.failing_checks_block", and
84 // nothing else; the optional issues field stands outside the mask, and alone84 // nothing else; the optional issues field stands outside the mask, and alone
85 // it is a valid update.85 // it is a valid update.
86-//
87-//hexcheck:ignore suspicious-business-logic-in-adapter maps the field mask of the contract to the optional fields of the use case; no rule is decided here
88 func updateParams(msg *pb.UpdateRepoRequest) (ports.UpdateRepoParams, error) {86 func updateParams(msg *pb.UpdateRepoRequest) (ports.UpdateRepoParams, error) {
89 var params ports.UpdateRepoParams87 var params ports.UpdateRepoParams
90 paths := msg.GetUpdateMask().GetPaths()88 paths := msg.GetUpdateMask().GetPaths()
M internal/adapters/rpc/server.go +5 -8
@@ -10,7 +10,6 @@ import (
1010 "connectrpc.com/connect"
1111
1212 pb "bnema.dev/ermite-api/gen/ermite"
13- "bnema.dev/ermite/internal/domain"
1413 "bnema.dev/ermite/internal/ports"
1514 )
1615
@@ -19,7 +18,7 @@ type serverService struct {
1918 version string
2019 features []string
2120 key ports.SigningKeyService
22- // oidc rotates the key of the ID tokens; nil when the server issues none.
21+ // oidc manages key rotation, including the disabled-provider response.
2322 oidc ports.OIDCKeyService
2423 }
2524
@@ -41,15 +40,13 @@ func (s *serverService) GetSigningKey(context.Context, *connect.Request[pb.GetSi
4140 }
4241
4342 // RotateOIDCKey replaces the key that signs the ID tokens of CI jobs. Only the
44-// owner may; failed_precondition when the server issues no ID tokens.
43+// owner may; failed_precondition when the server issues no ID tokens. An API
44+// built without an OIDC key service answers unimplemented.
4545 func (s *serverService) RotateOIDCKey(ctx context.Context, _ *connect.Request[pb.RotateOIDCKeyRequest]) (*connect.Response[pb.RotateOIDCKeyResponse], error) {
46- p := caller(ctx)
4746 if s.oidc == nil {
48- if !p.HasOwnerRights() {
49- return nil, connectError(ctx, domain.ErrForbidden)
50- }
51- return nil, connect.NewError(connect.CodeFailedPrecondition, errors.New("the server issues no OIDC ID tokens: it has no ERMITE_SECRETS_KEY"))
47+ return nil, connect.NewError(connect.CodeUnimplemented, errors.New("this API serves no OIDC key service"))
5248 }
49+ p := caller(ctx)
5350 r, err := s.oidc.RotateKey(ctx, p)
5451 if err != nil {
5552 return nil, connectError(ctx, err)
@@ -10,7 +10,6 @@ import (
10 "connectrpc.com/connect"10 "connectrpc.com/connect"
11 11
12 pb "bnema.dev/ermite-api/gen/ermite"12 pb "bnema.dev/ermite-api/gen/ermite"
13- "bnema.dev/ermite/internal/domain"
14 "bnema.dev/ermite/internal/ports"13 "bnema.dev/ermite/internal/ports"
15 )14 )
16 15
@@ -19,7 +18,7 @@ type serverService struct {
19 version string18 version string
20 features []string19 features []string
21 key ports.SigningKeyService20 key ports.SigningKeyService
22- // oidc rotates the key of the ID tokens; nil when the server issues none.21+ // oidc manages key rotation, including the disabled-provider response.
23 oidc ports.OIDCKeyService22 oidc ports.OIDCKeyService
24 }23 }
25 24
@@ -41,15 +40,13 @@ func (s *serverService) GetSigningKey(context.Context, *connect.Request[pb.GetSi
41 }40 }
42 41
43 // RotateOIDCKey replaces the key that signs the ID tokens of CI jobs. Only the42 // RotateOIDCKey replaces the key that signs the ID tokens of CI jobs. Only the
44-// owner may; failed_precondition when the server issues no ID tokens.43+// owner may; failed_precondition when the server issues no ID tokens. An API
44+// built without an OIDC key service answers unimplemented.
45 func (s *serverService) RotateOIDCKey(ctx context.Context, _ *connect.Request[pb.RotateOIDCKeyRequest]) (*connect.Response[pb.RotateOIDCKeyResponse], error) {45 func (s *serverService) RotateOIDCKey(ctx context.Context, _ *connect.Request[pb.RotateOIDCKeyRequest]) (*connect.Response[pb.RotateOIDCKeyResponse], error) {
46- p := caller(ctx)
47 if s.oidc == nil {46 if s.oidc == nil {
48- if !p.HasOwnerRights() {47+ return nil, connect.NewError(connect.CodeUnimplemented, errors.New("this API serves no OIDC key service"))
49- return nil, connectError(ctx, domain.ErrForbidden)
50- }
51- return nil, connect.NewError(connect.CodeFailedPrecondition, errors.New("the server issues no OIDC ID tokens: it has no ERMITE_SECRETS_KEY"))
52 }48 }
49+ p := caller(ctx)
53 r, err := s.oidc.RotateKey(ctx, p)50 r, err := s.oidc.RotateKey(ctx, p)
54 if err != nil {51 if err != nil {
55 return nil, connectError(ctx, err)52 return nil, connectError(ctx, err)
M internal/adapters/rpc/server_test.go +11 -0
@@ -49,6 +49,17 @@ func TestServerServiceGetSigningKey(t *testing.T) {
4949 assert.Equal(t, "ermite@example.com ssh-ed25519 AAAA", resp.Msg.GetAllowedSigners())
5050 }
5151
52+// An API built without an OIDC key service answers unimplemented instead of
53+// panicking, for the owner and for anyone else.
54+func TestRotateOIDCKeyWithoutService(t *testing.T) {
55+ t.Parallel()
56+ for _, p := range []domain.Principal{domain.Owner(), {}} {
57+ ctx := domain.ContextWithPrincipal(context.Background(), p)
58+ _, err := (&serverService{}).RotateOIDCKey(ctx, connect.NewRequest(&pb.RotateOIDCKeyRequest{}))
59+ assert.Equal(t, connect.CodeUnimplemented, connect.CodeOf(err))
60+ }
61+}
62+
5263 // GetVersion lists "passkeys" only when the API is built with passkeys.
5364 func TestAPIListsPasskeysFeatureWhenAvailable(t *testing.T) {
5465 t.Parallel()
@@ -49,6 +49,17 @@ func TestServerServiceGetSigningKey(t *testing.T) {
49 assert.Equal(t, "ermite@example.com ssh-ed25519 AAAA", resp.Msg.GetAllowedSigners())49 assert.Equal(t, "ermite@example.com ssh-ed25519 AAAA", resp.Msg.GetAllowedSigners())
50 }50 }
51 51
52+// An API built without an OIDC key service answers unimplemented instead of
53+// panicking, for the owner and for anyone else.
54+func TestRotateOIDCKeyWithoutService(t *testing.T) {
55+ t.Parallel()
56+ for _, p := range []domain.Principal{domain.Owner(), {}} {
57+ ctx := domain.ContextWithPrincipal(context.Background(), p)
58+ _, err := (&serverService{}).RotateOIDCKey(ctx, connect.NewRequest(&pb.RotateOIDCKeyRequest{}))
59+ assert.Equal(t, connect.CodeUnimplemented, connect.CodeOf(err))
60+ }
61+}
62+
52 // GetVersion lists "passkeys" only when the API is built with passkeys.63 // GetVersion lists "passkeys" only when the API is built with passkeys.
53 func TestAPIListsPasskeysFeatureWhenAvailable(t *testing.T) {64 func TestAPIListsPasskeysFeatureWhenAvailable(t *testing.T) {
54 t.Parallel()65 t.Parallel()
M internal/adapters/sigverify/gpg.go +0 -2
@@ -41,8 +41,6 @@ var _ ports.SignatureVerifier = GPG{}
4141
4242 // Verify implements ports.SignatureVerifier. signedAt is ignored: see the time
4343 // policy of GPG.
44-//
45-//hexcheck:ignore suspicious-business-logic-in-adapter this is the OpenPGP protocol check the port delegates to the adapter; the trust and email rules stay in domain
4644 func (GPG) Verify(payload, signature []byte, committerEmail string, _ int64, keys []domain.TrustedKey) (v domain.CommitVerification) {
4745 v = domain.CommitVerification{Format: domain.FormatGPG, Status: domain.StatusBadSignature}
4846 defer func() { // a parser bug must not take the server down
@@ -41,8 +41,6 @@ var _ ports.SignatureVerifier = GPG{}
41 41
42 // Verify implements ports.SignatureVerifier. signedAt is ignored: see the time42 // Verify implements ports.SignatureVerifier. signedAt is ignored: see the time
43 // policy of GPG.43 // policy of GPG.
44-//
45-//hexcheck:ignore suspicious-business-logic-in-adapter this is the OpenPGP protocol check the port delegates to the adapter; the trust and email rules stay in domain
46 func (GPG) Verify(payload, signature []byte, committerEmail string, _ int64, keys []domain.TrustedKey) (v domain.CommitVerification) {44 func (GPG) Verify(payload, signature []byte, committerEmail string, _ int64, keys []domain.TrustedKey) (v domain.CommitVerification) {
47 v = domain.CommitVerification{Format: domain.FormatGPG, Status: domain.StatusBadSignature}45 v = domain.CommitVerification{Format: domain.FormatGPG, Status: domain.StatusBadSignature}
48 defer func() { // a parser bug must not take the server down46 defer func() { // a parser bug must not take the server down
M internal/adapters/sigverify/ssh.go +0 -2
@@ -49,8 +49,6 @@ type sshSigBlob struct {
4949 }
5050
5151 // Verify implements ports.SignatureVerifier.
52-//
53-//hexcheck:ignore suspicious-business-logic-in-adapter this is the SSHSIG protocol check the port delegates to the adapter; the trust and email rules stay in domain
5452 func (SSH) Verify(payload, signature []byte, committerEmail string, signedAt int64, keys []domain.TrustedKey) (v domain.CommitVerification) {
5553 v = domain.CommitVerification{Format: domain.FormatSSH, Status: domain.StatusBadSignature}
5654 defer func() { // a parser bug must not take the server down
@@ -49,8 +49,6 @@ type sshSigBlob struct {
49 }49 }
50 50
51 // Verify implements ports.SignatureVerifier.51 // Verify implements ports.SignatureVerifier.
52-//
53-//hexcheck:ignore suspicious-business-logic-in-adapter this is the SSHSIG protocol check the port delegates to the adapter; the trust and email rules stay in domain
54 func (SSH) Verify(payload, signature []byte, committerEmail string, signedAt int64, keys []domain.TrustedKey) (v domain.CommitVerification) {52 func (SSH) Verify(payload, signature []byte, committerEmail string, signedAt int64, keys []domain.TrustedKey) (v domain.CommitVerification) {
55 v = domain.CommitVerification{Format: domain.FormatSSH, Status: domain.StatusBadSignature}53 v = domain.CommitVerification{Format: domain.FormatSSH, Status: domain.StatusBadSignature}
56 defer func() { // a parser bug must not take the server down54 defer func() { // a parser bug must not take the server down
M internal/adapters/sigverify/verifier.go +0 -2
@@ -42,8 +42,6 @@ func (Verifier) Verify(payload, signature []byte, committerEmail string, signedA
4242 // trusted completes the verification of a signature whose cryptography is
4343 // valid and whose key is trusted: verified when the key may sign for the
4444 // committer email, email_mismatch otherwise.
45-//
46-//hexcheck:ignore suspicious-business-logic-in-adapter the email rule is domain.TrustedKey.TrustsEmail; this only maps it to the port's status
4745 func trusted(v domain.CommitVerification, key domain.TrustedKey, committerEmail string) domain.CommitVerification {
4846 v.KeyID, v.KeyTitle = key.ID, key.Title
4947 if key.TrustsEmail(committerEmail) {
@@ -42,8 +42,6 @@ func (Verifier) Verify(payload, signature []byte, committerEmail string, signedA
42 // trusted completes the verification of a signature whose cryptography is42 // trusted completes the verification of a signature whose cryptography is
43 // valid and whose key is trusted: verified when the key may sign for the43 // valid and whose key is trusted: verified when the key may sign for the
44 // committer email, email_mismatch otherwise.44 // committer email, email_mismatch otherwise.
45-//
46-//hexcheck:ignore suspicious-business-logic-in-adapter the email rule is domain.TrustedKey.TrustsEmail; this only maps it to the port's status
47 func trusted(v domain.CommitVerification, key domain.TrustedKey, committerEmail string) domain.CommitVerification {45 func trusted(v domain.CommitVerification, key domain.TrustedKey, committerEmail string) domain.CommitVerification {
48 v.KeyID, v.KeyTitle = key.ID, key.Title46 v.KeyID, v.KeyTitle = key.ID, key.Title
49 if key.TrustsEmail(committerEmail) {47 if key.TrustsEmail(committerEmail) {
M internal/adapters/web/actions_artifacts.go +0 -4
@@ -53,8 +53,6 @@ type ArtifactsView struct {
5353 // runArtifacts is the artifacts section of a run, nil when there is nothing to
5454 // show: no artifacts on this server, a visitor who cannot list them, no
5555 // artifact, or a failed read, which is logged; the run page never fails for it.
56-//
57-//hexcheck:ignore suspicious-business-logic-in-adapter builds the artifacts section from the use case results; which errors hide it is presentation, no rule is decided here
5856 func (s *Server) runArtifacts(ctx context.Context, repo domain.RepoName, p RunPage) *ArtifactsView {
5957 if s.cfg.Artifacts == nil || !SignedIn(ctx) {
6058 return nil
@@ -112,8 +110,6 @@ func parseArtifactPath(r *http.Request) (int64, error) {
112110 // is never served: a zip is application/zip, anything else
113111 // application/octet-stream, and the answer is sandboxed and not sniffed, so
114112 // that nothing uploaded by a job can run in the origin of the forge.
115-//
116-//hexcheck:ignore suspicious-business-logic-in-adapter streams what the use case sends; the headers are about the response, no rule is decided here
117113 func (s *Server) artifactDownload(w http.ResponseWriter, r *http.Request) {
118114 ctx, cancel := context.WithTimeout(r.Context(), artifactTimeout)
119115 defer cancel()
@@ -53,8 +53,6 @@ type ArtifactsView struct {
53 // runArtifacts is the artifacts section of a run, nil when there is nothing to53 // runArtifacts is the artifacts section of a run, nil when there is nothing to
54 // show: no artifacts on this server, a visitor who cannot list them, no54 // show: no artifacts on this server, a visitor who cannot list them, no
55 // artifact, or a failed read, which is logged; the run page never fails for it.55 // artifact, or a failed read, which is logged; the run page never fails for it.
56-//
57-//hexcheck:ignore suspicious-business-logic-in-adapter builds the artifacts section from the use case results; which errors hide it is presentation, no rule is decided here
58 func (s *Server) runArtifacts(ctx context.Context, repo domain.RepoName, p RunPage) *ArtifactsView {56 func (s *Server) runArtifacts(ctx context.Context, repo domain.RepoName, p RunPage) *ArtifactsView {
59 if s.cfg.Artifacts == nil || !SignedIn(ctx) {57 if s.cfg.Artifacts == nil || !SignedIn(ctx) {
60 return nil58 return nil
@@ -112,8 +110,6 @@ func parseArtifactPath(r *http.Request) (int64, error) {
112 // is never served: a zip is application/zip, anything else110 // is never served: a zip is application/zip, anything else
113 // application/octet-stream, and the answer is sandboxed and not sniffed, so111 // application/octet-stream, and the answer is sandboxed and not sniffed, so
114 // that nothing uploaded by a job can run in the origin of the forge.112 // that nothing uploaded by a job can run in the origin of the forge.
115-//
116-//hexcheck:ignore suspicious-business-logic-in-adapter streams what the use case sends; the headers are about the response, no rule is decided here
117 func (s *Server) artifactDownload(w http.ResponseWriter, r *http.Request) {113 func (s *Server) artifactDownload(w http.ResponseWriter, r *http.Request) {
118 ctx, cancel := context.WithTimeout(r.Context(), artifactTimeout)114 ctx, cancel := context.WithTimeout(r.Context(), artifactTimeout)
119 defer cancel()115 defer cancel()
M internal/adapters/web/browse.go +0 -2
@@ -71,8 +71,6 @@ func (s *Server) openBlobAt(ctx context.Context, repo domain.RepoName, rev, path
7171 }
7272
7373 // resolveRevision resolves rev to the commit it designates.
74-//
75-//hexcheck:ignore suspicious-business-logic-in-adapter parses the URL text and calls the use case; no rule is decided here
7674 func (s *Server) resolveRevision(ctx context.Context, repo domain.RepoName, rev string) (domain.ResolvedRevision, error) {
7775 r, err := domain.ParseRevision(rev)
7876 if err != nil {
@@ -71,8 +71,6 @@ func (s *Server) openBlobAt(ctx context.Context, repo domain.RepoName, rev, path
71 }71 }
72 72
73 // resolveRevision resolves rev to the commit it designates.73 // resolveRevision resolves rev to the commit it designates.
74-//
75-//hexcheck:ignore suspicious-business-logic-in-adapter parses the URL text and calls the use case; no rule is decided here
76 func (s *Server) resolveRevision(ctx context.Context, repo domain.RepoName, rev string) (domain.ResolvedRevision, error) {74 func (s *Server) resolveRevision(ctx context.Context, repo domain.RepoName, rev string) (domain.ResolvedRevision, error) {
77 r, err := domain.ParseRevision(rev)75 r, err := domain.ParseRevision(rev)
78 if err != nil {76 if err != nil {
M internal/adapters/web/checks.go +2 -0
@@ -54,6 +54,8 @@ func (s *Server) checksFor(ctx context.Context, repo domain.RepoName, sha string
5454 }
5555
5656 // checksSummary reduces checks to the mark a list shows next to a commit.
57+//
58+//hexcheck:ignore suspicious-business-logic-in-adapter renders domain check outcomes as a commit-list mark, passed count, label and actions link
5759 func checksSummary(repo domain.RepoName, sha string, checks []ports.CheckView) ChecksSummary {
5860 if len(checks) == 0 {
5961 return ChecksSummary{}
@@ -54,6 +54,8 @@ func (s *Server) checksFor(ctx context.Context, repo domain.RepoName, sha string
54 }54 }
55 55
56 // checksSummary reduces checks to the mark a list shows next to a commit.56 // checksSummary reduces checks to the mark a list shows next to a commit.
57+//
58+//hexcheck:ignore suspicious-business-logic-in-adapter renders domain check outcomes as a commit-list mark, passed count, label and actions link
57 func checksSummary(repo domain.RepoName, sha string, checks []ports.CheckView) ChecksSummary {59 func checksSummary(repo domain.RepoName, sha string, checks []ports.CheckView) ChecksSummary {
58 if len(checks) == 0 {60 if len(checks) == 0 {
59 return ChecksSummary{}61 return ChecksSummary{}
M internal/adapters/web/commit.go +0 -2
@@ -96,8 +96,6 @@ func diffLines(files []DiffFile) (lines int, invisible bool) {
9696
9797 // commit shows a commit: its message, its totals, the changed files and the
9898 // diff against the first parent. ?view=split selects the split layout.
99-//
100-//hexcheck:ignore suspicious-business-logic-in-adapter builds the commit page from the use case results; no rule is decided here
10199 func (s *Server) commit(ctx context.Context, r *http.Request) (templ.Component, int, error) {
102100 sha := r.PathValue("sha")
103101 if !isCommitID(sha) {
@@ -96,8 +96,6 @@ func diffLines(files []DiffFile) (lines int, invisible bool) {
96 96
97 // commit shows a commit: its message, its totals, the changed files and the97 // commit shows a commit: its message, its totals, the changed files and the
98 // diff against the first parent. ?view=split selects the split layout.98 // diff against the first parent. ?view=split selects the split layout.
99-//
100-//hexcheck:ignore suspicious-business-logic-in-adapter builds the commit page from the use case results; no rule is decided here
101 func (s *Server) commit(ctx context.Context, r *http.Request) (templ.Component, int, error) {99 func (s *Server) commit(ctx context.Context, r *http.Request) (templ.Component, int, error) {
102 sha := r.PathValue("sha")100 sha := r.PathValue("sha")
103 if !isCommitID(sha) {101 if !isCommitID(sha) {
M internal/adapters/web/compare.go +3 -55
@@ -5,7 +5,6 @@ package web
55 import (
66 "context"
77 "net/http"
8- "slices"
98
109 "github.com/a-h/templ"
1110
@@ -13,10 +12,6 @@ import (
1312 "bnema.dev/ermite/internal/ports"
1413 )
1514
16-// compareDepth bounds the commits read on each side to find where a branch
17-// left its target.
18-const compareDepth = maxComparePages * pageSize
19-
2015 // compare shows a branch against the default branch of the repository: the
2116 // commits it is ahead by and, for the owner, the form that opens a merge
2217 // request.
@@ -27,8 +22,6 @@ const compareDepth = maxComparePages * pageSize
2722 // own diff is then the whole change). Otherwise it lists the commits, which
2823 // link to their own diffs, and says the whole diff comes with the merge
2924 // request.
30-//
31-//hexcheck:ignore suspicious-business-logic-in-adapter builds the compare page from the use case results; the merge rules are the use case's
3225 func (s *Server) compare(ctx context.Context, r *http.Request) (templ.Component, int, error) {
3326 d, err := s.loadRepo(ctx, r.PathValue("repo"))
3427 if err != nil {
@@ -56,7 +49,9 @@ func (s *Server) compare(ctx context.Context, r *http.Request) (templ.Component,
5649 p.Note = "this is the default branch: there is nothing to compare it with."
5750 p.AheadText = "0 commits"
5851 } else {
59- ahead, exact, behind, err = s.commitsAhead(ctx, d.repo.Name, src.CommitID.String(), tgt.CommitID.String())
52+ var comparison ports.CompareResult
53+ comparison, err = s.cfg.MergeRequests.Compare(ctx, principal(ctx), d.repo.Name, tgt.CommitID, src.CommitID)
54+ ahead, exact, behind = comparison.Ahead, comparison.Exact, comparison.Behind
6055 if err != nil {
6156 return nil, 0, err
6257 }
@@ -139,53 +134,6 @@ func (s *Server) compare(ctx context.Context, r *http.Request) (templ.Component,
139134 return Compare(l, p), http.StatusOK, nil
140135 }
141136
142-// commitsAhead lists the commits of the source that the target lacks, newest
143-// first. exact is false when the walk gave up before finding where the source
144-// meets the target. behind is true when the target has commits the source
145-// lacks (the tip of the target is not an ancestor of the source).
146-//
147-//hexcheck:ignore suspicious-business-logic-in-adapter walks the history pages the use case returns to find where the branch left its target; the page needs it, the forge decides nothing
148-func (s *Server) commitsAhead(ctx context.Context, repo domain.RepoName, srcTip, tgtTip string) (ahead []domain.Commit, exact, behind bool, err error) {
149- if srcTip == tgtTip {
150- return nil, true, false, nil
151- }
152- onTarget := map[string]bool{}
153- tokenT := ""
154- for len(onTarget) < compareDepth {
155- res, err := s.commitList(ctx, repo, tgtTip, "", pageSize, tokenT)
156- if err != nil {
157- return nil, false, false, err
158- }
159- for _, c := range res.Commits {
160- onTarget[c.ID.String()] = true
161- }
162- if tokenT = res.NextPageToken; tokenT == "" {
163- break
164- }
165- }
166- token := ""
167- for n := 0; n < compareDepth; {
168- res, err := s.commitList(ctx, repo, srcTip, "", pageSize, token)
169- if err != nil {
170- return nil, false, false, err
171- }
172- for _, c := range res.Commits {
173- n++
174- if onTarget[c.ID.String()] {
175- // The first commit of the source that the target has: where the
176- // branch left it. The target moved on when this is not its tip.
177- return ahead, true, c.ID.String() != tgtTip, nil
178- }
179- ahead = append(ahead, c)
180- }
181- if token = res.NextPageToken; token == "" {
182- // The whole history of the source is ahead: no common history.
183- return ahead, true, true, nil
184- }
185- }
186- return ahead, false, !slices.ContainsFunc(ahead, func(c domain.Commit) bool { return c.ID.String() == tgtTip }), nil
187-}
188-
189137 // openRequestFor finds the open merge request with this source and target.
190138 func (s *Server) openRequestFor(ctx context.Context, repo domain.RepoName, source, target string) (*domain.MergeRequest, error) {
191139 token := ""
@@ -5,7 +5,6 @@ package web
5 import (5 import (
6 "context"6 "context"
7 "net/http"7 "net/http"
8- "slices"
9 8
10 "github.com/a-h/templ"9 "github.com/a-h/templ"
11 10
@@ -13,10 +12,6 @@ import (
13 "bnema.dev/ermite/internal/ports"12 "bnema.dev/ermite/internal/ports"
14 )13 )
15 14
16-// compareDepth bounds the commits read on each side to find where a branch
17-// left its target.
18-const compareDepth = maxComparePages * pageSize
19-
20 // compare shows a branch against the default branch of the repository: the15 // compare shows a branch against the default branch of the repository: the
21 // commits it is ahead by and, for the owner, the form that opens a merge16 // commits it is ahead by and, for the owner, the form that opens a merge
22 // request.17 // request.
@@ -27,8 +22,6 @@ const compareDepth = maxComparePages * pageSize
27 // own diff is then the whole change). Otherwise it lists the commits, which22 // own diff is then the whole change). Otherwise it lists the commits, which
28 // link to their own diffs, and says the whole diff comes with the merge23 // link to their own diffs, and says the whole diff comes with the merge
29 // request.24 // request.
30-//
31-//hexcheck:ignore suspicious-business-logic-in-adapter builds the compare page from the use case results; the merge rules are the use case's
32 func (s *Server) compare(ctx context.Context, r *http.Request) (templ.Component, int, error) {25 func (s *Server) compare(ctx context.Context, r *http.Request) (templ.Component, int, error) {
33 d, err := s.loadRepo(ctx, r.PathValue("repo"))26 d, err := s.loadRepo(ctx, r.PathValue("repo"))
34 if err != nil {27 if err != nil {
@@ -56,7 +49,9 @@ func (s *Server) compare(ctx context.Context, r *http.Request) (templ.Component,
56 p.Note = "this is the default branch: there is nothing to compare it with."49 p.Note = "this is the default branch: there is nothing to compare it with."
57 p.AheadText = "0 commits"50 p.AheadText = "0 commits"
58 } else {51 } else {
59- ahead, exact, behind, err = s.commitsAhead(ctx, d.repo.Name, src.CommitID.String(), tgt.CommitID.String())52+ var comparison ports.CompareResult
53+ comparison, err = s.cfg.MergeRequests.Compare(ctx, principal(ctx), d.repo.Name, tgt.CommitID, src.CommitID)
54+ ahead, exact, behind = comparison.Ahead, comparison.Exact, comparison.Behind
60 if err != nil {55 if err != nil {
61 return nil, 0, err56 return nil, 0, err
62 }57 }
@@ -139,53 +134,6 @@ func (s *Server) compare(ctx context.Context, r *http.Request) (templ.Component,
139 return Compare(l, p), http.StatusOK, nil134 return Compare(l, p), http.StatusOK, nil
140 }135 }
141 136
142-// commitsAhead lists the commits of the source that the target lacks, newest
143-// first. exact is false when the walk gave up before finding where the source
144-// meets the target. behind is true when the target has commits the source
145-// lacks (the tip of the target is not an ancestor of the source).
146-//
147-//hexcheck:ignore suspicious-business-logic-in-adapter walks the history pages the use case returns to find where the branch left its target; the page needs it, the forge decides nothing
148-func (s *Server) commitsAhead(ctx context.Context, repo domain.RepoName, srcTip, tgtTip string) (ahead []domain.Commit, exact, behind bool, err error) {
149- if srcTip == tgtTip {
150- return nil, true, false, nil
151- }
152- onTarget := map[string]bool{}
153- tokenT := ""
154- for len(onTarget) < compareDepth {
155- res, err := s.commitList(ctx, repo, tgtTip, "", pageSize, tokenT)
156- if err != nil {
157- return nil, false, false, err
158- }
159- for _, c := range res.Commits {
160- onTarget[c.ID.String()] = true
161- }
162- if tokenT = res.NextPageToken; tokenT == "" {
163- break
164- }
165- }
166- token := ""
167- for n := 0; n < compareDepth; {
168- res, err := s.commitList(ctx, repo, srcTip, "", pageSize, token)
169- if err != nil {
170- return nil, false, false, err
171- }
172- for _, c := range res.Commits {
173- n++
174- if onTarget[c.ID.String()] {
175- // The first commit of the source that the target has: where the
176- // branch left it. The target moved on when this is not its tip.
177- return ahead, true, c.ID.String() != tgtTip, nil
178- }
179- ahead = append(ahead, c)
180- }
181- if token = res.NextPageToken; token == "" {
182- // The whole history of the source is ahead: no common history.
183- return ahead, true, true, nil
184- }
185- }
186- return ahead, false, !slices.ContainsFunc(ahead, func(c domain.Commit) bool { return c.ID.String() == tgtTip }), nil
187-}
188-
189 // openRequestFor finds the open merge request with this source and target.137 // openRequestFor finds the open merge request with this source and target.
190 func (s *Server) openRequestFor(ctx context.Context, repo domain.RepoName, source, target string) (*domain.MergeRequest, error) {138 func (s *Server) openRequestFor(ctx context.Context, repo domain.RepoName, source, target string) (*domain.MergeRequest, error) {
191 token := ""139 token := ""
M internal/adapters/web/context.go +1 -1
@@ -30,7 +30,7 @@ type ContextRows struct {
3030 // the numbers of the first hidden line on each side, which move together (the
3131 // lines are unchanged); n the number of lines, 0 for the rest of the file.
3232 //
33-//hexcheck:ignore suspicious-business-logic-in-adapter cuts a file the use case opened into rows of the diff; no rule is decided here
33+//hexcheck:ignore suspicious-business-logic-in-adapter validates diff-context HTTP parameters, rejects binary or oversized files, and caps rendered context rows
3434 func (s *Server) diffContext(ctx context.Context, r *http.Request) (templ.Component, int, error) {
3535 sha := r.PathValue("sha")
3636 if !isFullID(sha) {
@@ -30,7 +30,7 @@ type ContextRows struct {
30 // the numbers of the first hidden line on each side, which move together (the30 // the numbers of the first hidden line on each side, which move together (the
31 // lines are unchanged); n the number of lines, 0 for the rest of the file.31 // lines are unchanged); n the number of lines, 0 for the rest of the file.
32 //32 //
33-//hexcheck:ignore suspicious-business-logic-in-adapter cuts a file the use case opened into rows of the diff; no rule is decided here33+//hexcheck:ignore suspicious-business-logic-in-adapter validates diff-context HTTP parameters, rejects binary or oversized files, and caps rendered context rows
34 func (s *Server) diffContext(ctx context.Context, r *http.Request) (templ.Component, int, error) {34 func (s *Server) diffContext(ctx context.Context, r *http.Request) (templ.Component, int, error) {
35 sha := r.PathValue("sha")35 sha := r.PathValue("sha")
36 if !isFullID(sha) {36 if !isFullID(sha) {
M internal/adapters/web/diff_highlight.go +0 -2
M internal/adapters/web/events.go +3 -1
M internal/adapters/web/events_test.go +13 -0
M internal/adapters/web/fragments.go +43 -32
M internal/adapters/web/issues.go +3 -5
M internal/adapters/web/issues_form.go +3 -1
M internal/adapters/web/issues_post.go +0 -1
M internal/adapters/web/mr.go +96 -88
M internal/adapters/web/mr_files.go +0 -2
M internal/adapters/web/mr_post.go +0 -2
M internal/adapters/web/mr_test.go +4 -4
M internal/adapters/web/paths.go +0 -2
M internal/adapters/web/raw.go +0 -2
M internal/adapters/web/refs.go +0 -4
M internal/adapters/web/repo.go +3 -7
M internal/adapters/web/tree.go +0 -2
M internal/adapters/webauthn/webauthn.go +0 -2
M internal/app/forge.go +3 -8
A internal/domain/budget.go +85 -0
A internal/domain/budget_test.go +141 -0
A internal/domain/ci_workflow_guard.go +123 -0
A internal/domain/ci_workflow_guard_test.go +120 -0
M internal/domain/mergerequest.go +9 -0
M internal/domain/repo.go +8 -0
M internal/domain/repo_test.go +34 -0
A internal/domain/revision.go +54 -0
A internal/domain/revision_test.go +58 -0
M internal/ports/ci.go +41 -15
M internal/ports/dto.go +28 -0
M internal/ports/mocks/MergeRequestService.go +168 -0
M internal/ports/mocks/OIDCKeyService.go +44 -0
A internal/ports/mocks/PushTreeReader.go +223 -0
D internal/ports/mocks/WorkflowGuard.go +0 -127
M internal/ports/oidc.go +2 -0
M internal/ports/services.go +4 -0
M internal/usecase/ci.go +1 -1
A internal/usecase/ci_creation_policy_test.go +48 -0
M internal/usecase/ci_oidc.go +3 -9
M internal/usecase/ci_pull.go +9 -1
M internal/usecase/ci_runner.go +1 -1
M internal/usecase/events.go +2 -2
M internal/usecase/inbox.go +1 -1
M internal/usecase/issue_moderation.go +1 -1
M internal/usecase/issue_submissions.go +1 -1
M internal/usecase/issues.go +1 -1
M internal/usecase/mergerequests.go +57 -8
M internal/usecase/mergerequests_test.go +144 -0
M internal/usecase/oidc.go +27 -3
M internal/usecase/oidc_test.go +44 -1
M internal/usecase/pushes.go +30 -14
M internal/usecase/pushes_test.go +79 -17
M internal/usecase/repos_write.go +1 -3

Comments1 open / 1

bnema outdated

Scope extended: the business rules flagged in adapters now live in domain/usecase.

  • domain: Revision.Candidates, CommitObjectBudget/ValidateCommitObjects/DiffBudget, WorkflowChangeOf (port WorkflowGuard -> PushTreeReader), Repo.EffectiveSettings, IssuesEnabled/ShowsIssueSubmissionReceipt/AcceptsAnonymousIssues.
  • usecase: CI.createRun refuses disabled workflows (kvstore keeps the atomic precondition); OIDC service always built, decides disabled-mode errors; MergeRequests.Compare/PushSummary, Get returns NewCommits/HeadUnread.
  • Deliberate changes: the commits banner detects a rewrite by ancestry; the compare page uses the real target..source range.
  • Ignore directives: 129 -> 49, none marked 'pending move'.

Reviewed (correctness/security + architecture/maintainability/tests), findings fixed or deferred (GO-005 wiring, GO-011/012 budget loop factoring). Rebased on main 4534992. Build, vet, gofmt, hexcheck -fail-on warn, go test ./internal/... and -race on touched packages pass.

Keyboard shortcuts

Single-key shortcuts are on. ctrl k and esc work whatever this says.

ctrl k :
go to a section, branch, tag, file or repository
?
this list
j k
move the row cursor down / up
enter
open the row under the cursor; on Files, a directory opens or closes in place
↑ ↓ ← → home end
in the file tree (focus on a row): previous / next row, close / open a directory or go to its parent / first entry, first / last row. They never open a file
l h
on Files: open a directory or a file / close a directory or go up to its parent, then to the parent page. Elsewhere: into the side panel (on a merge request, its files) / back to the content; there j k move between its controls
g g G
first / last row
/
filter the list; on Files, find a file anywhere in the repository (esc clears, enter opens the best match)
b
switch branch or tag
g s g f g c g b g m g i g r g a
tabs: summary, files, commits, branches & tags, merge requests, issues, releases, actions (when available)
g h
home
w
wide mode: use the whole width of the screen, or not (remembered)
y
copy a permalink pinned to the commit
d u x
diff: split / unified / collapse or expand all
e E
diff: the whole file in view / every file, with the lines the hunks hide
n
merge request: next note in the diff
l
merge request: the list of files (j k move, enter opens, esc back)
M
merge request: the merge menu; there 1–3 pick the mode, D deletes the branch, enter merges
R
merge request: mark the head read
esc
close a dialog, leave a field

Branches 2

Tags 44

↑↓ moveenter openesc close