Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 29 additions & 0 deletions nginx/nginx.conf
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,35 @@ server {
proxy_read_timeout 300s;
}

# Canonical invite URL, used by the Top.gg listing, the README CTA and
# every tracked campaign link. nginx only proxies /api, so this path fell
# through to the SPA, whose catch-all bounces unknown paths to the landing
# page — every invite click was silently swallowed (#1888).
#
# Proxied to the backend rather than redirected here, because
# backend/src/routes/invite.ts logs the utm_* parameters before redirecting.
# A `return 302` at the edge would fix the click and still lose the
# attribution that .agents/tracking-urls.md is built around (#1894).
location = /invite {
add_header X-Frame-Options DENY always;
add_header X-Content-Type-Options nosniff always;
add_header Referrer-Policy strict-origin-when-cross-origin always;

# nosemgrep: generic.nginx.security.missing-internal.missing-internal
# The rule wants `internal;`, which would make this location
# unreachable from outside — this is a public entry point by design,
# exactly like the /api and / blocks below. Its SSRF concern does not
# apply: the upstream is a fixed literal with no request-derived input
# in the URL, so a client cannot steer where the proxy connects.
set $invite_upstream http://backend:3000;
proxy_pass $invite_upstream;
Comment thread
github-advanced-security[bot] marked this conversation as resolved.
Fixed
proxy_http_version 1.1;
proxy_set_header Host $host;
proxy_set_header X-Real-IP $remote_addr;
proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for;
proxy_set_header X-Forwarded-Proto $proxy_x_forwarded_proto;
}

location /api {
set $backend_upstream http://backend:3000;
proxy_pass $backend_upstream;
Expand Down
8 changes: 6 additions & 2 deletions packages/backend/src/routes/invite.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,14 @@
import type { Express, Request, Response } from 'express'
import { infoLog } from '@lucky/shared/utils'
import { logAndSwallow } from '@lucky/shared/utils/error'
import { buildBotInviteUrl } from '@lucky/shared/constants'
import { apiLimiter } from '../middleware/rateLimit'

const DISCORD_INVITE_URL =
'https://discord.com/oauth2/authorize?client_id=962198089161134131&scope=bot%20applications.commands&permissions=36970496'
// Was a hardcoded permissions=36970496 (Manage Messages, Connect, Speak) —
// over-scoped on message deletion and missing View Channels / Send Messages,
// so it neither matched the landing page nor actually worked. Single source of
// truth now lives in shared (#1894).
const DISCORD_INVITE_URL = buildBotInviteUrl()

function toUtmString(value: unknown): string | undefined {
return typeof value === 'string' ? value : undefined
Expand Down
29 changes: 14 additions & 15 deletions packages/frontend/src/lib/discord.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
import {
BOT_CLIENT_ID,
BOT_INVITE_PERMISSIONS,
buildBotInviteUrl,
} from '@lucky/shared/constants'

// Discord CDN URL helpers. The backend returns avatar/icon HASHES, not full
// URLs — build the CDN URL before passing to an <img>.

Expand All @@ -9,25 +15,18 @@
return `https://cdn.discordapp.com/icons/${guildId}/${iconHash}.png?size=64`
}

// Minimal permission set (View Channels, Send Messages, Embed Links, Connect,
// Speak) — matches the /invite command. NOT Administrator: requesting only what
// the bot needs is safer and converts better with server admins, and the public
// listings state that Lucky asks for no admin permission.
export const BOT_INVITE_PERMISSIONS = '3165184'

// Public Discord Application ID (a.k.a. client_id). Safe to ship: it appears in
// every OAuth invite link and is not a secret. Used as the default so the CTA
// works out of the box; override via VITE_DISCORD_CLIENT_ID for a fork.
const DEFAULT_DISCORD_CLIENT_ID = '962198089161134131'

/**
* The one invite URL. Both the landing CTA and the docs page must use this:
* they previously built their own, drifted, and the docs copy shipped
* `permissions=8` (Administrator) while the landing page and every public
* listing promised the minimal set (#1888).
* listing promised the minimal set (#1888). The permission set and URL shape
* now come from shared, which the backend `/invite` redirect uses too (#1894).
*
* VITE_DISCORD_CLIENT_ID overrides the application id for a fork.
*/
export function getBotInviteUrl(): string {
const clientId =
import.meta.env.VITE_DISCORD_CLIENT_ID || DEFAULT_DISCORD_CLIENT_ID
return `https://discord.com/oauth2/authorize?client_id=${clientId}&scope=bot%20applications.commands&permissions=${BOT_INVITE_PERMISSIONS}`
const clientId = import.meta.env.VITE_DISCORD_CLIENT_ID || BOT_CLIENT_ID
return buildBotInviteUrl(clientId)
}

export { BOT_INVITE_PERMISSIONS }

Check warning on line 32 in packages/frontend/src/lib/discord.ts

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Use `export…from` to re-export `BOT_INVITE_PERMISSIONS`.

See more on https://sonarcloud.io/project/issues?id=LucasSantana-Dev_Lucky&issues=AZ-jtyHC9PSKVKj2pwYS&open=AZ-jtyHC9PSKVKj2pwYS&pullRequest=1893
5 changes: 5 additions & 0 deletions packages/shared/src/constants/index.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
export { COLOR, type ColorKey } from './colors'
export { API_ROUTES } from './apiRoutes'
export { BOT_STATS_MEMBERS_KEY, BOT_STATS_TTL_SECONDS } from './stats'
export {
BOT_CLIENT_ID,
BOT_INVITE_PERMISSIONS,
buildBotInviteUrl,
} from './invite'
export {
TOP_GG_BOT_ID,
TOP_GG_VOTE_TIERS,
Expand Down
59 changes: 59 additions & 0 deletions packages/shared/src/constants/invite.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
import { describe, expect, it } from '@jest/globals'
import {
BOT_CLIENT_ID,
BOT_INVITE_PERMISSIONS,
buildBotInviteUrl,
} from './invite'

// Discord permission bits, for readable assertions.
const ADMINISTRATOR = 1n << 3n
const MANAGE_MESSAGES = 1n << 13n
const VIEW_CHANNEL = 1n << 10n
const SEND_MESSAGES = 1n << 11n
const EMBED_LINKS = 1n << 14n
const CONNECT = 1n << 20n
const SPEAK = 1n << 21n

describe('bot invite permissions', () => {
const bits = BigInt(BOT_INVITE_PERMISSIONS)

// The public listings state Lucky asks for no admin permission. Three
// copies of this value had already drifted: the docs page shipped
// Administrator and the backend redirect shipped Manage Messages (#1888,
// #1894), so this is a claim worth pinning rather than a style preference.
it('never requests Administrator', () => {
expect(bits & ADMINISTRATOR).toBe(0n)
})

it('never requests Manage Messages', () => {
expect(bits & MANAGE_MESSAGES).toBe(0n)
})

it('requests exactly the permissions the bot needs to function', () => {
const expected =
VIEW_CHANNEL | SEND_MESSAGES | EMBED_LINKS | CONNECT | SPEAK
expect(bits).toBe(expected)
})
})

describe('buildBotInviteUrl', () => {
it('targets the Discord OAuth endpoint with the shared permission set', () => {
expect(buildBotInviteUrl()).toBe(
`https://discord.com/oauth2/authorize?client_id=${BOT_CLIENT_ID}` +
`&scope=bot%20applications.commands&permissions=${BOT_INVITE_PERMISSIONS}`,
)
})

// Without applications.commands the bot joins but registers no slash
// commands, which is the shape of the failure that got the Top.gg
// verification rejected (#1885).
it('requests the applications.commands scope', () => {
expect(buildBotInviteUrl()).toContain(
'scope=bot%20applications.commands',
)
})

it('accepts a fork client id', () => {
expect(buildBotInviteUrl('123')).toContain('client_id=123')
})
})
28 changes: 28 additions & 0 deletions packages/shared/src/constants/invite.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
import { TOP_GG_BOT_ID } from './topgg'

/**
* Minimal permission set for the bot invite: View Channels, Send Messages,
* Embed Links, Connect, Speak.
*
* NOT Administrator, and deliberately no Manage Messages. The public listings
* (Top.gg, README) state that Lucky only asks for what it needs, so this value
* is a promise we make to server admins, not a preference.
*
* This lives in shared because three copies had already drifted apart (#1888,
* #1894): the docs page shipped `8` (Administrator), the backend redirect
* shipped `36970496` (Manage Messages + Connect + Speak, and missing View
* Channels / Send Messages entirely), and only the landing page was correct.
*/
export const BOT_INVITE_PERMISSIONS = '3165184'

/** The Discord application id. Public: it appears in every OAuth invite URL. */
export const BOT_CLIENT_ID = TOP_GG_BOT_ID

/**
* The canonical Discord OAuth invite URL.
*
* @param clientId - Override for forks running their own application.
*/
export function buildBotInviteUrl(clientId: string = BOT_CLIENT_ID): string {
return `https://discord.com/oauth2/authorize?client_id=${clientId}&scope=bot%20applications.commands&permissions=${BOT_INVITE_PERMISSIONS}`
}
12 changes: 9 additions & 3 deletions vercel.json
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,14 @@
"key": "Permissions-Policy",
"value": "camera=(), microphone=(), geolocation=(), interest-cohort=()"
},
{ "key": "X-Frame-Options", "value": "DENY" },
{ "key": "X-Content-Type-Options", "value": "nosniff" },
{
"key": "X-Frame-Options",
"value": "DENY"
},
{
"key": "X-Content-Type-Options",
"value": "nosniff"
},
{
"key": "Referrer-Policy",
"value": "strict-origin-when-cross-origin"
Expand All @@ -31,7 +37,7 @@
"redirects": [
{
"source": "/invite",
"destination": "https://discord.com/oauth2/authorize?client_id=962198089161134131&scope=bot%20applications.commands&permissions=3165184",
"destination": "https://lucky-api.lucassantana.tech/invite",
"permanent": false
}
],
Expand Down
Loading