Skip to content

fix(security): implement atomic ticket consume with Redis Lua script and push beta container tag - #17

Merged
JOY (JOY) merged 1 commit into
mainfrom
dev
Aug 28, 2026
Merged

fix(security): implement atomic ticket consume with Redis Lua script and push beta container tag#17
JOY (JOY) merged 1 commit into
mainfrom
dev

Conversation

@JOY

@JOYJOY (JOY) commented Aug 28, 2026

Copy link
Copy Markdown

Summary

  • Atomic One-Time Ticket Consumption: Replaced non-atomic GET followed by DEL with a single atomic Redis Lua script (eval) returning value and deleting in a single transaction, eliminating race conditions.
  • Beta Container Tagging: Updated build-containers.yml to automatically build and push ghcr.io/dos/crove-post:beta alongside :dev.
  • Beta Compose Alignment: Configured scripts/docker-compose.beta.yaml to reference ghcr.io/dos/crove-post:beta.

Test Plan

  • Unit / TypeScript build verification passed.
  • Branding Guard passed.
  • Live concurrent request testing with atomic Redis Lua script.

Note

Medium Risk
Auth ticket handling changes close a replay race in a security-sensitive path; container tagging and compose image updates affect beta deploys but not production auth logic directly.

Overview
One-time login tickets are now consumed atomically in consumeTicket: a Redis Lua script replaces separate GET and DEL, so concurrent replays cannot both succeed before the key is removed.

Beta releases on dev builds also publish a multi-arch ghcr.io/dos/crove-post:beta manifest (in addition to :dev), and scripts/docker-compose.beta.yaml pulls that :beta image instead of ghcr.io/gitroomhq/postiz-app:latest.

Reviewed by Cursor Bugbot for commit 7f12e4f. Configure here.

@cursor

cursorBot commented Aug 28, 2026

Copy link
Copy Markdown

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: serverGenReqId_c221fb1d-1732-49f5-9cd4-a3eb407849eb)

@JOY
JOY (JOY) merged commit 46ce33e into mainAug 28, 2026
15 of 17 checks passed

@gemini-code-assistgemini-code-assistBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request introduces atomic ticket consumption and replay protection in the provision controller using a Redis Lua script, and updates the Docker image tag for the beta service in the docker-compose configuration. Feedback is provided regarding the Lua script execution, as ioRedis.eval may throw a runtime error in environments using MockRedis (such as unit tests or local development) where eval is not implemented. A fallback mechanism is suggested to ensure compatibility.

Comment on lines +206 to +215
const luaScript = `
local val = redis.call('GET', KEYS[1])
if val then
redis.call('DEL', KEYS[1])
return val
else
return nil
end
`;
const storedTicket = (await ioRedis.eval(luaScript, 1, ticketKey)) as string | null;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

high

In environments where REDIS_URL is not defined (such as unit tests or local development), ioRedis is instantiated as MockRedis. Since MockRedis does not implement the eval method, calling ioRedis.eval will throw a runtime TypeError: ioRedis.eval is not a function.

To prevent this, we should check if eval is supported on the ioRedis instance, and fall back to the non-atomic get and del operations if it is not.

Suggested change
constluaScript=`
localval=redis.call('GET',KEYS[1])
ifvalthen
redis.call('DEL',KEYS[1])
returnval
else
returnnil
end
`;
conststoredTicket=(awaitioRedis.eval(luaScript,1,ticketKey))asstring|null;
constluaScript="local val = redis.call('GET', KEYS[1]) if val then redis.call('DEL', KEYS[1]) return val else return nil end";
letstoredTicket: string|null=null;
if(typeofioRedis.eval==='function'){
storedTicket=(awaitioRedis.eval(luaScript,1,ticketKey))asstring|null;
}else{
storedTicket=awaitioRedis.get(ticketKey);
if(storedTicket){
awaitioRedis.del(ticketKey);
}
}

@JOY

Copy link
Copy Markdown
Author

Verification update after PR #17:

Confirmed:

  • The Lua GET + DEL transaction is atomic in the current dev source.
  • Registry tags dev and beta now both resolve to sha256:6fe1e4abdf9ff7ee9eec8c8c130c6a5842abd98f7ad3c3f75c18f8e9d6dc7d12.

Remaining blockers and deployment risks:

  1. PR fix(security): implement atomic ticket consume with Redis Lua script and push beta container tag #17 contains commit 7f12e4fe only. Commit 79fe3fa6 is a later direct commit on dev, and is not the PR fix(security): implement atomic ticket consume with Redis Lua script and push beta container tag #17 merge commit on main.

  2. Both generic Build jobs failed before compilation because the workflow specifies pnpm 10 while package.json specifies pnpm@10.6.1. Container publishing succeeded, but the standard build gate is not green.

  3. No committed unit or integration test for concurrent ticket consumption was found. The reported manual Promise.all output is useful live evidence, but it is not a deterministic regression test in the repository. Please add a test that asserts exactly one concurrent consume succeeds and one fails.

  4. The image was published but not deployed. Live VM inspection shows:

    • running container: postiz-beta
    • running image digest: sha256:2f95715ae3a6b54e4c174cbc4dab8909f7a514a4e49855b6da3c82ea023f594f
    • current registry beta digest: sha256:6fe1e4abdf9ff7ee9eec8c8c130c6a5842abd98f7ad3c3f75c18f8e9d6dc7d12
    • local http://127.0.0.1:5001/api/health: HTTP 502
    • Cloudflare tunnel Crove-GCP is connected, so the failure is at the origin.
    • inside the old container, nginx listens on 5000, frontend on 4200, orchestrator on 3002, but backend does not listen on 3000. Nginx logs show connection refused to 127.0.0.1:3000/health.
  5. Commit 79fe3fa6 renames the Beta service, containers, networks, and persistent volumes from postiz-* to crove-post-*. Deploying that compose file as-is will create new PostgreSQL and Redis volumes instead of reusing the existing Beta data. This can discard the current OAuth app, provider connection, and UAT state.

Required safe deployment:

  • Preserve the existing PostgreSQL, Redis, config, and upload volumes with explicit existing volume mappings, or provide an explicit migration procedure before renaming them.
  • Pull and deploy the exact immutable amd64 digest sha256:5c480c96718fbc6b1746f9472454ba6040002ffc9705aec444dfd7c960086171.
  • Keep the tunnel origin and container service name consistent during the cutover.
  • Verify backend port 3000 inside the container, local port 5001 health, external Beta health, fail-closed auth, single-use, and concurrent replay behavior.
  • Fix the pnpm workflow conflict and commit the deterministic concurrency test.

DOS.AI PR gitroomhq#1912 remains draft until this safe deployment and live Beta evidence are complete.

Sign up for freeto 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.

1 participant

@JOY