Skip to content

feat: add config apply action#39

Merged
intel352 merged 2 commits into
mainfrom
feat/auth-admin-apply
Jun 2, 2026
Merged

feat: add config apply action#39
intel352 merged 2 commits into
mainfrom
feat/auth-admin-apply

Conversation

@intel352
Copy link
Copy Markdown
Contributor

@intel352 intel352 commented Jun 2, 2026

Summary

  • Add optional generic Apply changes action for config-form contributions that advertise metadata.apply_path.
  • Harden config-form admin endpoint calls to same-origin /api/admin... routes with strict JSON handling.
  • Run validation before apply when validate_path is present, fail closed on invalid/malformed validation, disable controls during requests, and clear write-only secret fields after successful apply.

Verification

  • go test ./...
  • git diff --check
  • Exercised by Scenario 90 Docker + Playwright in the paired workflow-scenarios branch.

Review Notes

  • Adversarial review found and the patch addressed: validation bypass, unsafe arbitrary apply_path, weak error handling, duplicate submits, stale validation caching, non-JSON fail-open behavior, and retained secret input values.

Copy link
Copy Markdown

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

Adds an optional “Apply changes” action to config-form admin tool contributions (gated by metadata.apply_path) and hardens config-form endpoint calls by enforcing same-origin /api/admin... routing plus stricter JSON parsing/handling. It also ensures validation is run (when available) before apply, disables form controls during requests, and clears write-only secret inputs after a successful apply.

Changes:

  • Introduces resolveAdminEndpoint() + fetchAdminJSON() to enforce same-origin admin API routing and strict-ish JSON/error handling for config-form endpoints.
  • Adds optional “Apply changes” flow with pre-apply validation (when validate_path exists), busy/disabled UI state, and secret-field clearing after successful apply.
  • Extends embedded-asset regression tests to assert presence of the new UI/action strings and helpers.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
internal/ui_dist/index.html Adds apply action + validation gating, same-origin admin endpoint resolution, strict JSON handling, busy-state UI disabling, and secret-field clearing.
internal/assets_test.go Updates embedded admin shell assertions to include new strings/helpers introduced in the UI.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread internal/ui_dist/index.html
@intel352 intel352 merged commit 1bcc382 into main Jun 2, 2026
8 checks passed
@intel352 intel352 deleted the feat/auth-admin-apply branch June 2, 2026 11:07
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.

2 participants