Skip to content

fix: decode legacy v6 sealed sessions on unseal#479

Merged
gjtorikian merged 2 commits intomainfrom
add-v6-preservation
May 6, 2026
Merged

fix: decode legacy v6 sealed sessions on unseal#479
gjtorikian merged 2 commits intomainfrom
add-v6-preservation

Conversation

@gjtorikian
Copy link
Copy Markdown
Contributor

Summary

  • Adds a v6 fallback path to WorkOS::Encryptors::AesGcm#unseal so cookies sealed by the old encryptor-gem format still decrypt after the v7 upgrade.
  • Without this, every existing browser cookie would fail to unseal after upgrading and users would be silently logged out — v6 used the raw passphrase as the AES key with no version prefix; v7 prepends a version byte and SHA-256-derives the key.
  • Fallback re-raises the original v7 error when both decoders fail, so genuinely malformed payloads still surface accurate errors.
  • Adds unit coverage in test_encryptors_aes_gcm.rb and an end-to-end SessionManager#authenticate test exercising a v6-sealed cookie.

Test plan

  • bundle exec rake test TEST=test/workos/test_encryptors_aes_gcm.rb
  • bundle exec rake test TEST=test/workos/test_session.rb
  • Manually verify a session cookie sealed by the previous SDK version still authenticates against this build.

Upgrading from the `encryptor` gem (v6) to the in-tree AES-GCM
sealer (v7) changed the on-the-wire payload: v7 prepends a
version byte and derives the key via SHA-256, while v6 used the
raw passphrase as the key with no version prefix. Without a
fallback, every existing browser cookie would fail to unseal
after upgrade, silently logging users out.

Unseal now tries v7 first and falls back to the v6 layout on
failure, re-raising the original v7 error if both fail so error
messages stay accurate for genuinely malformed payloads.
@gjtorikian gjtorikian requested review from a team as code owners May 6, 2026 06:06
@gjtorikian gjtorikian requested a review from dandorman May 6, 2026 06:06
The v6 decoding fix in 126a713 had no regression coverage. This
test exercises the full path — loading a v6-sealed cookie,
refreshing against the API, and verifying the resealed session
uses the current format — so future changes to the seal/unseal
logic cannot silently break v6 compatibility.
@gjtorikian gjtorikian merged commit 1d8b4aa into main May 6, 2026
7 checks passed
@gjtorikian gjtorikian deleted the add-v6-preservation branch May 6, 2026 06:08
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 6, 2026

Greptile Summary

This PR adds a v6 legacy fallback path to WorkOS::Encryptors::AesGcm#unseal so that browser cookies sealed by the old encryptor-gem format (raw passphrase as AES key, no version byte) continue to decrypt after the v7 upgrade, preventing users from being silently logged out.

  • unseal now attempts v7 decryption first via the extracted decode_v7 private method; on failure it falls back to decode_old (v6 layout), re-raising the original v7 error only when both paths fail.
  • Unit tests in test_encryptors_aes_gcm.rb verify the v6 roundtrip, and two new end-to-end tests in test_session.rb exercise SessionManager#authenticate and #refresh against v6-sealed cookies.

Confidence Score: 4/5

Safe to merge; the fallback is well-isolated and existing error contracts are preserved.

The core logic change is small and well-tested. The fallback correctly tries v6 decryption on any v7 failure and re-raises the original error when both attempts fail. The only open question is whether the v6 encryptor gem silently truncated passphrases longer than 32 bytes — if it did, decode_old would fail to decrypt those valid v6 cookies with no hint to the caller.

lib/workos/encryptors/aes_gcm.rb — specifically the raw-passphrase key assignment in decode_old and its behaviour when the passphrase length differs from 32 bytes.

Important Files Changed

Filename Overview
lib/workos/encryptors/aes_gcm.rb Adds a v6 fallback path in unseal via decode_old; refactors inline decryption into decode_v7 and parse_decoded private methods; moves private keyword up.
test/workos/test_encryptors_aes_gcm.rb Adds test_unseal_reads_legacy_v6_payload and a private legacy_v6_seal helper that mirrors the v6 encryptor format.
test/workos/test_session.rb Adds two end-to-end tests (authenticate and refresh) exercising v6-sealed cookies through SessionManager; includes a duplicate legacy_v6_seal helper also present in test_encryptors_aes_gcm.rb.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[unseal sealed, key] --> B[raw = Base64.decode64 sealed]
    B --> C[decode_v7 raw, key]
    C --> D{Version byte == 0x01?}
    D -->|No| E[raise ArgumentError: Unknown seal version]
    D -->|Yes| F[Derive key via SHA-256]
    F --> G[AES-256-GCM decrypt]
    G --> H{Decryption OK?}
    H -->|Yes| I[parse_decoded]
    I --> J[Return Hash / String]
    H -->|No| K[Rescue: try decode_old]
    E --> K
    K --> L[decode_old raw, key]
    L --> M[AES-256-GCM decrypt raw passphrase as key]
    M --> N{Decryption OK?}
    N -->|Yes| I
    N -->|No| O[Re-raise original_error from decode_v7]
Loading

Reviews (1): Last reviewed commit: "test(session): Cover legacy v6 sealed se..." | Re-trigger Greptile

tag = encrypted.byteslice(encrypted.bytesize - 16, 16)

cipher = OpenSSL::Cipher.new("aes-256-gcm").decrypt
cipher.key = key.to_s
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 decode_old passes the raw passphrase string directly to OpenSSL without checking its length. AES-256-GCM requires exactly 32 bytes; if the cookie password is longer or shorter, OpenSSL raises OpenSSL::Cipher::CipherError, which unseal's rescue silently swaps for the original v7 error message. If the v6 encryptor gem silently truncated longer passphrases to 32 bytes, valid v6 cookies sealed with a longer password would fail to decrypt here with no useful diagnostic. Adding an explicit comment (or guard) clarifying that key.to_s must already be 32 bytes would make this constraint visible.

Comment on lines +445 to 454
def legacy_v6_seal(data, key)
cipher = OpenSSL::Cipher.new("aes-256-gcm").encrypt
iv = SecureRandom.random_bytes(12)
cipher.key = key
cipher.iv = iv
ciphertext = cipher.update(JSON.generate(data)) + cipher.final

Base64.encode64(iv + ciphertext + cipher.auth_tag)
end
end
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 The legacy_v6_seal helper is defined identically in both test_encryptors_aes_gcm.rb and test_session.rb. Duplicating it means any future change to the v6 seal logic (e.g., to match a newly discovered variant) must be made in two places. Consider extracting it to a shared test helper module (e.g., test/support/legacy_seal_helpers.rb) and including it in both test classes.

Suggested change
def legacy_v6_seal(data, key)
cipher = OpenSSL::Cipher.new("aes-256-gcm").encrypt
iv = SecureRandom.random_bytes(12)
cipher.key = key
cipher.iv = iv
ciphertext = cipher.update(JSON.generate(data)) + cipher.final
Base64.encode64(iv + ciphertext + cipher.auth_tag)
end
end
# Consider moving this to a shared test helper module (e.g. test/support/legacy_seal_helpers.rb)
# so it stays in sync with the identical copy in test_encryptors_aes_gcm.rb.
def legacy_v6_seal(data, key)
cipher = OpenSSL::Cipher.new("aes-256-gcm").encrypt
iv = SecureRandom.random_bytes(12)
cipher.key = key
cipher.iv = iv
ciphertext = cipher.update(JSON.generate(data)) + cipher.final
Base64.encode64(iv + ciphertext + cipher.auth_tag)
end
end

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant