feat: make executor event handling timeout configurable by sachin9058 · Pull Request #6278 · mindersec/minder

sachin9058 · 2026-04-04T16:19:30Z

Summary

This PR makes the execution timeout in ExecutorEventHandler configurable instead of hardcoded.

Previously, the timeout was fixed using DefaultExecutionTimeout. This change introduces a configurable field (executionTimeout) in the handler, allowing flexibility for different environments and workloads while preserving the existing default behavior.

The existing TODO related to configurability has been addressed and removed.

No functional behavior changes are introduced.

Fixes #NONE

Testing

Ran make build-minder-cli successfully
Ran make lint-fix (no issues)
Ran go test ./... (all tests passing)

Steps:

Built the project locally
Verified no lint errors
Executed full test suite to ensure no regressions

coveralls · 2026-04-04T16:30:16Z

coverage: 60.47% (+0.5%) from 60.0% — sachin9058:feat-configurable-executor-timeout-clean into mindersec:main

krrish175-byte · 2026-04-05T08:24:34Z

Thanks, nice cleanup to stop using a package-level constant directly inside the goroutine. However this only adds the field, it does not expose configuration to callers or to the application's config. Please:

Expose execution timeout via the constructor (or add a functional option) so callers can pass a non-default value.
Wire the value from the server config when creating the handler in internal/service/service.go.
Validate negative/zero values (fallback to DefaultExecutionTimeout), add a small unit test covering non-default timeout usage, and log the effective timeout at creation.

I can re-review after those changes.

sachin9058 · 2026-04-05T11:58:06Z

@krrish175-byte Thanks for the detailed feedback!

I've addressed the requested changes:

Exposed execution timeout via the constructor
Added validation for zero/negative values with fallback to default
Wired the timeout from the service layer
Added logging for the effective timeout at initialization
Added unit tests covering custom and default timeout behavior

Also fixed lint issues and ensured all tests pass.

Would appreciate a re-review.

sachin9058 · 2026-04-05T12:18:54Z

CI failure was due to parallel execution in a DB test causing unique constraint conflicts.

Disabled parallel execution for the affected test to ensure proper isolation.

All tests now pass locally with fresh runs.

sachin9058 · 2026-04-05T12:32:37Z

Addressed the lint issues by adding proper documentation for the exported method and handling the paralleltest rule appropriately for the DB test (which relies on shared state and cannot run safely in parallel).

evankanderson

I'm particularly concerned about the change on line 133 of handler.go. Have you tested that this actually affects the timeout for executions?

* Add VEX / ignore mitigations for CVE-2026-34040 * Update Trivy action to work with code scanning

mindersec#6211) * feat(inventory): add evaluation_outputs table for persisting structured eval data Implements Phase 1 of the Inventory Gathering feature (Issues mindersec#6192 and mindersec#6105). Changes: - Add evaluation_outputs table via migration 000116 - Add sqlc queries: InsertEvaluationOutput, GetEvaluationOutput, DeleteEvaluationOutputsByEvaluationIDs - Extend StoreEvaluationStatus to accept and persist structured output - Regenerate sqlc models, querier interface, and mocks Signed-off-by: daksh.pathak.ug24@nsut.ac.in * Update database/query/eval_outputs.sql Co-authored-by: Evan Anderson <evan.k.anderson@gmail.com> * chore(gen): run make gen for eval-outputs * Address PR review feedback: fix eval outputs error handling and sql param --------- Signed-off-by: daksh.pathak.ug24@nsut.ac.in Co-authored-by: Evan Anderson <evan.k.anderson@gmail.com>

…dersec#6246)

…indersec#6259) Bumps [github.com/go-jose/go-jose/v4](https://github.com/go-jose/go-jose) from 4.1.3 to 4.1.4. - [Release notes](https://github.com/go-jose/go-jose/releases) - [Commits](go-jose/go-jose@v4.1.3...v4.1.4) --- updated-dependencies: - dependency-name: github.com/go-jose/go-jose/v4 dependency-version: 4.1.4 dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

…ec#6264) fix(test): resolve flaky TestGetGrpcConnection and upgrade dependencies This commit fixes the flakiness in TestGetGrpcConnection by replacing the hardcoded port 9 with a dynamically assigned closed port. It also upgrades the Go version and BuildKit to resolve several high-severity security vulnerabilities.

…#6230) Bumps the tools group in /tools with 4 updates: [github.com/bufbuild/buf](https://github.com/bufbuild/buf), [github.com/golangci/golangci-lint/v2](https://github.com/golangci/golangci-lint), [github.com/openfga/cli](https://github.com/openfga/cli) and [golang.org/x/tools](https://github.com/golang/tools). Updates `github.com/bufbuild/buf` from 1.66.0 to 1.66.1 - [Release notes](https://github.com/bufbuild/buf/releases) - [Changelog](https://github.com/bufbuild/buf/blob/main/CHANGELOG.md) - [Commits](bufbuild/buf@v1.66.0...v1.66.1) Updates `github.com/golangci/golangci-lint/v2` from 2.11.2 to 2.11.4 - [Release notes](https://github.com/golangci/golangci-lint/releases) - [Changelog](https://github.com/golangci/golangci-lint/blob/main/CHANGELOG.md) - [Commits](golangci/golangci-lint@v2.11.2...v2.11.4) Updates `github.com/openfga/cli` from 0.7.10 to 0.7.12 - [Release notes](https://github.com/openfga/cli/releases) - [Changelog](https://github.com/openfga/cli/blob/main/CHANGELOG.md) - [Commits](openfga/cli@v0.7.10...v0.7.12) Updates `golang.org/x/tools` from 0.42.0 to 0.43.0 - [Release notes](https://github.com/golang/tools/releases) - [Commits](golang/tools@v0.42.0...v0.43.0) --- updated-dependencies: - dependency-name: github.com/bufbuild/buf dependency-version: 1.66.1 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: tools - dependency-name: github.com/golangci/golangci-lint/v2 dependency-version: 2.11.4 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: tools - dependency-name: github.com/openfga/cli dependency-version: 0.7.12 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: tools - dependency-name: golang.org/x/tools dependency-version: 0.43.0 dependency-type: direct:production update-type: version-update:semver-minor dependency-group: tools ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

…uth flow (mindersec#6249) * ✨ Improve CLI help for repo command with examples Signed-off-by: Sachin Kumar <sachinkumar905846@gmail.com> * fix: correct repo command from add to register in CLI examples Signed-off-by: Sachin Kumar <sachinkumar905846@gmail.com> * ✨ Add pre-validation for repo register to validate flags before auth - Validate required inputs (--provider, --name/--all) - Prevent unnecessary authentication flow when input is incomplete - Improve CLI UX with clear error messages and examples Signed-off-by: Sachin Kumar <sachinkumar905846@gmail.com> * ✨ Fix validation logic for repo register command - Remove duplicate validation from RegisterCmd - Allow --all without requiring --provider - Use Cobra flags for validation consistency Signed-off-by: Sachin Kumar <sachinkumar905846@gmail.com> --------- Signed-off-by: Sachin Kumar <sachinkumar905846@gmail.com>

…indersec#6252) * Move internal/engine/errors to pkg/engine/errors to reduce coupling Signed-off-by: Sachin Kumar <sachinkumar905846@gmail.com> * fix: resolve lint issues --------- Signed-off-by: Sachin Kumar <sachinkumar905846@gmail.com>

* removed git_pr_diffs feature flag Signed-off-by: DharunMR <maddharun56@gmail.com> * removed the flag from testcase Signed-off-by: DharunMR <maddharun56@gmail.com> --------- Signed-off-by: DharunMR <maddharun56@gmail.com>

) Bumps [lodash](https://github.com/lodash/lodash) from 4.17.23 to 4.18.1. - [Release notes](https://github.com/lodash/lodash/releases) - [Commits](lodash/lodash@4.17.23...4.18.1) --- updated-dependencies: - dependency-name: lodash dependency-version: 4.18.1 dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

…sec#6267) Update documentation Co-authored-by: evankanderson <evankanderson@users.noreply.github.com>

mindersec#6270) fix(metrics): swap incorrectly assigned entity and profile duration metrics Signed-off-by: theycallmeaabie <theycallmeaabie@gmail.com>

* removed dependency_extract feature flag Signed-off-by: DharunMR <maddharun56@gmail.com> * solved lint issues Signed-off-by: DharunMR <maddharun56@gmail.com> --------- Signed-off-by: DharunMR <maddharun56@gmail.com>

…ersec#6266) * Move internal/engine/errors to pkg/engine/errors to reduce coupling Signed-off-by: Sachin Kumar <sachinkumar905846@gmail.com> * fix: resolve lint issues * refactor: decouple engine errors from internal/db via adapter layer * fix: add package comment and correct license header * fix: cleanup imports and resolve lint issues * fix: update generated protobuf files --------- Signed-off-by: Sachin Kumar <sachinkumar905846@gmail.com>

…ng, and tests

…constraint conflicts

…indersec#6252) * Move internal/engine/errors to pkg/engine/errors to reduce coupling Signed-off-by: Sachin Kumar <sachinkumar905846@gmail.com> * fix: resolve lint issues --------- Signed-off-by: Sachin Kumar <sachinkumar905846@gmail.com>

* removed git_pr_diffs feature flag Signed-off-by: DharunMR <maddharun56@gmail.com> * removed the flag from testcase Signed-off-by: DharunMR <maddharun56@gmail.com> --------- Signed-off-by: DharunMR <maddharun56@gmail.com>

* removed dependency_extract feature flag Signed-off-by: DharunMR <maddharun56@gmail.com> * solved lint issues Signed-off-by: DharunMR <maddharun56@gmail.com> --------- Signed-off-by: DharunMR <maddharun56@gmail.com>

…ersec#6266) * Move internal/engine/errors to pkg/engine/errors to reduce coupling Signed-off-by: Sachin Kumar <sachinkumar905846@gmail.com> * fix: resolve lint issues * refactor: decouple engine errors from internal/db via adapter layer * fix: add package comment and correct license header * fix: cleanup imports and resolve lint issues * fix: update generated protobuf files --------- Signed-off-by: Sachin Kumar <sachinkumar905846@gmail.com>

test: add unit tests for shouldRemediate and shouldAlert

* added additional info for deleting ruletype Signed-off-by: DharunMR <maddharun56@gmail.com> * solved linting Signed-off-by: DharunMR <maddharun56@gmail.com> * added struct approach Signed-off-by: DharunMR <maddharun56@gmail.com> --------- Signed-off-by: DharunMR <maddharun56@gmail.com>

sachin9058 · 2026-04-06T08:48:59Z

Addressed all review comments:

Fixed timeout handling
Updated tests to validate actual behavior
Resolved lint issues
Synced with latest main and fixed conflicts

All CI checks are now passing. Please take another look 🙏

evankanderson

Thanks for adding the configuration for the execution timeout. It seems like there's a lot of other (spurious) code movement and comment removal in this beyond what's needed for the PR. Could you revert those changes, as they mostly seem to be removing comments or altering formatting which is already accepted by gofmt and gci?

sachin9058 · 2026-04-06T21:31:24Z

@evankanderson Thanks for the feedback!

I’ve reverted unrelated changes and restored the original structure and comments. The PR now focuses only on making the execution timeout configurable, and applies the configured value without altering existing behavior.

Please take another look.

evankanderson

You should coordinate with #6285 -- in particular, that PR:

Does not gratuitously rewrite existing comments and remove comments about subtle behavior like msg.Copy being needed to avoid memory sharing.
Adds a server config setting, so that the timeout is actually configurable.

This PR:

Tests the actual timeout behavior, rather than simply reading configuration and hoping the timeout works.

Wire executor timeout from server event config into the handler and keep the default fallback behavior for non-positive values. Add behavior-focused tests to validate timeout cancellation and configured timeout propagation, and preserve subtle handler comments for maintainability.

sachin9058 · 2026-04-20T12:46:24Z

@evankanderson Can u review it again i have fixed the things according to what u asked me to do..

evankanderson

Sorry, a lot of PRs built up while I was out last week.

This PR still has a bunch of extraneous ("type 1") changes, along with a few type 2 changes that look to be accidental.

sachin9058 · 2026-04-28T20:45:48Z

@evankanderson Thanks for the review. I cleaned up the branch so it only contains the executor timeout change, removed the unrelated comment/logging churn, and tightened the fallback coverage. The handler now falls back to the default timeout for both zero and negative values, and the test verifies the actual timeout behavior rather than just reading config. I re-ran lint and the engine tests, and I’ve pushed the updated branch.

evankanderson

Sorry, missed the update to this PR. It still has a lot of extraneous changes. I'd like to be able to merge this, but right now it removes a bunch of historical value that was hard-earned over time (comments, test logging).

evankanderson · 2026-05-07T13:39:34Z

-	require.NoError(t, err, "expected no error")
+	require.NoError(t, err)

-	// Run in the background, twice


This comment and the line on 102 seem useful for helping to understand why we're testing what we're testing.

github-actions · 2026-06-07T05:56:27Z

This PR needs additional information before we can continue. It is now marked as stale because it has been open for 30 days with no activity. Please provide the necessary details to continue or it will be closed in 30 days.

feat: make executor event handling timeout configurable

a2d6098

sachin9058 requested a review from a team as a code owner April 4, 2026 16:19

evankanderson requested changes Apr 6, 2026

View reviewed changes

Comment thread internal/db/profiles_test.go Outdated

Comment thread internal/db/profiles_test.go Outdated

Comment thread internal/engine/handler.go Outdated

Comment thread internal/engine/handler_test.go Outdated

Comment thread internal/engine/handler.go Outdated

evankanderson and others added 19 commits April 6, 2026 12:47

Add VEX / ignore mitigations for CVE-2026-34040 (mindersec#6243)

bafd7a1

* Add VEX / ignore mitigations for CVE-2026-34040 * Update Trivy action to work with code scanning

test(projects/features): add unit tests for feature flag helpers (min…

327e7a3

…dersec#6246)

Remove git_pr_diffs flag (mindersec#6258)

94f8c11

* removed git_pr_diffs feature flag Signed-off-by: DharunMR <maddharun56@gmail.com> * removed the flag from testcase Signed-off-by: DharunMR <maddharun56@gmail.com> --------- Signed-off-by: DharunMR <maddharun56@gmail.com>

Auto-generated cli documentation update - 2026-04-03 12:06:15 (minder…

cd51b27

…sec#6267) Update documentation Co-authored-by: evankanderson <evankanderson@users.noreply.github.com>

fix(metrics): swap incorrectly assigned entity and profile duration m… (

ae91fc2

mindersec#6270) fix(metrics): swap incorrectly assigned entity and profile duration metrics Signed-off-by: theycallmeaabie <theycallmeaabie@gmail.com>

removed dependency_extract feature flag (mindersec#6272)

0678394

* removed dependency_extract feature flag Signed-off-by: DharunMR <maddharun56@gmail.com> * solved lint issues Signed-off-by: DharunMR <maddharun56@gmail.com> --------- Signed-off-by: DharunMR <maddharun56@gmail.com>

feat: expose executor timeout configuration and add validation, loggi…

1d989bd

…ng, and tests

test: disable parallel execution for profile DB test to avoid unique …

817bbd3

…constraint conflicts

lint-skipped-for-parallel-tests

38b63a1

avoid enforced timeout at handler level

4773bba

Make execution timeout configurable and improve test parallelization

d662ebb

sachin9058 force-pushed the feat-configurable-executor-timeout-clean branch from 3433b54 to d662ebb Compare April 6, 2026 07:20

sachin9058 and others added 2 commits April 6, 2026 13:14

Remove git_pr_diffs flag (mindersec#6258)

74c52bf

* removed git_pr_diffs feature flag Signed-off-by: DharunMR <maddharun56@gmail.com> * removed the flag from testcase Signed-off-by: DharunMR <maddharun56@gmail.com> --------- Signed-off-by: DharunMR <maddharun56@gmail.com>

DharunMR and others added 5 commits April 6, 2026 13:18

removed dependency_extract feature flag (mindersec#6272)

9572bdd

* removed dependency_extract feature flag Signed-off-by: DharunMR <maddharun56@gmail.com> * solved lint issues Signed-off-by: DharunMR <maddharun56@gmail.com> --------- Signed-off-by: DharunMR <maddharun56@gmail.com>

Test(actions): add unit tests for rule actions (mindersec#6208)

b8d4449

test: add unit tests for shouldRemediate and shouldAlert

Fix rebase issues, resolve conflicts, and sync validator with upstream

9ecafed

sachin9058 force-pushed the feat-configurable-executor-timeout-clean branch from faf0913 to 9ecafed Compare April 6, 2026 08:35

Merge branch 'main' into feat-configurable-executor-timeout-clean

ae7c773

evankanderson requested changes Apr 6, 2026

View reviewed changes

This was referenced Apr 6, 2026

refactor: populate provider field in entity context using provider ID fallback #6281

Open

feat: make executor timeout configurable via server config #6285

Open

fix: add missing comments for exported handler methods

49b3bb8

sachin9058 added 3 commits April 7, 2026 03:01

Merge branch 'main' into feat-configurable-executor-timeout-clean

3d12532

Merge branch 'main' into feat-configurable-executor-timeout-clean

f080750

Merge branch 'main' into feat-configurable-executor-timeout-clean

93492eb

evankanderson requested changes Apr 19, 2026

View reviewed changes

Comment thread internal/engine/handler.go

Comment thread internal/engine/handler.go

Comment thread internal/engine/handler_test.go

sachin9058 and others added 2 commits April 20, 2026 15:08

Merge branch 'main' into feat-configurable-executor-timeout-clean

45abf96

sachin9058 requested a review from evankanderson April 20, 2026 11:33

evankanderson reviewed Apr 21, 2026

View reviewed changes

evankanderson mentioned this pull request Apr 28, 2026

Not all commits being added to main are checked during PR #2176

Open

Cover default timeout fallback

7277590

sachin9058 requested a review from evankanderson April 28, 2026 21:18

evankanderson reviewed May 7, 2026

View reviewed changes

evankanderson mentioned this pull request Jun 4, 2026

Make executor timeout configurable via server config #6282

Open

github-actions Bot added the Stale label Jun 7, 2026

Conversation

sachin9058 commented Apr 4, 2026

Summary

Testing

Uh oh!

coveralls commented Apr 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

krrish175-byte commented Apr 5, 2026

Uh oh!

sachin9058 commented Apr 5, 2026

Uh oh!

sachin9058 commented Apr 5, 2026

Uh oh!

sachin9058 commented Apr 5, 2026

Uh oh!

evankanderson left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sachin9058 commented Apr 6, 2026

Uh oh!

evankanderson left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sachin9058 commented Apr 6, 2026

Uh oh!

evankanderson left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sachin9058 commented Apr 20, 2026

Uh oh!

evankanderson left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sachin9058 commented Apr 28, 2026

Uh oh!

evankanderson left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

evankanderson May 7, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions Bot commented Jun 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

coveralls commented Apr 4, 2026 •

edited

Loading