Skip to content

Remove explicit credentials configuration for S3 client - #56

Closed
will2hew wants to merge 1 commit into
docmost:mainfrom
will2hew:will/s3-credentials
Closed

will2hew wants to merge 1 commit into
docmost:mainfrom
will2hew:will/s3-credentials

Conversation

@will2hew

@will2hew will2hew commented Jul 5, 2024 •

Copy link
Copy Markdown
Contributor

By explicitly providing the credentials configuration block, we were overwriting the AWS Node SDK's built in environment variable parsing. This meant that when using short lived tokens, which uses AWS_SESSION_TOKEN as well as AWS_ACCESS_KEY_ID and AWS_SECRET_ACCESS_KEY that it was failing, as it would see the access key and the access key ID, but not a valid session.

By removing it, we let the Node SDK natively get the credentials. This works with both the legacy (and not recommended) long-term credentials, providing both the access key and ID, but also with the more secure recommended credentials that utilise a session token.

The docs will need to be updated to reference AWS_ACCESS_KEY_ID and AWS_SECRET_ACCESS_KEY rather than AWS_S3_SECRET_ACCESS_KEY and AWS_S3_SECRET_ACCESS_KEY

Closes #50

@Philipinho

Copy link
Copy Markdown
Member

#71 closes this.

@Philipinho Philipinho closed this Jul 7, 2024
restyler pushed a commit to restyler/docmost that referenced this pull request Dec 24, 2025
feat: email filtering for members in the add "space/group members" modal.
vvzvlad pushed a commit to vvzvlad/gitmost that referenced this pull request Jun 20, 2026
…t#56

Keep the backlog focused on deferred TESTS; the related non-test gaps
(model-allow-list, restriction-cache invalidation, server embed-recursion
guard, collectPageEmbeds cycle guard, jest DI/lib0-ESM debt) are now
tracked as issues docmost#52-docmost#56 and only linked from the backlog.

Co-Authored-By: Claude Opus 4.8 <[email protected]>
vvzvlad pushed a commit to vvzvlad/gitmost that referenced this pull request Jun 20, 2026
vvzvlad pushed a commit to vvzvlad/gitmost that referenced this pull request Jun 20, 2026
Add .github/workflows/test.yml (pnpm + Node 22): on pull_request and push
to develop it installs, builds @docmost/editor-ext and runs `pnpm -r test`
across all packages (server Jest, client Vitest, editor-ext Vitest,
packages/mcp node:test). So tests now run automatically in CI, not just
on demand.

To make the run green, quarantine the 16 pre-existing stock NestJS
`should be defined` scaffold specs via jest `testPathIgnorePatterns` —
they never compiled (missing DI providers / lib0 ESM) and assert nothing
useful. Tracked for a proper fix/removal in issue docmost#56. Verified each
pattern drops only its scaffold (46 of 62 suites still collected) and the
full `pnpm -r test` is green: server 587, client 185, editor-ext 56,
mcp 247.

Co-Authored-By: Claude Opus 4.8 <[email protected]>
vvzvlad pushed a commit to vvzvlad/gitmost that referenced this pull request Jun 21, 2026
…ocmost#56)

16 suites were disabled via testPathIgnorePatterns due to two root causes: lib0
ESM not transformed (the @hocuspocus/server -> lib0/decoding.js chain) and stock
'should be defined' specs built via Test.createTestingModule without providers.
Add lib0 to transformIgnorePatterns; convert the 14 DI placeholders to direct
new X(...) instantiation with stub deps (keeping a real construct smoke test);
re-enable the suites. Also updates the public-share limiter test to assert the
fail-closed behavior from docmost#62 (surfaced now that the suite runs). Full server
suite: 67 passed, 689 tests.

Co-Authored-By: Claude Opus 4.8 <[email protected]>
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.

Utilise the AWS client's native credential parsing

2 participants