Skip to content

chore: maintenance pass — unbreak CI, refresh dependencies, fix SAS signing - #331

Open
AlexanderYW wants to merge 4 commits into
masterfrom
chore/maintenance-2026
Open

chore: maintenance pass — unbreak CI, refresh dependencies, fix SAS signing#331
AlexanderYW wants to merge 4 commits into
masterfrom
chore/maintenance-2026

Conversation

@AlexanderYW

@AlexanderYW AlexanderYW commented Sep 1, 2026

Copy link
Copy Markdown
Owner

Context

The package has been frozen since April 2023. The driver itself is small and sound, but everything around it aged out: CI no longer runs, several dev tools are unmaintained, and one latent bug was about to become a real one.

Scope stays on AdonisJS v5. v6 replaced Drive with flydrive and removed the @ioc:Adonis/Core/Drive binding this package extends, so v6 support is a rewrite, not a bump — tracked separately.

The functional bug

generateBlobSASURL recovered its signing credential by reaching into (blockBlobClient as any).pipeline.factories and taking the last entry. In @azure/storage-blob 12.33 pipeline is protected and marked @internal, while StorageClient now exposes a public readonly credential. This backs getSignedUrl, copy and move.

The old lookup also failed silently: with a token (Entra ID) or anonymous credential it handed generateBlobSASQueryParameters an object that could not sign, producing a bogus URL. It now raises E_CANNOT_GENERATE_SAS naming the config needed.

Also fixed

  • azureContainer was declared non-optional but only assigned in the else branch — undefined whenever connection_string was set.
  • CannotDeleteContainerException reported E_CANNOT_CREATE_CONTAINER. Behavioural change: anyone matching on that code for the delete path will see the corrected E_CANNOT_DELETE_CONTAINER.
  • Deprecated global unescape() in getUrl → decodeURIComponent, with a fallback since it throws on malformed input.
  • Stray console.log(error) in putStream.
  • instructions.md had name/key mapped backwards (name: Env.get('AZURE_KEY')). name is the account name, key is the access key. Following the docs gave you auth failures that were hard to trace. Verified the corrected mapping end to end against Azurite; the suite missed it because it authenticates with a connection string.

Dependencies

from to
@azure/storage-blob ^12.13.0 ^12.33.0
@azure/identity ^3.1.3 ^4.13.0
TypeScript 4.9 5.9
ESLint 8 (eslintrc) 10 (flat config)
Prettier 2 3

@poppinss/utils is deliberately held at ^5 — @adonisjs/core v5 pins that line, and Exception.invoke() was replaced by a constructor in v6/v7.

Dropped: eslint-plugin-adonis (unpublished since Oct 2022, pinned to eslint ^8, blocked the upgrade), mrm/@adonisjs/mrm-preset (tsconfig.json extended a path inside node_modules, so the compiler config was unreadable without installing first — now inlined), and got (unused).

CI

CodeQL was failing outright — codeql-action@v2 was disabled by GitHub. The test matrix targeted Node 16/17/18, all EOL, on checkout@v2 and setup-node@v1.

  • Node matrix → 20/22/24; actions → checkout@v5 / setup-node@v5 with npm caching; azurite → 3.37.0; CodeQL → v3
  • npm-publish.yml was missing cp .env.example .env, so the pre-publish gate ran the suite with no credentials — it was not actually testing anything before publishing
  • Publish now uses --provenance; workflow permissions scoped explicitly
  • Dependabot: daily → weekly with grouped dev bumps (daily + ungrouped left 14 stale branches on the remote), and now tracks github-actions

Verification

All run locally against a real Azurite emulator:

  • npm run lint, npx tsc --noEmit, npm run build — clean
  • npm test — 33/33 pass (31 before; added a getSignedUrl group covering both the signed-URL happy path and the new error)
  • Shared-key auth path (name + key + local_address) driven end to end — put/get/copy/move/signed URL/stats
  • The compiled output in build/ loaded and exercised as a consumer would get it
  • npm pack --dry-run — published file list unchanged

Known remaining advisories

npm audit still reports issues, all dev-only and unfixable in scope: the @adonisjs/core@5 chain (bodyparser, http-server, cookie, file-type) only clears by moving to AdonisJS v7, which is the whole v6 question; and tmp via np, where the "fix" is downgrading np to 2.x. Runtime dependencies are clean.

Follow-ups (not in this PR)

  • Migrate japa 4 → @japa/runner (japa 4 is unmaintained; no advisories, and the migration fights @adonisjs/require-ts's CJS loader for no user-visible gain)
  • Decide the AdonisJS v6/v7 story — flydrive-based driver as a major, or a sibling package
  • Delete the 14 superseded dependabot/* branches

🤖 Generated with Claude Code

AlexanderYW and others added 4 commits September 1, 2026 21:45
@AlexanderYW @claude
chore: modernise the toolchain and refresh dependencies
30470c1 
The package has been frozen since April 2023 and every dev tool was one to
two majors behind, with several no longer maintained at all.

Runtime dependencies:
  - @azure/storage-blob ^12.13.0 -> ^12.33.0
  - @azure/identity     ^3.1.3   -> ^4.13.0
  - @poppinss/utils held at ^5: @adonisjs/core v5 pins that line, and
    Exception.invoke() (used by every custom exception here) was replaced
    by a constructor in v6/v7.

Dev dependencies:
  - TypeScript 4.9 -> 5.9, @types/node 18 -> 22
  - ESLint 8 -> 10 with a flat config, Prettier 2 -> 3, np 7 -> 12
  - Drop eslint-plugin-adonis: unpublished since Oct 2022 and pinned to
    eslint ^8, which blocked the ESLint upgrade. The flat config keeps an
    equivalent rule set via typescript-eslint.
  - Drop mrm/@adonisjs/mrm-preset and inline the resolved compilerOptions.
    tsconfig.json previously extended a path *inside* node_modules, so the
    compiler config could not be read without installing first.
  - Drop got: unused by both test/ and test-helpers/.

Also adds an engines field (node >=18) and a `test:azurite` script, since
nothing in the repo started the emulator that `npm test` requires.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@AlexanderYW @claude
fix: sign SAS URLs via the supported credential API
b4c3be6 
generateBlobSASURL recovered the signing credential by reaching into
`(blockBlobClient as any).pipeline.factories` and taking the last entry.
In @azure/storage-blob 12.33 `pipeline` is protected and marked @internal,
while StorageClient now exposes a public readonly `credential` — use that.

The old lookup also failed silently: with a token (Entra ID) or anonymous
credential it handed generateBlobSASQueryParameters an object that could
not sign, producing a bogus URL. It now throws E_CANNOT_GENERATE_SAS with
a message naming the config needed. getSignedUrl re-throws it unwrapped,
since a missing shared key is a configuration problem rather than a
metadata lookup failure.

This path backs getSignedUrl, copy and move, all covered by the suite.

Other fixes in this change:
  - azureContainer was declared non-optional but only assigned in the
    else branch, so it was undefined whenever connection_string was set.
  - CannotDeleteContainerException reported E_CANNOT_CREATE_CONTAINER.
  - Replace the deprecated global unescape() in getUrl with
    decodeURIComponent (the correct inverse of the SDK's encoding),
    falling back to the raw URL since it throws on malformed input.
  - Drop a stray console.log(error) from putStream's catch.
  - Remove tslint pragmas; the repo has used eslint for years.

Adds a getSignedUrl group covering both the signed-URL happy path and the
new error, taking the suite to 33 tests.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@AlexanderYW @claude
ci: get the workflows running again
26cfa52 
CodeQL has been failing outright: github/codeql-action@v2 was disabled by
GitHub, so the workflow could not start. The test matrix was also targeting
Node 16/17/18 — all end-of-life — via actions/checkout@v2 and the long
deprecated actions/setup-node@v1.

  test-package.yml
    - Node matrix 16/17/18 -> 20/22/24, fail-fast disabled
    - checkout@v2 -> v5, setup-node@v1 -> v5 with npm caching
    - azurite 3.23.0 -> 3.37.0

  npm-publish.yml
    - Node 14 -> 22, same action bumps
    - Add the missing `cp .env.example .env` to the build job. Without it
      the pre-publish gate ran the suite with no credentials configured,
      so it was not actually testing anything before publishing.
    - Publish with --provenance and the id-token permission
    - Drop the commented-out publish-gpr job

  codeql-analysis.yml
    - codeql-action v2 -> v3, javascript -> javascript-typescript
    - Drop the legacy `git checkout HEAD^2` pull request workaround
    - Scope permissions explicitly

  dependabot.yml
    - daily -> weekly, and group dev dependency bumps. Daily updates with
      no grouping left 14 stale branches on the remote.
    - Track github-actions too, so the above cannot silently rot again.

The devcontainer was pinned to Node 14 and azurite 3.23.0 and used the
`settings`/`extensions` keys that moved under `customizations.vscode` years
ago. It also started a second azurite inside the app container, which
already shares a network namespace with the storage service on :10000.

Adds .gitattributes to normalise line endings; the two devcontainer files
were CRLF despite .editorconfig asking for lf, which is why they show as
fully rewritten here.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@AlexanderYW @claude
docs: fix the config docs, badges and missing contributing guide
e2630c3 
instructions.md mapped the shared key credentials the wrong way round:

    name: Env.get('AZURE_KEY'),    // AZURE_KEY holds the account *key*
    key:  Env.get('AZURE_SECRET'), // AZURE_SECRET held "devstoreaccount1"

`name` is the storage account name and `key` is its access key — that is the
order StorageSharedKeyCredential takes. Anyone following the docs got
authentication failures that are hard to trace back to the config. Verified
the corrected mapping end to end against Azurite (put/get/copy/move/signed
URL). The test suite never caught this because it authenticates with a
connection string.

.env.example is renamed to AZURE_ACCOUNT_NAME / AZURE_ACCOUNT_KEY, since
AZURE_KEY / AZURE_SECRET is what made the mix-up easy in the first place.

instructions.md also said the disk "will be using the gcs driver", and gained
notes on the two documented limitations: SAS signing needs a shared key, and
visibility is a container-level setting in Azure so get/setVisibility throw.

README:
  - Both GitHub badges used shields' `github/workflow/status` endpoint, which
    was removed in 2022 and rendered as "invalid". Moved to
    `github/actions/workflow/status`.
  - The Snyk badge pointed at adonisjs/drive-gcs, a different project.
  - license-url pointed at LICENSE.md; the file is LICENSE.
  - It linked a CONTRIBUTING.md that never existed — now written, covering
    the Azurite-backed test setup that was previously undocumented.
  - Added installation, quick start, a compatibility table stating plainly
    that 5.x does not work on AdonisJS v6, and the caveats. The README was
    badges-only, with everything real hidden in instructions.md.

Also drops config.json (@adonisjs/mrm-preset metadata, dead now that mrm is
gone, and inaccurate: it claimed minNodeVersion 14.17.0 and
runGhActionsOnWindows) and corrects the drive-gcs copyright header in
test-helpers plus the leftover GCS/bucket wording in adonis-typings.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@AlexanderYW