✨ feat(buckets/policy): add bucket policy UI, handlers, MinIO client methods, and tests #13

wilinz · 2026-02-07T16:46:56Z

No description provided.

…methods, and tests

…displaying.

damacus · 2026-02-08T02:14:48Z

Thanks 👍

Can you provide a quick screenshot of the new functionality please

Copilot

Pull request overview

Adds bucket policy support across the UI and backend so users can view and set bucket access policies (private / public-read / public-read-write / custom), and exposes policy status in bucket listings and the object browser.

Changes:

Add bucket policy view/edit UI (settings card + reusable partial) and policy badges in bucket list/browser.
Add MinIO client interface methods + new Echo handlers/routes to get/set bucket policies.
Add unit tests for policy JSON canonicalization/type detection and update journey test mocks for new policy lookups.

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 10 comments.

Show a summary per file

File	Description
`views/partials/bucket_policy.html`	New reusable HTMX/Alpine bucket policy editor fragment.
`views/pages/buckets.html`	Shows a policy-type badge next to each bucket in the list.
`views/pages/bucket_settings.html`	Adds a bucket policy settings section (currently duplicated vs partial).
`views/pages/browser.html`	Adds policy badge/modal and “Direct Link” dialog + copy action.
`views/layouts/base.html`	Removes global HTMX boosted navigation attributes from `<body>`.
`internal/services/minio_factory.go`	Extends `MinioClient` with bucket policy methods; exports `ShouldUseSSL`.
`internal/renderer/renderer.go`	Registers `bucket_policy` as a self-executing partial template.
`internal/handlers/policy_test.go`	Adds tests for `canonicalJSON` and `detectPolicyType`.
`internal/handlers/buckets_handler.go`	Fetches policy + type in list/browser/settings; adds get/set policy handlers.
`cmd/server/mocks_test.go`	Adds mocked `GetBucketPolicy`/`SetBucketPolicy`.
`cmd/server/main.go`	Registers new bucket policy routes.
`cmd/server/bucket_journey_test.go`	Updates mocks to account for policy lookups in journeys.

Copilot · 2026-02-08T02:18:17Z

internal/handlers/buckets_handler.go

+		// No policy is not an error, just return empty
+		policy = ""


GetBucketPolicy currently treats any error from MinIO as "no policy" by forcing policy = "". This can hide real failures (e.g., access denied / endpoint issues) and show an incorrect policy state.

Recommend distinguishing "no policy" from other errors (e.g., via MinIO error code) and returning a visible error message for non-not-found failures.

Suggested change

// No policy is not an error, just return empty

policy = ""

// Distinguish "no policy" from other MinIO errors

errResp := minio.ToErrorResponse(err)

if errResp.Code == "NoSuchBucketPolicy" || errResp.StatusCode == http.StatusNotFound {

// No policy is not an error, just return empty

policy = ""

} else {

return c.Render(http.StatusOK, "bucket_policy", map[string]interface{}{

"BucketName": bucketName,

"PolicyType": "private",

"Error": fmt.Sprintf("Failed to get bucket policy: %s", err.Error()),

})

}

Copilot · 2026-02-08T02:18:18Z

views/pages/bucket_settings.html

+                <form hx-post="/buckets/{{ .BucketName }}/policy" class="space-y-4 pt-4 border-t border-zinc-700">
+                    <div>
+                        <label class="block text-xs text-zinc-500 mb-2">Access Policy</label>
+                        <div class="grid grid-cols-2 gap-2">
+                            <label class="flex items-center gap-2 p-3 bg-zinc-800/50 rounded-lg cursor-pointer border border-transparent hover:border-zinc-600 transition-colors"


This hx-post form has no hx-target/hx-swap, but SetBucketPolicy can respond with the full bucket_policy fragment on validation/server errors. With HTMX defaults, the response will replace only the <form> element, which will break the surrounding policy UI (duplicate/nested components, Alpine state issues).

Add an explicit target (e.g., a wrapping container) and swap strategy (commonly outerHTML) so error responses replace the entire policy component consistently.

Copilot · 2026-02-08T02:18:18Z

views/pages/bucket_settings.html

+            <div class="space-y-4" x-data="{
+                policyType: '{{ .PolicyType }}',
+                customPolicy: '',
+                showEditor: {{ if eq .PolicyType "custom" }}true{{ else }}false{{ end }},
+                init() {
+                    const el = this.$refs.policyData;
+                    if (el && el.value) this.customPolicy = el.value;
+                }
+            }">
+                <textarea x-ref="policyData" class="hidden">{{ .FormattedPolicy }}</textarea>
+


The bucket policy UI is duplicated inline here, while there is also a new reusable views/partials/bucket_policy.html fragment and a GET /buckets/:bucketName/policy handler. Keeping two copies increases the chance of UI drift and makes HTMX updates harder.

Consider loading the policy section the same way as the other settings cards (e.g., hx-get on load to render the bucket_policy partial) and removing the duplicated markup from this page.

Copilot · 2026-02-08T02:18:18Z

views/partials/bucket_policy.html

+    <form hx-post="/buckets/{{ .BucketName }}/policy" class="space-y-4 pt-4 border-t border-zinc-700">
+        <div>
+            <label class="block text-xs text-zinc-500 mb-2">Access Policy</label>
+            <div class="grid grid-cols-2 gap-2">
+                <label class="flex items-center gap-2 p-3 bg-zinc-800/50 rounded-lg cursor-pointer border border-transparent hover:border-zinc-600 transition-colors"


This policy form has no hx-target/hx-swap. Since SetBucketPolicy returns the full bucket_policy fragment on errors, HTMX will replace only the <form> by default, leaving the rest of the component stale and potentially nesting fragments.

Add a stable wrapper (id/class) and set the form to target/swap that wrapper so the whole component updates together on both success and error paths.

Copilot · 2026-02-08T02:18:19Z

views/pages/browser.html

+                    const url = '{{ .EndpointURL }}/{{ .BucketName }}/' + key;
+                    this.directLinkURL = url;
+                    this.directLinkDialogOpen = true;
+                    this.$nextTick(() => lucide.createIcons());
+                },
+
+                copyDirectLinkToClipboard() {
+                    navigator.clipboard.writeText(this.directLinkURL);


copyDirectLink builds a URL by string concatenation without URL-encoding the object key. Keys containing spaces, #, ?, or non-ASCII characters will produce invalid links.

Use proper URL construction/encoding (e.g., encode the key path segment) and consider handling the navigator.clipboard.writeText(...) promise to report failures (clipboard APIs often require HTTPS/permissions).

Suggested change

const url = '{{ .EndpointURL }}/{{ .BucketName }}/' + key;

this.directLinkURL = url;

this.directLinkDialogOpen = true;

this.$nextTick(() => lucide.createIcons());

},

copyDirectLinkToClipboard() {

navigator.clipboard.writeText(this.directLinkURL);

const encodedKey = encodeURIComponent(key);

const url = '{{ .EndpointURL }}/{{ .BucketName }}/' + encodedKey;

this.directLinkURL = url;

this.directLinkDialogOpen = true;

this.$nextTick(() => lucide.createIcons());

},

copyDirectLinkToClipboard() {

if (!navigator.clipboard || !navigator.clipboard.writeText) {

console.error('Clipboard API is not available in this browser/context.');

alert('Clipboard is not available. Please copy the link manually.');

return;

}

navigator.clipboard.writeText(this.directLinkURL)

.then(() => {

// Optionally, you could show a success notification here.

})

.catch((err) => {

console.error('Failed to copy direct link to clipboard:', err);

alert('Failed to copy link to clipboard. Please copy it manually.');

});

Copilot · 2026-02-08T02:18:19Z

internal/handlers/buckets_handler.go

+		// Fetch bucket policy
+		policy, _ := client.GetBucketPolicy(c.Request().Context(), b.Name)
+		policyType := detectPolicyType(policy, b.Name)
+
 		bucketsWithStats = append(bucketsWithStats, BucketWithStats{
 			BucketInfo:    b,
 			Size:          size,
 			FormattedSize: utils.FormatBytes(size),
+			PolicyType:    policyType,
 		})


ListBuckets now calls GetBucketPolicy once per bucket and ignores any error. This introduces an N+1 network/API call pattern that can noticeably slow the buckets list for larger installations, and an error will silently mislabel the bucket as "private".

Consider lazy-loading policy badges (HTMX per-row), caching policies for the request, or fetching policies concurrently with a small worker pool; also handle errors explicitly (e.g., show an "unknown" badge/tooltip instead of defaulting to private).

Copilot · 2026-02-08T02:18:19Z

internal/handlers/buckets_handler.go

+	// Fetch bucket policy
+	policy, _ := client.GetBucketPolicy(c.Request().Context(), bucketName)
+	policyType := detectPolicyType(policy, bucketName)
+	formattedPolicy := ""


BrowseBucket ignores GetBucketPolicy errors (policy, _ := ...). If policy retrieval fails (auth/network/etc.), the UI will present the bucket as "private" and may generate incorrect direct links.

Handle the error and only treat the policy as empty when the error is the specific "no policy"/not-found condition; otherwise surface an error state to the template.

Copilot · 2026-02-08T02:18:19Z

views/pages/browser.html

                                    </button>
+                                    <button @click="copyDirectLink('{{ .Key }}')"
+                                        class="p-2 text-zinc-400 hover:text-white hover:bg-zinc-700 rounded-md transition-colors" title="Copy Direct Link">
+                                        <i data-lucide="link" size="16"></i>
+                                    </button>


The UI says direct links are "Only available when your bucket policy includes public read access", but the "Copy Direct Link" action is always shown/enabled. This is misleading for private/custom buckets and may encourage sharing non-working URLs.

Consider conditionally hiding/disabling this button (or changing the dialog copy) based on .PolicyType so behavior matches the UI guidance.

Suggested change

</button>

<button @click="copyDirectLink('{{ .Key }}')"

class="p-2 text-zinc-400 hover:text-white hover:bg-zinc-700 rounded-md transition-colors" title="Copy Direct Link">

<i data-lucide="link" size="16"></i>

</button>

</button>

{{ if eq $.PolicyType "public-read" }}

<button @click="copyDirectLink('{{ .Key }}')"

class="p-2 text-zinc-400 hover:text-white hover:bg-zinc-700 rounded-md transition-colors" title="Copy Direct Link">

<i data-lucide="link" size="16"></i>

</button>

{{ end }}

Copilot · 2026-02-08T02:18:19Z

views/layouts/base.html


-<body class="bg-background text-zinc-100 h-screen w-screen flex overflow-hidden" x-data="{ collapsed: false }"
-    hx-boost="true" hx-target="#main-content" hx-select="#main-content" hx-swap="outerHTML">
+<body class="bg-background text-zinc-100 h-screen w-screen flex overflow-hidden" x-data="{ collapsed: false }">


Removing hx-boost/hx-target/hx-select/hx-swap from the <body> changes navigation behavior application-wide (links will no longer be boosted into #main-content). This appears unrelated to the bucket policy feature set and could regress UX/performance.

If this was intentional, consider keeping the boosted navigation (or replacing it with a narrower scope) and remove the now-redundant hx-boost="false" usage on the logout link; otherwise, revert this change to keep the PR scoped.

Copilot · 2026-02-08T02:18:20Z

internal/handlers/buckets_handler.go

+	// Get bucket policy
+	policy, _ := client.GetBucketPolicy(c.Request().Context(), bucketName)
+	policyType := detectPolicyType(policy, bucketName)
+	formattedPolicy := ""


BucketSettings also ignores GetBucketPolicy errors (policy, _ := ...), which can silently misreport policy state in the settings UI.

Handle errors explicitly and only treat the policy as empty when the error represents "no policy"; otherwise surface an error message so users can tell the policy couldn't be loaded.

damacus · 2026-02-08T23:19:09Z

Thanks for the PR. Ca ln you address the comments please. The main one I'm concerned about is squashing error codes.

wilinz added 3 commits February 8, 2026 00:41

✨ feat(buckets/policy): add bucket policy UI, handlers, MinIO client …

8aab387

…methods, and tests

🐛 fix(views/layouts/base.html): fixed the issue where the UI was not …

6b51771

…displaying.

✨ feat(browser/minio): add Direct Link dialog & copy-to-clipboard

d689f18

damacus requested a review from Copilot February 8, 2026 02:11

Copilot started reviewing on behalf of damacus February 8, 2026 02:12 View session

Copilot AI reviewed Feb 8, 2026

View reviewed changes

-		// No policy is not an error, just return empty
-		policy = ""
+		// Distinguish "no policy" from other MinIO errors
+		errResp := minio.ToErrorResponse(err)
+		if errResp.Code == "NoSuchBucketPolicy" || errResp.StatusCode == http.StatusNotFound {
+			// No policy is not an error, just return empty
+			policy = ""
+		} else {
+			return c.Render(http.StatusOK, "bucket_policy", map[string]interface{}{
+				"BucketName": bucketName,
+				"PolicyType": "private",
+				"Error":      fmt.Sprintf("Failed to get bucket policy: %s", err.Error()),
+			})
+		}

✨ feat(buckets/policy): add bucket policy UI, handlers, MinIO client methods, and tests #13

Are you sure you want to change the base?

✨ feat(buckets/policy): add bucket policy UI, handlers, MinIO client methods, and tests #13

Uh oh!

Conversation

wilinz commented Feb 7, 2026

Uh oh!

damacus commented Feb 8, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 8, 2026

Choose a reason for hiding this comment

Uh oh!

damacus commented Feb 8, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants