Bump the default prepackaged Playbooks plugin from 2.9.0 to 2.9.1 (non-FIPS).
FIPS prepackaged plugin package list is unchanged.
Co-authored-by: Cursor <cursoragent@cursor.com>
* MM-68952: Resolve public channel mentions for non-members under Compliance
Channel mention name resolution reused HasPermissionToReadChannel, a
content-read check that returns false for non-members of a public channel
when Compliance Monitoring is enabled (MM-45272) or when the channel is on
another team (MM-66791). As a result, the channel_mentions post prop was
stripped per-viewer (since #34235), and the webapp fell back to rendering the
raw (anonymized) channel slug instead of a clickable link.
Introduce HasPermissionToResolveChannelMention, which exposes only a public
channel display name and link (not content) and is therefore independent of
ComplianceSettings, while still requiring team membership for public channels
(blocks cross-team disclosure) and channel membership for private/DM/GM
channels. Switch the three mention call sites (FillInPostProps,
sanitizeChannelMentionsForUser, channelMentionsBroadcastHook) to the new
helper. HasPermissionToReadChannel and all content-read paths are unchanged.
* MM-68952: Add author-side and E2E coverage for channel mention resolution
Add a Go test (TestFillInPostPropsChannelMentionResolution) that exercises the
author-side persistence of the channel_mentions prop in FillInPostProps. It
locks in the new behavior: an author who is a team member but not a member of a
referenced public channel now persists the mention prop even when Compliance
Monitoring is enabled, while public channels on other teams and private
channels the author is not in are still dropped.
Add a Playwright spec (channel_mention_resolution.spec.ts) with a license-free
cross-team case (a public channel mention stays unresolved for a viewer not on
the channel's team) and a license-gated case (with Compliance enabled, a team
member who is not in the channel sees the resolved mention link).
* MM-68952: Tighten channel mention test assertions and fix lint
Use strings.Builder when assembling the test message to avoid the
golangci-lint stringsbuilder (modernize) warning about string += string
in a loop. Tighten the cross-team public and private non-member cases to
assert.Nil on the resolved mentions so the contract requires no persisted
channel_mentions map rather than merely an empty one.
---------
(cherry picked from commit 61643e1066)
Co-authored-by: Maria A Nunez <maria.nunez@mattermost.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Apply CheckUserAllAuthenticationCriteria after guest magic-link token
authentication in POST /api/v4/users/login, matching the web one-time-link
handler and password login paths.
Add regression test ensuring deactivated guests receive 401 inactive while
active guests can still log in via magic_link_token.
(cherry picked from commit 5c360d8077)
Co-authored-by: Julien Tant <785518+JulienTant@users.noreply.github.com>
Co-authored-by: Cursor Agent <cursoragent@cursor.com>
Co-authored-by: Julien Tant <JulienTant@users.noreply.github.com>
The plugin API was silently redirecting calls using the old
"custom_profile_attributes" group name to the new "access_control" group.
Replace the silent alias with an explicit error so plugin developers get
a clear message telling them what to change.
(cherry picked from commit 159ed5ad96)
Co-authored-by: David Krauser <david@krauser.org>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
* [MM-68693] Resource level permission policies and new simulation (#36472)
(cherry picked from commit ba1cec51a5)
---------
Co-authored-by: Ibrahim Serdar Acikgoz <serdaracikgoz86@gmail.com>
* Add TestDesanitizeRemovesAllFakeSettings to catch future omissions
Walks every string field in the config after a Sanitize+desanitize
round-trip and fails if any still holds FakeSetting. This catches the
case where a field is added to Sanitize without a corresponding
desanitize entry.
* Fix ElasticsearchSettings.ClientKey being incorrectly masked as a secret
ClientKey is a file path, not a secret value. Masking it caused the
asterisk string to be persisted to the database on config writes, which
broke TLS client auth on restart.
* Fix desanitize missing entries for fields added in 504fb96fdd504fb96fdd masked five fields in Sanitize without adding the
corresponding desanitize entries, meaning a config save through the
API would permanently overwrite those fields with FakeSetting:
- FileSettings.ExportAmazonS3SecretAccessKey
- ServiceSettings.GoogleDeveloperKey
- ServiceSettings.GiphySdkKey
- CacheSettings.RedisPassword
- AutoTranslationSettings.LibreTranslate.APIKey
* fixup! Add TestDesanitizeRemovesAllFakeSettings to catch future omissions
* fixup! Fix desanitize missing entries for fields added in 504fb96fdd
---------
Co-authored-by: Mattermost Build <build@mattermost.com>
* MM-68762: Add Postgres migrations for discoverable private channels
Three online-safe migrations introduce the schema that supports the
Discoverable Private Channels feature (PRs 2-5 of MM-68430 will land
behind it):
- 000175 adds Channels.Discoverable BOOLEAN NOT NULL DEFAULT FALSE.
Metadata-only on Postgres >= 11; no table rewrite.
- 000176 creates a partial index on
(TeamId) WHERE Discoverable AND Type='P' AND DeleteAt=0
using CREATE INDEX CONCURRENTLY (-- morph:nontransactional) so the
build never blocks writes on the populated Channels table.
- 000177 creates the ChannelJoinRequests table with three indexes, the
important one being the partial unique index on (ChannelId, UserId)
WHERE Status = 'pending'. That keeps the full audit history intact
while still enforcing at-most-one active pending request per
(channel, user).
Co-authored-by: Ibrahim Serdar Acikgoz <isacikgoz@users.noreply.github.com>
* MM-68762: Add FeatureFlagDiscoverableChannels (default false)
Gates the per-channel Discoverable toggle and the channel-join-request
flow. Default-OFF so all PRs in the MM-68430 series can land on master
without exposing partial UX.
Co-authored-by: Ibrahim Serdar Acikgoz <isacikgoz@users.noreply.github.com>
* MM-68762: Add Discoverable + ChannelJoinRequest models
- Channel gains a Discoverable bool, ChannelPatch a *bool, both serialized
as 'discoverable'. Patch() applies it, Auditable() logs it, and IsValid()
rejects Discoverable=true on any non-private channel so a misconfigured
patch can never produce a public discoverable channel.
- New ChannelJoinRequest type captures the per-row state of a non-member's
request: pending -> approved | denied | withdrawn. Rows are append-only
with reviewer and timestamps so the table is also the audit trail.
IsValid() enforces:
* recognized status,
* Message and DenialReason rune limits,
* DenialReason only on denied rows (no orphan reasons),
* reviewer + reviewed_at present for any terminal review (approved /
denied) but not for self-service withdrawal.
- Two new WebSocket event constants -- channel_join_request_created and
channel_join_request_updated -- that later PRs broadcast on the admin
queue and the requester's My Pending Requests panel.
Unit tests cover Patch(), the new IsValid() rule on Discoverable, the
PreSave/PreUpdate timestamp behavior on ChannelJoinRequest, and every
IsValid branch including the reviewer-required-on-review invariant.
Co-authored-by: Ibrahim Serdar Acikgoz <isacikgoz@users.noreply.github.com>
* MM-68762: Add discoverable-channel permissions
Two new channel-scoped permissions, each independently rebindable from
the System Console:
- manage_private_channel_discoverability gates the per-channel toggle so
admins can restrict who can flip discoverability without also handing
out manage_private_channel_properties.
- manage_channel_join_requests gates the queue list / approve / deny /
count endpoints (added in PR 2).
Both are added to the channel_admin role bootstrap so new deployments
get them by default, and a new permissions migration
(add_discoverable_channel_permissions) grants them to channel_admin,
team_admin and system_admin scheme roles on existing deployments.
Co-authored-by: Ibrahim Serdar Acikgoz <isacikgoz@users.noreply.github.com>
* MM-68762: Add ChannelJoinRequestStore and wire Discoverable into channel store
- channelSliceColumns / channelToSlice / updateChannelT now include the
new Discoverable column so Save() and Update() round-trip the field.
Existing select paths inherit the column automatically because every
read goes through channelSliceColumns.
- New ChannelJoinRequestStore interface and SQL implementation:
Save / Get / GetPendingForChannelAndUser / GetForChannel / GetForUser
/ Update / CountPending. Save translates the
idx_channeljoinrequests_pending_unique partial unique index violation
into store.ErrConflict so the app layer (PR 2) can return 409 without
re-parsing pq errors.
- Storetest suite at storetest/channel_join_request_store.go is invoked
from sqlstore via the existing StoreTest harness; covers insert /
partial-unique conflict / re-insert after withdrawal / NotFound /
status filtering / pagination with TotalCount / Update / CountPending.
- Mocks and retrylayer / timerlayer are regenerated via make store-mocks
and go generate ./channels/store -- no hand-written generator output.
Co-authored-by: Ibrahim Serdar Acikgoz <isacikgoz@users.noreply.github.com>
* MM-68762: Add TS types for Discoverable channels + join requests
webapp/platform/types:
- Channel.discoverable?: boolean alongside existing policy_enforced /
policy_is_active so the web client sees the same wire shape the server
emits.
- ChannelJoinRequest, ChannelJoinRequestStatus, ChannelJoinRequestList,
GetChannelJoinRequestsOptions for the API contract surfaced in PR 2.
webapp/platform/client:
- WebSocketEvents enum gains ChannelJoinRequestCreated and
ChannelJoinRequestUpdated so PR 3 can hang WS handlers off them
without redeclaring constants.
These are model-only updates with no UI consumer yet; PR 3 introduces
the toggle, request flow, and admin queue surfaces.
Co-authored-by: Ibrahim Serdar Acikgoz <isacikgoz@users.noreply.github.com>
* MM-68762: Split ChannelJoinRequests indexes into concurrent migrations
The mattermost-govet concurrentIndex lint check enforces CREATE INDEX
CONCURRENTLY on every CREATE INDEX statement, even on an empty
freshly-created table where it would be a no-op. The original 000177
file inlined three CREATE INDEX statements; that failed check-style.
Mirror the convention used by 000166_create_views +
000167_create_views_channel_id_delete_at_index: keep the CREATE TABLE
in its own (transactional) file, and move each index into a separate
nontransactional file that runs CREATE INDEX CONCURRENTLY. Verified
locally against Postgres 15 that all four new migrations apply in
order and the storetest suite (partial unique constraint + paged
list + count) still passes.
Co-authored-by: Ibrahim Serdar Acikgoz <isacikgoz@users.noreply.github.com>
* MM-68762: Wire new permission migration into test fixtures
Two CI test surfaces missed when the channel_admin role and the
permission-migration list gained the new
manage_private_channel_discoverability and manage_channel_join_requests
entries:
- testlib/store.go: the shared mocked SystemStore used by
SetupWithStoreMock / SetupEnterpriseWithStoreMock needs an explicit
GetByName expectation for every migration key (because the mock
panics on unexpected calls). Add the new
MigrationKeyAddDiscoverableChannelPermissions key so
TestCreateOrUpdateAccessControlPolicy, the elasticsearch
aggregation_job_test, and every other mock-store test stop panicking
on server bootstrap.
- cmd/mmctl/commands/permissions_test.go: TestResetPermissionsCmd
hard-codes the channel_admin default permission list and expects
PatchRole to be called with exactly that slice. Extend the expected
slice with the two new permission ids so the mmctl reset path stays
in sync with the role bootstrap.
Co-authored-by: Ibrahim Serdar Acikgoz <isacikgoz@users.noreply.github.com>
* MM-68762: Register new idx_channels_discoverable_team in TestGetSchemaDefinition
The schema-dump test asserts an exact index count and definition map
for the channels table. Migration 000176 added
idx_channels_discoverable_team — a partial btree on (teamid) gated by
discoverable=true AND type='P' AND deleteat=0. Bump the expected count
from 12 to 13 and add the index's CREATE INDEX definition as produced
by pg_indexes (note: type is cast to channel_type, the existing
domain). Verified locally against Postgres 15.
Co-authored-by: Ibrahim Serdar Acikgoz <isacikgoz@users.noreply.github.com>
* MM-68762: Fix golangci-lint findings in ChannelJoinRequest store
Two golangci-lint findings on the freshly-added files:
- sqlstore/channel_join_request_store.go:133 (modernize): collapse the
'if page < 0 { page = 0 }' clamp into max(opts.Page, 0).
- storetest/channel_join_request_store.go:243 (govet shadow): the
inner Save loop redeclared err with :=, shadowing the outer err
captured from the first CountPending call. Switch to plain
assignment so the same err is reused.
Verified locally with golangci-lint v2.11.4 across public/...,
channels/app/..., channels/store/..., channels/testlib/... and
cmd/mmctl/commands/... — 0 issues.
Co-authored-by: Ibrahim Serdar Acikgoz <isacikgoz@users.noreply.github.com>
* MM-68762: Sync channel_admin bootstrap with TestDoAdvancedPermissionsMigration
app_test.go pins the exact list of permissions the channel_admin role
is expected to hold after DoAdvancedPermissionsMigration completes.
The role bootstrap in role.go grew two entries
(manage_private_channel_discoverability and manage_channel_join_requests),
so the test's expected slice needs the same two entries appended in
the same order, otherwise assert.Equal fails on slice ordering.
This is the same class of fix as the mmctl/permissions_test.go change
in a previous commit -- two parallel test fixtures encode the
channel_admin defaults and have to be updated in lockstep with the
bootstrap.
Co-authored-by: Ibrahim Serdar Acikgoz <isacikgoz@users.noreply.github.com>
* MM-68762: Add English translations for new model error keys
12 keys were emitted by the new Discoverable + ChannelJoinRequest
validation paths but had no en.json entry, which trips i18n-check on
CI. Add the missing entries with one-line English copy that mirrors
adjacent model errors (Invalid <field>., Create at must be a valid
time., etc.). The new entries are:
- model.channel.is_valid.discoverable.app_error
- model.channel_join_request.is_valid.channel_id.app_error
- model.channel_join_request.is_valid.create_at.app_error
- model.channel_join_request.is_valid.denial_reason.app_error
- model.channel_join_request.is_valid.denial_reason_status.app_error
- model.channel_join_request.is_valid.id.app_error
- model.channel_join_request.is_valid.message.app_error
- model.channel_join_request.is_valid.reviewed_by.app_error
- model.channel_join_request.is_valid.reviewer.app_error
- model.channel_join_request.is_valid.status.app_error
- model.channel_join_request.is_valid.update_at.app_error
- model.channel_join_request.is_valid.user_id.app_error
Generated through 'make i18n-extract'; verified clean with
'make i18n-check'. Per the workspace rule, only en.json was modified --
no other locale files.
Co-authored-by: Ibrahim Serdar Acikgoz <isacikgoz@users.noreply.github.com>
* MM-68762: Address CodeRabbit review: stable pagination + redact denial reason from audit log
Two production-code findings from CodeRabbit on the freshly-added
ChannelJoinRequest server code:
- sqlstore/channel_join_request_store.go (GetForChannel / GetForUser):
OrderBy("CreateAt DESC") alone is unstable when two rows share a
millisecond (NewId is monotonic-ish but CreateAt is millisecond
resolution), so offset paging could duplicate or skip rows between
pages. Add Id DESC as a deterministic tie-breaker on both list
queries.
- model/channel_join_request.Auditable: the denial reason is admin-typed
free text and could carry sensitive content. Mirror the existing
has_message pattern by emitting has_denial_reason as a boolean
presence flag instead of the raw value. Reviewer id, review timestamp,
and status are still logged, so the audit trail keeps every piece
needed for compliance review.
Co-authored-by: Ibrahim Serdar Acikgoz <isacikgoz@users.noreply.github.com>
* MM-68762: Tighten model tests per CodeRabbit review
Two test-only findings from CodeRabbit:
- TestChannelJoinRequestPreUpdateAdvancesUpdateAt previously asserted
GreaterOrEqual(r.UpdateAt, originalCreate). Because validRequest
initialises UpdateAt to GetMillis() (same call site as CreateAt), a
no-op PreUpdate would still pass that check. Seed r.UpdateAt = 1
before calling PreUpdate() and assert Greater(r.UpdateAt, int64(1))
so any regression that drops the GetMillis assignment fails the test.
- TestChannelIsValidDiscoverable did not cover ChannelTypeGroup. Add the
case alongside ChannelTypeOpen and ChannelTypeDirect so the contract
that 'only ChannelTypePrivate accepts Discoverable=true' is fully
pinned across all four channel types.
Co-authored-by: Ibrahim Serdar Acikgoz <isacikgoz@users.noreply.github.com>
* MM-68762: Mock ChannelJoinRequest accessor in retrylayer test
retrylayer_test.go's genStore() helper mocks every Store() accessor
because retrylayer.New() wraps the entire surface. The new
ChannelJoinRequest() method I added on Store was missing from the
mock, so TestRetry/on_regular_error_should_not_retry panicked with
'Unexpected Method Call ChannelJoinRequest()' on Postgres shard 0.
Add the mock alongside the other accessors. No production code
change.
Co-authored-by: Ibrahim Serdar Acikgoz <isacikgoz@users.noreply.github.com>
---------
Co-authored-by: Cursor Agent <cursoragent@cursor.com>
Co-authored-by: Ibrahim Serdar Acikgoz <isacikgoz@users.noreply.github.com>
* Return descriptive errors from Role.IsValid and Role.IsValidWithoutId
Previously both methods returned bool, leaving callers with no context
about which validation check failed. Now both return error with a
message identifying the specific constraint that was violated.
* Add tests for Role.IsValid and Role.IsValidWithoutId
* Log migration key on doPermissionsMigration failure
---------
Co-authored-by: Mattermost Build <build@mattermost.com>
* Remove QueryRowx (no-timeout) from sqlxDBWrapper; migrate caller to QueryRowX
QueryRowx forwarded to context.Background() with no timeout, while QueryRowX
(uppercase) already enforces the wrapper timeout. Removing the no-timeout
variant eliminates an accidental footgun and migrates the one sqlstore caller
(plugin_store) to the timeout-enforcing method.
* sqlxRow, with timeout cancel after Scan
Introduce sqlxRow, which pairs *sqlx.Row with its context cancel function.
QueryRowX (on both sqlxDBWrapper and sqlxTxWrapper) now returns *sqlxRow;
Scan calls cancel immediately after the row is consumed, releasing the
timeout context as soon as possible rather than waiting for the timer.
* sqlxRows, with timeout cancel on Close
Introduce sqlxRows, which embeds *sqlx.Rows and holds the timeout context's
cancel function. Close() cancels the context immediately after the rows are
done, releasing timeout resources as soon as iteration completes rather than
waiting for the timer to fire.
Introduce rowScanner interface (Next/Scan/Err) so the internal helpers
scanRowsIntoMap and scanRetentionIdsForDeletion accept any row iterator
rather than the concrete *sql.Rows, accommodating the new return types
without threading *sqlxRows through every caller.
* abolish the X suffix altogether
* delete unused NamedQuery
* add timeout tests for Query and QueryRow on db and tx wrappers
* fix tx/timeout tests: handle pq driver.ErrBadConn on killed connection
* fixup! fix tx/timeout tests: handle pq driver.ErrBadConn on killed connection
* override Next() on sqlxRows to cancel on EOF
* rm redundant row.Err(), handled by Scan
* apply timeout to context unless deadline set
* rebind consistently
* address review feedback: rename, trace, and QueryContext fix
- Rename withQueryTimeout → ensureQueryTimeout to better convey that it
respects existing deadlines rather than overriding them.
- Add missing w.trace blocks to QueryRowContext and ExecContext.
- Change QueryContext to use ensureQueryTimeout and return *sqlxRows
(instead of *sql.Rows) so the cancel is deferred to Close/Next
rather than released prematurely.
* fix Beginx → Begin after master merge
* fix golangci-lint inline warning in sqlx_wrapper_test
---------
Co-authored-by: Mattermost Build <build@mattermost.com>
* Simplify invite_people email parsing
Replace backwards in-place mutation loop with a straightforward forward
filter into a new slice. Extract into parseEmailList so the logic can be
unit tested directly.
* MM-68150: Upgrade golangci-lint to v2.12.2
Remove //go:fix inline from NewPointer, which is a generic function not
yet supported by the inline analyzer, and fix 11 slicesbackward
modernize issues flagged by the new version.
* MM-68150: Enable all linters by default; disable those with >20 existing issues
Switch from opt-in (default: none) to opt-out (default: all) so new
linters added to golangci-lint are evaluated automatically. Explicitly
disable every linter that has more than 20 pre-existing violations,
deferring those for later cleanup. Also disable a handful of linters
whose violations are intentional patterns in this codebase (nilerr,
dogsled, sqlclosecheck, iotamixing, predeclared, containedctx, iface,
gocheckcompilerdirectives, promlinter, goprintffuncname, gomoddirectives).
* MM-68150: Fix mirror linter issues
Replace Write([]byte(s)) with WriteString(s), and FindIndex([]byte(s))
with FindStringIndex(s), to avoid unnecessary allocations.
* MM-68150: Fix nosprintfhostport linter issue
Use net.JoinHostPort to construct host:port strings instead of
fmt.Sprintf with a manually formatted pattern.
* MM-68150: Fix rowserrcheck and sqlclosecheck linter issues
Check rows.Err() after iteration loops in schema_dump.go. In the
sqlx_wrapper test, defer rows.Close() rather than closing inline.
* MM-68150: Fix nilnesserr linter issues — wrong variable in error handlers
In 11 places, a stale variable (often the outer err from a prior
assignment) was used instead of the freshly-checked error variable
(appErr, rowErr, jsonErr, writeErr, esErr). Each produces a typed-nil
wrapped in a non-nil interface, silently discarding the real error.
* MM-68150: Add i18n string for app.compile_csv_chunks.write_error
---------
Co-authored-by: Mattermost Build <build@mattermost.com>
* Generalize file backend error types
Replace S3FileBackendAuthError and S3FileBackendNoBucketError with
backend-agnostic FileBackendAuthError and FileBackendNoBucketError so
non-S3 drivers can return them and the admin "Test Connection" flow
keeps surfacing useful messages.
The old S3-prefixed names are kept as type aliases of the generic
types so external code (plugins, historical consumers) continues to
compile, and so existing S3 construction sites stay untouched.
The type switch in connectionTestErrorToAppError now matches the
generic types, with new i18n keys (test_connection_auth.app_error
and test_connection_no_bucket.app_error) whose wording does not name
S3. The old S3-specific i18n keys are dropped via `make i18n-extract`
since they are no longer referenced from code; the api4 test that
asserted on those keys is updated, and the Cypress
`MM-T996 Amazon S3 connection error messaging` spec that asserted
on the old user-facing string is updated to the new wording.
------
AI assisted commit
* Pull in Azure SDK and uuid dependencies
Bring in github.com/Azure/azure-sdk-for-go/sdk/azcore and
.../sdk/storage/azblob (with .../sdk/internal as their indirect
dependency). The two are needed by the upcoming Azure Blob Storage
filestore backend and its lazy-Range-backed reader. The bump of
golang.org/x/{crypto,net,sys,term,text} comes transitively from
azblob's minimum versions.
Also promotes github.com/google/uuid from indirect to direct,
since the Azure backend uses it to generate block IDs that share
the same wire format the SDK itself produces in UploadStream.
------
AI assisted commit
* Add azureRangeReader, a seekable Range-backed blob reader
A small standalone type that satisfies the FileBackend interface's
ReadCloseSeeker + the broader io.ReaderAt contract on top of Azure
Blob Storage HTTP Range requests. Lands as its own commit because
the upcoming Azure FileBackend driver builds on it, and the reader
itself is independently useful — and independently testable against
a fake downloader without standing up an Azure client.
Design notes:
* Read opens an HTTP Range stream lazily at the current offset and
reuses it for sequential reads. Seek to a different offset closes
the open stream; the next Read re-opens it.
* Seek to the same offset is a no-op and does not close the open
stream, so callers like zip.NewReader that probe with redundant
seeks don't kick off a fresh download.
* ReadAt issues a dedicated ranged DownloadStream per call and does
not touch the streaming cursor — matches the io.ReaderAt contract
the bulk-import worker's zip.NewReader path relies on.
* Close cancels the context (which any in-flight Azure call will
observe and abort), stops the deadline timer, and closes the
current body if any. It is safe to call when no body was ever
opened.
* CancelTimeout lets long-running consumers like the import worker
opt out of the per-operation deadline that would otherwise kill
multi-minute downloads partway through.
The implementation talks to a small blobDownloader interface rather
than *blob.Client directly so the unit tests can substitute a fake
downloader that records every requested Range and tracks Close
calls on the bodies it hands out.
------
AI assisted commit
* Add Azure Blob Storage filestore driver
Implements the FileBackend interface against Azure Blob Storage in
a new azurestore.go (~520 LOC). The driver is not yet selectable
via NewFileBackend's switch — that wiring lands in the next commit
together with the admin config surface — but the driver itself is
complete and self-contained behind the FileBackendSettings struct.
Filesstore.go grows three pieces of supporting infrastructure that
the driver consumes:
* a `driverAzure = "azureblob"` constant alongside the existing
driverS3 and driverLocal,
* an Azure-specific block on FileBackendSettings (storage account,
access key, container, path prefix, endpoint, SSL flag, request
timeout),
* a CheckMandatoryAzureFields validator that mirrors
CheckMandatoryS3Fields.
Behavioural notes that warrant calling out:
* Reader returns the previously-added azureRangeReader, so reads
stream lazily over HTTP Range and ReadAt is available for the
bulk-import worker's zip.NewReader path. The deadline timer is
armed before the initial GetProperties call so the HEAD itself
is bounded.
* WriteFile and AppendFile both go through StageBlock +
CommitBlockList via a shared stageBlocks helper, never the SDK's
UploadStream. UploadStream's small-payload fast path falls back
to single-shot PutBlob, which leaves the resulting blob with no
committed block list; a subsequent AppendFile that calls
CommitBlockList on that blob would then clobber its content.
Routing every write through the block-list mechanism keeps
AppendFile correct regardless of payload size.
* AppendFile stages the new chunk as one or more blocks and commits
the existing committed block list plus the newly staged IDs.
The new bytes go up exactly once — no re-download, no
re-concatenate, no re-upload of the prior contents.
* WriteFileContext does not wrap the caller-supplied context with
its own timeout — that timeout is applied in WriteFile only,
matching the S3 driver, so long-running TryWriteFileContext
callers (like message-export bulk writes) opt out of the
per-operation timeout the way the abstraction documents.
Authentication is shared-key only for this drop; Microsoft Entra
ID / managed identity is deferred to a follow-up. The endpoint is
configurable so the same code targets the production Azure host
(vhost style — {account}.blob.core.windows.net) or Azurite /
Azure Government / sovereign clouds (path style —
host[:port]/{account}).
------
AI assisted commit
* Wire Azure backend into config, validation, and driver selection
This commit registers the previously-added AzureFileBackend driver
with the rest of the system. Until now the driver was usable only
via direct construction; after this commit, `DriverName: "azureblob"`
in config.json is a fully-supported deployment configuration.
Five integration sites are touched:
* `newFileBackend` in filesstore.go now dispatches `driverAzure` to
NewAzureFileBackend, alongside the existing s3 and local cases.
NewFileBackendSettingsFromConfig (and its export counterpart) gain
an Azure branch that maps the model.FileSettings fields onto the
Azure-specific FileBackendSettings fields.
* `model.FileSettings` grows the user-facing Azure config schema:
storage account, access key, container, path prefix, endpoint,
SSL flag, request timeout, plus matching Export* fields for the
dedicated export store. SetDefaults populates them so deployments
that never opted into Azure don't carry nil pointers. `isValid`
accepts the new ImageDriverAzure constant.
* `Config.Sanitize()` masks AzureAccessKey and ExportAzureAccessKey
the same way it masks AmazonS3SecretAccessKey, so the shared key
never reaches an API consumer in plain text.
* `desanitize()` restores the masked keys on a config write so a
PATCH that doesn't touch the key doesn't clobber it with the
FakeSetting placeholder.
* `configSensitivePaths` covers both Azure key paths so audit
diffs don't include them either.
* `ConfigToFileBackendSettings` in the `mattermost db` CLI helper
gets the Azure branch its production counterpart already has —
without it, `mattermost db migrate` / `db downgrade` would fail
on Azure-configured deployments with "missing azure storage
account setting".
Finally, the shared FileBackendTestSuite is now wired against
Azurite via TestAzureFileBackendTestSuite, which skips when
CI_AZURITE_HOST is unreachable. The test-infra wiring (the docker
service, the env vars, the start_dependencies entry) landed in a
previous PR; this commit is what makes the suite actually exercise
the Azure driver end to end.
------
AI assisted commit
* Validate Azure timeout and path prefix in Config.IsValid
Parity with the S3-side checks that already cover
AmazonS3RequestTimeoutMilliseconds and AmazonS3PathPrefix. Without
these, a zero/negative AzureRequestTimeoutMilliseconds passes
validation and later creates immediately-expired request contexts,
and leading/trailing whitespace in AzurePathPrefix produces blob
keys that don't match what the admin configured.
Same checks added for the Export* counterparts. The
file_driver.app_error translation is updated to mention the new
'azureblob' option alongside 'local' and 'amazons3'.
------
AI assisted commit
* Stream zip entries from the Azure backend
writeZipEntry was calling ReadFile, which loads the entire blob
into memory before writing it to the archive. For large blobs or
deep directories this spikes RSS or OOMs the goroutine. Switch to
Reader (the streaming azureRangeReader) and io.Copy into the zip
entry so memory stays bounded regardless of blob size.
------
AI assisted commit
* Use a backend-agnostic fallback for FileBackendNoBucketError
The fallback Error() message was "no such bucket", which leaks S3
terminology when an Azure caller returns the type with no wrapped
Err. Use "no such bucket or container" so logs and external error
handling stay neutral across backends.
------
AI assisted commit
* Defend Azure path prefix against directory traversal
Reject ".." in AzurePathPrefix and ExportAzurePathPrefix at config
validation time, since path.Join collapses traversal segments and a
prefix like "../other-tenant" would otherwise escape the configured
isolation boundary.
Harden the prefix helper as a second line of defense: if the joined
path no longer sits inside pathPrefix, fall back to joining the prefix
with the base name of the caller-supplied path. That preserves the
prefix invariant for plugin and import paths that the upload code does
not sanitize uniformly.
------
AI assisted commit
* Honor SkipVerify when constructing the Azure client
FileBackendSettings.SkipVerify is plumbed through from the System Console
the same way it is for S3, so admins toggling the flag for self-signed
endpoints (Azurite, sovereign clouds) get the behavior they expect
without having to drop SSL entirely and send the shared key in clear
text.
------
AI assisted commit
* Warn when the Azure request timeout falls back to its default
Config.IsValid already rejects non-positive AzureRequestTimeoutMilliseconds
for any path that goes through config validation, so this warn only fires
for direct callers that bypass validation (tests, helpers). Logging the
substitution turns a silent coercion into something an operator can
correlate against unexpected request behavior.
------
AI assisted commit
* Cap Azure request timeout at 10 minutes
Reject AzureRequestTimeoutMilliseconds values above the ceiling so an
operator (or someone who has admin access) cannot effectively disable
timeouts by setting the value to math.MaxInt64. A hung Azure call then
holds a goroutine open until the OS gives up.
Applies the same bound to ExportAzureRequestTimeoutMilliseconds. S3 has
the same gap; treating it is out of scope here but worth a follow-up.
------
AI assisted commit
* Refuse AppendFile on blobs without a committed block list
A blob written by another tool (Azure portal, azcopy, a migration script,
a plugin using Put Blob) has its content in the blob but an empty
committed-block list. Committing a new block list against such a blob
silently replaces the existing content with only the appended bytes.
Check the blob's properties before staging when the committed-block list
is empty, and refuse with a clear error if the blob has content. Same
hazard for an admin pointing the backend at an existing container with
pre-existing files.
Adds an integration test against Azurite to lock the behavior in.
------
AI assisted commit
* Surface truncated reads from azureRangeReader
Read closed the body cleanly and returned io.EOF even when the remote
stream terminated before the blob's content length. Callers (and any
retry layer above) then accepted a partial blob as complete.
ReadAt unconditionally rewrote io.ErrUnexpectedEOF to io.EOF, which made
truncated downloads indistinguishable from clean reads. That is exactly
what zip.NewReader consumes for archive readers, so the bulk-import
worker would silently import partial archives.
Read now closes the body, nils it, and returns io.ErrUnexpectedEOF when
EOF arrives before offset reaches size. ReadAt only collapses
ErrUnexpectedEOF to EOF when the full count was delivered and the stream
was consumed to the end of the blob. Otherwise the truncation
propagates with context.
Both code paths are exercised by new fakeDownloader-backed tests.
------
AI assisted commit
* Move container provisioning out of Azure TestConnection
Auto-creating the container inside TestConnection meant a typo in the
System Console (mattermosst instead of mattermost) silently provisioned
an unwanted container in the admin's Azure subscription, with no audit
log and no warning. They'd discover it later when uploads landed
somewhere unexpected.
TestConnection now returns FileBackendNoBucketError when the container
is missing, mirroring the S3 contract. A new MakeContainer method
mirrors S3FileBackend.MakeBucket, and Server.Start dispatches via two
capability interfaces (bucketMaker / containerMaker) instead of a hard
S3 type assertion — so the NoBucket error is no longer silently
swallowed for backends Server.Start has not been taught about.
------
AI assisted commit
* Carry file backend auth detail through to AppError
The Test Connection button collapsed every typed backend failure into
the same generic i18n message. Operators trying to debug bad credentials
or a missing bucket only saw "Unable to authenticate against the file
storage backend" with no SDK code to grep for in their logs.
Use errors.As so the typed checks survive future wrapping, and pass the
underlying error string through the NewAppError details argument. The
AppError serializer surfaces that detail to the admin console alongside
the translated message, so a bad S3 InvalidAccessKeyId or an Azure
AuthenticationFailed shows up in the toast without an i18n schema
change.
------
AI assisted commit
* Remove non-ascii characters from comments
------
AI assisted commit
* Make linter happy
------
AI assisted commit
* Harden Azure prefix boundary check
strings.HasPrefix on the joined path is a string-level check, not a
path-level one, so a configured prefix of "mattermost" accepts a joined
result of "mattermost-evil/...". A crafted caller path like
"../mattermost-evil/secrets" would collapse via path.Join to that exact
sibling and slip through the boundary check, escaping the configured
prefix scope.
Require the joined path to be the cleaned prefix itself or to start with
the prefix followed by a path separator. The fallback path.Join uses the
same cleaned prefix for consistency.
------
AI assisted commit
* Provision Azurite container in standalone test setup
The shared FileBackendTestSuite's SetupTest already handles a missing
container by detecting FileBackendNoBucketError from TestConnection and
calling MakeContainer, but TestAzureFileBackendAppendRefusesNonBlockBlob
bypasses SetupTest and calls TestConnection directly. On a fresh Azurite
instance the test would fail before exercising the append-refusal logic.
Extract a newAzuriteBackend(t) helper alongside azuriteSettings(t) that
builds the backend and ensures the container exists, mirroring the
suite's setup. Use errors.As for forward compatibility with future
wrapping.
------
AI assisted commit
* Fix grammar in email-settings i18n string
"Email settings has unset values." -> "Email settings have unset values."
------
AI assisted commit
* Make Azure MakeContainer idempotent
Treat a ContainerAlreadyExists response as success so that two nodes
racing through TestConnection plus MakeContainer at boot both converge
instead of having the loser fail. Mirrors how the S3 backend handles
the equivalent BucketAlreadyOwnedByYou case.
------
AI assisted commit
* Narrow AzureEndpoint comment to path-style only
The setting only builds path-style URLs, so it cannot reach sovereign
clouds like Azure Government or Azure China, which require vhost-style
endpoints. Update the comment to reflect what the code actually does
and document that sovereign-cloud support is out of scope.
------
AI assisted commit
unified-ci-app[bot]