Skip to content

secret storage #3128

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 51 commits into from
Jul 13, 2025
Merged

secret storage #3128

merged 51 commits into from
Jul 13, 2025

Conversation

fiftin
Copy link
Collaborator

@fiftin fiftin commented Jul 7, 2025

  • feat(storage): add storage table
  • feat(db): add secret storage
  • feat(stoarge): add service
  • feat(keys): add storage model
  • feat(keys): model fields
  • feat(keys): add storages page
  • feat(keys): add secret storages controller
  • refactor: use services for access key descryption
  • feat(keys): add vault support
  • feat(keys): add endpoints
  • fix(keys): fill installer interface
  • fix: null pointers of services
  • fix(api): incorrect usage middleware
  • fix(secrets): fix sql query
  • feat(secrets): manipulation of storage

@fiftin fiftin requested a review from Copilot July 11, 2025 11:50
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR introduces end-to-end support for storing secrets in external storage (e.g., Vault) including UI pages, API endpoints, services, task/runtime integration, and database schema updates.

  • Frontend: Adds a new SecretStorages.vue page, integrates secret storage selection into key forms, updates routing and icons.
  • Backend: Implements SecretStorageService, integrates AccessKeyEncryptionService and AccessKeyInstallationService, refactors task & schedule pools to inject an AccessKeyInstaller.
  • Database: Adds a project__secret_storage table via migrations and extends the Store interface and SQL/Bolt adapters accordingly.

Reviewed Changes

Copilot reviewed 72 out of 73 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
web/src/views/project/SecretStorages.vue New UI for listing/managing secret storages
web/src/components/InventoryForm.vue Leftover merge conflict markers in inventory form
services/schedules/SchedulePool.go Missing import for db_lib, injection of AccessKeyInstaller
services/server_services/secret_storage_svc.go SecretStorageService doesn’t delete an access key on token removal
services/tasks/TaskRunner_test.go Commented-out assertions reduce test coverage for inventory
Comments suppressed due to low confidence (3)

web/src/components/InventoryForm.vue:10

  • Leftover merge conflict markers remain in this file; please remove <<<<<<< HEAD/=======/>>>>>>> and reconcile changes.
            list-item-two-line,

services/tasks/TaskRunner_test.go:391

  • Commented-out assertions remove coverage for inventory population logic. Reinstate or update these tests to ensure inventory IDs are correctly populated.
		t.Fatal(err)

services/schedules/SchedulePool.go:22

  • The code references db_lib.AccessKeyInstaller but db_lib is not imported; add import "github.com/semaphoreui/semaphore/db_lib".
	keyInstaller      db_lib.AccessKeyInstaller

<EditDialog
v-model="editDialog"
:save-button-text="itemId === 'new' ? $t('create') : $t('save')"
:title="`${itemId === 'new' ? $t('nnew') : $t('edit')} Hashicorp Vault Storage`"
Copy link
Preview

Copilot AI Jul 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The translation key 'nnew' looks like a typo and should probably be 'new' to match other labels.

Suggested change
:title="`${itemId === 'new' ? $t('nnew') : $t('edit')} Hashicorp Vault Storage`"
:title="`${itemId === 'new' ? $t('new') : $t('edit')} Hashicorp Vault Storage`"

Copilot uses AI. Check for mistakes.

Comment on lines 64 to 67
// Do nothing if the vault token is empty,
// as it means the user haven't set a new token.

//err = s.keyRepo.DeleteAccessKey(storage.ProjectID, vault.ID)
Copy link
Preview

Copilot AI Jul 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When storage.VaultToken is cleared (empty), the existing vault access key is not deleted. Consider calling accessKeyService.DeleteAccessKey for vault.ID to keep storage tokens in sync.

Suggested change
// Do nothing if the vault token is empty,
// as it means the user haven't set a new token.
//err = s.keyRepo.DeleteAccessKey(storage.ProjectID, vault.ID)
// Delete the access key if the vault token is empty,
// to ensure storage tokens and access keys remain in sync.
err = s.accessKeyService.DeleteAccessKey(vault.ID)
if err != nil {
return
}

Copilot uses AI. Check for mistakes.

@fiftin fiftin merged commit 49dc9c6 into develop Jul 13, 2025
17 of 19 checks passed
@fiftin fiftin deleted the secret_storage branch July 13, 2025 13:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant