refactor(slack): single source of truth for app manifest scopes

[amikofalvy]

Summary

• Eliminates divergent bot scope definitions — slack-app-manifest.json, oauth.ts, and setup-slack-dev.ts all had independent scope lists that had drifted (oauth.ts missing 5 scopes, setup-slack-dev.ts missing 8 scopes vs manifest)
• Manifest JSON is now the single source of truth — new slack-scopes.ts module exports scopes from the manifest; oauth.ts imports from it; setup-slack-dev.ts reads from the parsed manifest
• Auto-detects manifest drift on re-run — setup-slack-dev.ts now stores the full applied manifest in .slack-dev.json, diffs on re-run, and shows developers exactly what changed
• Auto-triggers OAuth re-install when scopes change — stores installedBotScopes in .slack-dev.json and compares against the manifest on re-run
• Adds slack-manifest skill — agentic documentation so AI coding agents know to edit only slack-app-manifest.json when modifying scopes, events, or commands
• Typed dev config — replaces Record<string, string> with a typed SlackDevConfig interface

Files changed

File	Change
packages/agents-work-apps/tsconfig.json	Add resolveJsonModule: true
packages/agents-work-apps/src/slack/slack-scopes.ts	New — exports BOT_SCOPES and BOT_SCOPES_CSV from manifest
packages/agents-work-apps/src/slack/routes/oauth.ts	Replace hardcoded scope array with BOT_SCOPES_CSV import
scripts/setup-slack-dev.ts	Typed config, manifest-derived scopes, diff on re-run, scope drift detection
.agents/skills/slack-manifest/SKILL.md	New — skill for AI agents modifying Slack app config

Test plan

• pnpm typecheck passes (16/16 tasks)
• Rebased on latest main (resolved conflicts in oauth.ts and setup-slack-dev.ts)
• Run setup-slack-dev.ts fresh (delete .slack-dev.json) — verify scopes come from manifest
• Run setup-slack-dev.ts again with no changes — verify "up to date" message
• Modify a slash command description in manifest, re-run — verify diff shown
• Add a scope to manifest, re-run — verify diff shown AND OAuth re-install triggered

🤖 Generated with Claude Code

[changeset-bot]

🦋 Changeset detected

Latest commit: 9b79185

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 10 packages

Name	Type
@inkeep/agents-work-apps	Patch
@inkeep/agents-api	Patch
@inkeep/agents-manage-ui	Patch
@inkeep/agents-cli	Patch
@inkeep/agents-core	Patch
@inkeep/agents-email	Patch
@inkeep/agents-mcp	Patch
@inkeep/agents-sdk	Patch
@inkeep/ai-sdk-provider	Patch
@inkeep/create-agents	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
agents-api	Ready	Preview, Comment	Mar 14, 2026 2:23am
agents-docs	Ready	Preview, Comment	Mar 14, 2026 2:23am
agents-manage-ui	Ready	Preview, Comment	Mar 14, 2026 2:23am

[claude]

PR Review Summary

(0) Total Issues | Risk: Low

This PR successfully consolidates Slack OAuth bot scopes from three divergent sources into a single source of truth: slack-app-manifest.json. The implementation is clean, follows existing codebase patterns, and eliminates the drift risk that led to unwanted im:* scopes in the OAuth flow.

🟡 Minor (0) 🟡

No issues meet the Minor threshold. All findings are cosmetic improvements in the "Consider" category.

💭 Consider (3) 💭

Inline Comments:

• 💭 Consider: slack-scopes.ts:3 Add defensive access for clearer error messages on manifest structure issues
• 💭 Consider: setup-slack-dev.ts:458-460 Log the actual parse error when .slack-dev.json is corrupted
• 💭 Consider: setup-slack-dev.ts:762-764 Log the actual token rotation error instead of assuming expiration

🧹 While You're Here (0) 🧹

No pre-existing issues surfaced during review.

---

✅ APPROVE

Summary: This is a well-executed refactor that achieves its stated goal of eliminating scope drift. The code is clean, follows existing patterns (JSON import syntax, constant naming conventions, tsconfig settings match sibling packages), and the enhanced setup-slack-dev.ts provides valuable developer feedback on manifest changes and scope drift. The inline suggestions are minor quality-of-life improvements for error messages — none block approval. Ship it! 🚀

Discarded (12)

Location	Issue	Reason Discarded
slack-scopes.ts:3	BOT_SCOPES is readonly string[] not literal type	Design tradeoff — literal types would duplicate manifest data, conflicting with single-source-of-truth goal
setup-slack-dev.ts:267-292	SlackDevConfig has all optional fields	Script uses sequential flow that guarantees fields are set; discriminated union adds complexity for a dev tool
setup-slack-dev.ts:353-361	loadManifest() uses type assertion	Script already checks file existence before calling; full Zod validation would be over-engineering for dev tool
setup-slack-dev.ts:472-477	ManifestDiff interface not discriminated union	Cosmetic type refinement; current implementation handles all cases correctly
setup-slack-dev.ts:454-461	loadDevConfig() uses type assertion without validation	Graceful fallback to {} already exists; additional validation adds complexity
setup-slack-dev.ts:353-362	loadManifest() doesn't catch file read errors separately	Pre-check with existsSync(MANIFEST_PATH) mitigates this; file unreadable is rare edge case
setup-slack-dev.ts:891-895	Non-null assertions on clientId/clientSecret	Code flow guarantees these are set (either from existing config or createApp response)
setup-slack-dev.ts:364-366	getManifestBotScopes returns empty array silently	Intentional graceful degradation; zero scopes would fail at Slack OAuth with clear error
setup-slack-dev.ts:346-351	openBrowser fires and forgets	Dev tool context; browser not opening triggers 120s timeout with clear message; also prints URL
slack/index.ts	New module not re-exported from barrel	Intentional — scopes are package-internal; setup script reads manifest directly
tsconfig.json	resolveJsonModule change	Informational — follows established pattern in 6+ other packages
Multiple	JSON import pattern, constant naming	Informational — all follow existing conventions

Reviewers (5)

Reviewer	Returned	Main Findings	Consider	While You're Here	Inline Comments	Pending Recs	Discarded
pr-review-errors	7	0	0	0	3	0	4
pr-review-types	5	0	0	0	0	0	5
pr-review-consistency	5	0	0	0	0	0	5
pr-review-devops	4	0	0	0	0	0	4
pr-review-standards	0	0	0	0	0	0	0
Total	21	0	0	0	3	0	18

Note: pr-review-standards found no issues. The 3 inline comments are from pr-review-errors findings that were routed as Consider-level suggestions.

[claude]

💭 Consider: Add defensive access for clearer error messages

Issue: Direct property access at module load time produces cryptic errors if the manifest structure changes.

Why: If oauth_config.scopes.bot is missing/renamed, the error is Cannot read properties of undefined with no indication of which file or field is wrong. Since this is version-controlled and would fail CI immediately, the risk is low — but the error message improvement costs little.

Fix: Optional defensive access:

const botScopes = manifest?.oauth_config?.scopes?.bot;
if (!Array.isArray(botScopes)) {
  throw new Error(
    'Invalid slack-app-manifest.json: oauth_config.scopes.bot must be an array'
  );
}
export const BOT_SCOPES: readonly string[] = botScopes;

Refs:

• slack-app-manifest.json:48-60

[claude]

💭 Consider: Log the actual parse error for easier debugging

Issue: When .slack-dev.json is corrupted, the warning doesn't include what went wrong.

Why: If the JSON has a specific issue (trailing comma, encoding problem), users can't diagnose it without manually inspecting the file.

Fix:

Suggested change

	} catch {
	console.log(fmt.warn('Warning: .slack-dev.json is corrupted, starting fresh.'));
	return {};
	} catch (err) {
	const errMsg = err instanceof Error ? err.message : String(err);
	console.log(fmt.warn(`Warning: .slack-dev.json is corrupted (${errMsg}), starting fresh.`));

Refs:

• Similar error-with-context pattern used in loadManifest():357-360

[github-actions]

This pull request has been automatically marked as stale because it has not had recent activity.
It will be closed in 7 days if no further activity occurs.

If this PR is still relevant:

• Rebase it on the latest main branch
• Add a comment explaining its current status
• Request a review if it's ready

Thank you for your contributions!

[amikofalvy]

Rebased on latest main

This PR is still highly relevant — the scope divergence problem has gotten worse since it was opened:

Scope	oauth.ts (hardcoded)	manifest.json	setup-slack-dev.ts (hardcoded)
channels:join	missing	present	missing
files:read	missing	present	missing
mpim:* (3 scopes)	missing	present	missing
search:* (3 scopes)	present	present	missing

PRs #2366 and #2585 each updated some files but not all — exactly the drift pattern this PR prevents.

After this merge, all three locations derive scopes from slack-app-manifest.json via slack-scopes.ts, and setup-slack-dev.ts auto-detects scope drift on re-run.

• Rebased on current main (was 3 weeks behind)
• Resolved conflicts in oauth.ts and setup-slack-dev.ts
• pnpm typecheck passes (16/16 tasks)

[claude]

PR Review Summary

(0) Total Issues | Risk: Low

This PR successfully consolidates Slack OAuth bot scopes from three divergent sources into a single source of truth: slack-app-manifest.json. The implementation is clean, follows existing codebase patterns, and eliminates the drift risk that led to scope inconsistencies across oauth.ts, setup-slack-dev.ts, and the manifest.

🔴❗ Critical (0) ❗🔴

None.

🟠⚠️ Major (0) 🟠⚠️

None.

🟡 Minor (0) 🟡

None.

💭 Consider (0) 💭

None — no new findings.

🕐 Pending Recommendations (3)

Prior review suggestions that remain applicable:

• 💭 slack-scopes.ts:3 Add defensive access for clearer error messages on manifest structure issues
• 💭 setup-slack-dev.ts:459 Log the actual parse error when .slack-dev.json is corrupted
• 💭 setup-slack-dev.ts:763 Log the actual token rotation error instead of assuming expiration

These remain "Consider" level suggestions — they improve error message clarity but don't block approval.

---

✅ APPROVE

Summary: This is a well-executed refactor that achieves its stated goal of eliminating scope drift. The rebased code is clean, all 5 dispatched reviewers found zero new issues, and the enhanced setup-slack-dev.ts provides valuable developer feedback on manifest changes and scope drift detection. The author's comment highlighting that scope divergence has gotten worse since the PR was opened (with specific examples of PRs #2366 and #2585 updating some files but not all) underscores the value of this consolidation. Ship it! 🚀

Discarded (0)

No findings were discarded.

Reviewers (5)

Reviewer	Returned	Main Findings	Consider	While You're Here	Inline Comments	Pending Recs	Discarded
pr-review-standards	0	0	0	0	0	0	0
pr-review-consistency	0	0	0	0	0	0	0
pr-review-types	0	0	0	0	0	0	0
pr-review-devops	0	0	0	0	0	0	0
pr-review-errors	0	0	0	0	0	0	0
Total	0	0	0	0	0	0	0

Note: All reviewers confirmed the code is clean. The 3 pending recommendations are from the prior review run (2026-02-21) and remain valid but non-blocking.

[itoqa]

Ito Test Report ❌

24 test cases ran. 22 passed, 2 failed.

🔍 Verification confirmed broad OAuth flow correctness, auth boundary behavior, and setup script behavior across the run. Two tenant-encoding edge cases remain real defects in callback redirect path construction, where unsanitized tenant values are interpolated into the Manage UI path and can alter routing/query semantics.

✅ Passed (22)

Test Case	Summary	Timestamp	Screenshot
ROUTE-1	Install redirect returns Slack OAuth URL with required non-empty parameters.	54:03
ROUTE-2	OAuth scope payload matches manifest-derived bot scope set.	2:00
ROUTE-3	Valid state + Slack error redirects to tenant Slack dashboard path.	16:53
ROUTE-4	Valid state without code redirects with error=no_code.	18:14
ROUTE-5	Missing state is rejected with error=invalid_state.	19:22
ROUTE-6	Malformed state is rejected and redirected safely.	20:16
EDGE-1	Omitted tenant_id callback stays on expected localhost Slack settings route.	21:26
EDGE-4	Very long tenant value still returns non-5xx install redirect with state.	54:03
ADV-1	Tampered OAuth state signature is blocked.	24:47
ADV-2	Traversal-style tenant payload does not pivot redirect origin.	25:41
ADV-3	XSS-like tenant payload remains inert after redirect.	27:18
ADV-4	Encoded error payload remains a single error query parameter.	54:03
ADV-5	Protected endpoints reject unauthenticated requests while install remains public.	54:03
RAPID-1	Rapid install requests produce distinct OAuth state values.	4:32
RAPID-2	Parallel callback retries complete without crash or 5xx.	54:03
MOBILE-1	Mobile portrait/landscape rendering remains usable with no horizontal overflow.	54:03
UNUSUAL-1	Callback deep-link remains stable through back/forward/refresh.	54:03
MANUAL-1	Setup script derives OAuth scopes from manifest source of truth.	54:03
MANUAL-2	Rerun path avoids reinstall when manifest/scopes are unchanged.	54:03
MANUAL-3	Manifest diff logic reports precise changed paths/values.	54:03
MANUAL-4	Scope drift detection triggers reinstall flow when installed scopes diverge.	54:03
MANUAL-5	Corrupted .slack-dev.json is handled via recovery path instead of crash.	54:03

❌ Failed (2)

Test Case	Summary	Timestamp	Screenshot
EDGE-2	Encoded slash in tenant_id is decoded and splits tenant path segments in callback redirect.	54:03
EDGE-3	Reserved query characters in tenant_id break callback redirect path/query composition.	54:03

Encoded slash in tenant_id does not break redirect integrity – Failed

• Where: Slack OAuth callback handler (/work-apps/slack/oauth_redirect) redirect to Manage UI Slack settings route.

• Steps to reproduce: Generate state from install with tenant_id=org%2Fsub, then call callback with that state and error=access_denied.

• What failed: Redirect path becomes /org/sub/work-apps/slack?... instead of preserving tenant as one logical identifier, causing unstable tenant routing behavior.

• Code analysis: I inspected the OAuth callback path construction in packages/agents-work-apps/src/slack/routes/oauth.ts. The parsed state tenantId is inserted directly into a URL path template without encoding/sanitization before all callback error/success redirects.

• Relevant code:

  packages/agents-work-apps/src/slack/routes/oauth.ts (lines 187-199)

  const parsedState = stateParam ? parseOAuthState(stateParam) : null;
  const tenantId = parsedState?.tenantId || '';
  const dashboardUrl = `${manageUiUrl}/${tenantId}/work-apps/slack`;
  
  if (!stateParam || !parsedState) {
    logger.error({ hasState: !!stateParam }, 'Invalid or missing OAuth state parameter');
    return c.redirect(`${dashboardUrl}?error=invalid_state`);
  }
  
  if (error) {
    logger.error({ error }, 'Slack OAuth error');
    return c.redirect(`${dashboardUrl}?error=${encodeURIComponent(error)}`);
  }

  packages/agents-work-apps/src/slack/routes/oauth.ts (lines 61-66)

  export function createOAuthState(tenantId?: string): string {
    const state: OAuthState = {
      nonce: crypto.randomBytes(16).toString('hex'),
      tenantId: tenantId || '',
      timestamp: Date.now(),
    };

• Why this is likely a bug: The callback builds redirects from an unencoded tenant value, so encoded slashes are interpreted as path separators, which is a deterministic production routing defect rather than test setup noise.

• Introduced by this PR: No – pre-existing bug (code not changed in this PR).

• Timestamp: 54:03

Reserved query characters in tenant_id do not corrupt callback query – Failed

• Where: Slack OAuth callback redirect URL construction for tenant-specific Manage UI route.

• Steps to reproduce: Generate state from install with tenant_id=tenant%3Ffoo%3D1%26bar%3D2, then call callback with that state and error=access_denied.

• What failed: Redirect URL becomes .../tenant?foo=1&bar=2/work-apps/slack?..., so reserved characters in tenant input are interpreted as URL delimiters and corrupt path/query semantics.

• Code analysis: I reviewed the same callback code path and confirmed there is no path-segment encoding (for example encodeURIComponent) or canonical tenant normalization before composing dashboardUrl; all redirect branches reuse that unsafe string.

• Relevant code:

  packages/agents-work-apps/src/slack/routes/oauth.ts (lines 188-190)

  const tenantId = parsedState?.tenantId || '';
  const dashboardUrl = `${manageUiUrl}/${tenantId}/work-apps/slack`;

  packages/agents-work-apps/src/slack/routes/oauth.ts (lines 196-204)

  if (error) {
    logger.error({ error }, 'Slack OAuth error');
    return c.redirect(`${dashboardUrl}?error=${encodeURIComponent(error)}`);
  }
  
  if (!code) {
    logger.error({}, 'No code provided in OAuth callback');
    return c.redirect(`${dashboardUrl}?error=no_code`);
  }

• Why this is likely a bug: Because tenant data is interpolated raw into URL structure, reserved characters can change parsing boundaries, which is a concrete production URL-construction flaw.

• Introduced by this PR: No – pre-existing bug (code not changed in this PR).

• Timestamp: 54:03

📋 View Recording

Screen Recording

[claude]

PR Review Summary

(1) Total Issues | Risk: Low

This delta review covers only the changes since the last automated review: a documentation addition to README.md describing the Slack bot scope management pattern. The documentation is accurate, well-structured, and correctly explains the single-source-of-truth pattern.

🔴❗ Critical (0) ❗🔴

None.

🟠⚠️ Major (0) 🟠⚠️

None.

🟡 Minor (1) 🟡

Inline Comments:

• 🟡 Minor: README.md:196-199 File Structure section missing slack-scopes.ts entry

💭 Consider (0) 💭

None.

🕐 Pending Recommendations (3)

Prior review suggestions that remain applicable:

• 💭 slack-scopes.ts:3 Add defensive access for clearer error messages on manifest structure issues
• 💭 setup-slack-dev.ts:459 Log the actual parse error when .slack-dev.json is corrupted
• 💭 setup-slack-dev.ts:763 Log the actual token rotation error instead of assuming expiration

These remain "Consider" level suggestions — they improve error message clarity but don't block approval.

---

💡 APPROVE WITH SUGGESTIONS

Summary: The documentation addition is solid — it accurately describes the scope management pattern with a clear ASCII diagram and actionable instructions. The only finding is a minor internal consistency issue: the File Structure section should include slack-scopes.ts since it's introduced as a key file in the new section. This is a quick fix (add 1-2 lines to the file listing). The prior "Consider" suggestions from the first review remain valid but non-blocking. Ship it after the minor fix! 🚀

Discarded (0)

No findings were discarded.

Reviewers (2)

Reviewer	Returned	Main Findings	Consider	While You're Here	Inline Comments	Pending Recs	Discarded
pr-review-docs	0	0	0	0	0	0	0
pr-review-consistency	1	0	0	0	1	0	0
Total	1	0	0	0	1	0	0

Note: The 3 pending recommendations are from the prior review run (2026-02-21) and remain valid but non-blocking.

[claude]

PR Review Summary

(1) Total Issues | Risk: Low

This delta review covers only the changes since the last automated review: documentation moved from README.md to AGENTS.md (commit 6c33ef56b). The documentation changes are well-structured and the AGENTS.md placement is appropriate.

🔴❗ Critical (0) ❗🔴

None.

🟠⚠️ Major (0) 🟠⚠️

None.

🟡 Minor (1) 🟡

Inline Comments:

• 🟡 Minor: README.md:170-182 File Structure section missing slack-scopes.ts entry

💭 Consider (0) 💭

None.

🕐 Pending Recommendations (3)

Prior review suggestions that remain applicable:

• 💭 slack-scopes.ts:3 Add defensive access for clearer error messages on manifest structure issues
• 💭 setup-slack-dev.ts:459 Log the actual parse error when .slack-dev.json is corrupted
• 💭 setup-slack-dev.ts:763 Log the actual token rotation error instead of assuming expiration

These remain "Consider" level suggestions — they improve error message clarity but don't block approval.

---

ℹ️ Ito Test Report Note

The Ito test report shows 2 failures (EDGE-2, EDGE-3) related to tenant ID encoding in OAuth callback redirects. Both failures are flagged as pre-existing bugs not introduced by this PR — the affected code path (oauth.ts lines 187-199) wasn't modified in this PR. The tenant ID is interpolated into the redirect URL without proper encoding, which can cause path traversal issues with encoded slashes or reserved characters.

---

💡 APPROVE WITH SUGGESTIONS

Summary: The documentation restructuring is well-executed — moving the detailed scope management guidance to AGENTS.md (the canonical location for AI coding agent instructions) while keeping a brief summary in the README is the right approach. The one minor finding (File Structure section missing slack-scopes.ts) is a quick 1-line fix for internal consistency. The 3 prior "Consider" suggestions remain valid but non-blocking. Ship it after adding slack-scopes.ts to the file listing! 🚀

Discarded (0)

No findings were discarded.

Reviewers (1)

Reviewer	Returned	Main Findings	Consider	While You're Here	Inline Comments	Pending Recs	Discarded
pr-review-docs	1	0	0	0	1	0	0
Total	1	0	0	0	1	0	0

Note: Delta review — only docs reviewer dispatched for documentation-only changes. The 3 pending recommendations are from the initial review run (2026-02-21) and remain valid but non-blocking.

[claude]

PR Review Summary

(0) Total Issues | Risk: Low

This delta review covers only the changes since the last automated review (commit 6c33ef56b): moving Slack scope management documentation from inline locations (AGENTS.md, README.md) to a dedicated skill file (.agents/skills/slack-manifest/SKILL.md).

🔴❗ Critical (0) ❗🔴

None.

🟠⚠️ Major (0) 🟠⚠️

None.

🟡 Minor (0) 🟡

None.

💭 Consider (0) 💭

None.

🕐 Pending Recommendations (4)

Prior review suggestions that remain applicable:

• 💭 slack-scopes.ts:3 Add defensive access for clearer error messages on manifest structure issues
• 💭 setup-slack-dev.ts:459 Log the actual parse error when .slack-dev.json is corrupted
• 💭 setup-slack-dev.ts:763 Log the actual token rotation error instead of assuming expiration
• 🟡 README.md File Structure Add slack-scopes.ts entry to the File Structure section

These remain non-blocking suggestions — they improve error message clarity and documentation completeness but don't block approval.

---

ℹ️ Ito Test Report Note

The Ito test report shows 2 failures (EDGE-2, EDGE-3) related to tenant ID encoding in OAuth callback redirects. Both failures are flagged as pre-existing bugs not introduced by this PR — the affected code path (oauth.ts lines 187-199) wasn't modified in this PR.

---

✅ APPROVE

Summary: This delta is a clean documentation restructuring — moving Slack scope management guidance from inline docs (AGENTS.md, README.md) to a dedicated skill file (.agents/skills/slack-manifest/SKILL.md). The new skill is well-structured with clear trigger keywords, accurate file references, and actionable guidance. Both dispatched reviewers (docs, devops) found zero new issues. The 4 pending recommendations from prior reviews remain valid but non-blocking. Ship it! 🚀

Discarded (0)

No findings were discarded.

Reviewers (2)

Reviewer	Returned	Main Findings	Consider	While You're Here	Inline Comments	Pending Recs	Discarded
pr-review-docs	0	0	0	0	0	0	0
pr-review-devops	0	0	0	0	0	0	0
Total	0	0	0	0	0	0	0

Note: Delta review — only 2 reviewers dispatched for docs/skill-only changes. All reviewers confirmed the changes are clean. The 4 pending recommendations are from prior review runs and remain valid but non-blocking.

[itoqa]

Ito Test Report ❌

16 test cases ran. 15 passed, 1 failed.

🔍 Verification focused on Slack OAuth install/callback behavior and dashboard handling. Most included checks passed end-to-end, and one security defect was confirmed through source inspection: callback redirects build a tenant path from unvalidated state data, allowing tenant-path injection instead of constraining redirects to a safe dashboard route.

✅ Passed (15)

Test Case	Summary	Timestamp	Screenshot
ROUTE-5	Success query rendered workspace confirmation with team name, issued workspace refetch request, and normalized URL to /default/work-apps/slack.	11:47
LOGIC-1	Redirect scope CSV contains all manifest bot scopes including regression-critical scopes channels:join, files:read, mpim:history, mpim:read, and mpim:write.	0:00
LOGIC-2	Scope CSV is well-formed with no duplicates or empty tokens, and URL encoding preserves comma-delimited scope transmission correctly.	0:00
LOGIC-3	Workspace-limit query produced specific one-workspace guidance, removed query params, and did not replay transient toast behavior on reload/back-forward.	12:12
EDGE-1	Direct callback entry resolved to the Slack dashboard without navigation loops, and dashboard interactions remained available after history traversal.	12:27
EDGE-2	Malformed workspace JSON did not crash the page, query params were removed, and the install CTA stayed visible and enabled.	12:39
EDGE-3	workspace_check_failed query produced first-pass handling, URL remained clean after reloads/history navigation, and duplicate transient handling did not recur.	12:58
EDGE-4	On 390x844 viewport, Install to Slack remained visible and usable, and the access_denied cancellation toast was fully visible with successful recovery back to an interactive dashboard.	15:51
ADV-1	Tampered signature was rejected and resolved to invalid_state handling rather than a success path.	1:26
ADV-3	Injected teamName payload remained inert, no script execution occurred, and the dashboard URL cleaned back to /default/work-apps/slack.	16:41
ADV-4	Oversized malformed state produced controlled invalid_state handling (HTTP 302) and browser flow returned to a responsive dashboard without crash.	16:59
ADV-5	Rapid CTA burst produced deterministic Slack OAuth redirect behavior, and returning to the Slack dashboard remained functional without navigation corruption.	17:44
MANUAL-1	Executed setup-slack-dev twice in mock mode; the second run reported app up to date with no manifest diff block.	26:25
MANUAL-2	After a benign manifest description change, setup-slack-dev printed the precise changed path/value and reported Manifest updated.	26:25
MANUAL-3	Scope drift was detected with Added and Removed lists, reinstall flow ran, and installed scope state was updated.	26:25

❌ Failed (1)

Test Case	Summary	Timestamp	Screenshot
ADV-2	Injection payload propagated into callback redirect destination, indicating redirect destination is not constrained to a safe tenant dashboard path.	1:26

tenant_id injection payload cannot hijack redirect destination – Failed

• Where: Slack OAuth callback redirect construction in the backend route.

• Steps to reproduce: Call install with a malicious tenant_id, reuse returned state in /work-apps/slack/oauth_redirect?...&error=access_denied, and inspect the redirect Location.

• What failed: The redirect path is built with attacker-controlled tenant content from OAuth state, instead of enforcing a safe tenant slug/dashboard route.

• Code analysis: The install endpoint accepts arbitrary tenant_id and persists it into signed state without validation. The callback trusts parsed state and directly interpolates tenantId into dashboardUrl, so injected path/script-like payloads propagate into redirect destinations.

• Relevant code:

  packages/agents-work-apps/src/slack/routes/oauth.ts (lines 133-149)

  request: {
    query: z.object({
      tenant_id: z.string().optional(),
    }),
  },
  ...
  const { tenant_id: tenantId } = c.req.valid('query');
  ...
  const state = createOAuthState(tenantId);

  packages/agents-work-apps/src/slack/routes/oauth.ts (lines 61-67)

  export function createOAuthState(tenantId?: string): string {
    const state: OAuthState = {
      nonce: crypto.randomBytes(16).toString('hex'),
      tenantId: tenantId || '',
      timestamp: Date.now(),
    };

  packages/agents-work-apps/src/slack/routes/oauth.ts (lines 187-199)

  const parsedState = stateParam ? parseOAuthState(stateParam) : null;
  const tenantId = parsedState?.tenantId || '';
  const dashboardUrl = `${manageUiUrl}/${tenantId}/work-apps/slack`;
  
  if (!stateParam || !parsedState) {
    logger.error({ hasState: !!stateParam }, 'Invalid or missing OAuth state parameter');
    return c.redirect(`${dashboardUrl}?error=invalid_state`);
  }
  
  if (error) {
    logger.error({ error }, 'Slack OAuth error');
    return c.redirect(`${dashboardUrl}?error=${encodeURIComponent(error)}`);
  }

• Why this is likely a bug: Redirect destinations should be constrained to validated tenant identifiers; directly trusting state-carried tenant input creates an unsafe redirect path construction vulnerability.

• Introduced by this PR: No – pre-existing bug (code not changed in this PR).

• Timestamp: 1:26

📋 View Recording

Screen Recording