Skip to content

feedback: prove who is sending; refuse signed-out and staging - #288

Merged
Fermionic-Lyu merged 4 commits into
mainfrom
feat/feedback-identity
Sep 24, 2026
Merged

Fermionic-Lyu merged 4 commits into
mainfrom
feat/feedback-identity

Conversation

@Fermionic-Lyu

@Fermionic-Lyu Fermionic-Lyu commented Sep 23, 2026 •

Copy link
Copy Markdown
Member

insta feedback against InstaCloud production now proves who is sending, so the report opens a support ticket the team can answer in the console's Support.

  • Signed in (a session or an insta_ key): just before sending, fetch a five-minute assertion from GET /me/feedback-assertion and send it as Insta-User-Assertion. The fetch is bearer-only (evidence: false, as the login /me probe does). Agent evidence would add a POST /agent/sessions that the 5s timeout cannot bound, and its 401s (clock skew, a stale binding) are not fixed by signing in.
  • Signed out: refused before any prompt: insta login first, exit 2, stderr plus {"status":"refused"} for --json. A 401 on the fetch is the same refusal. This cannot reuse the submit-failure path, which exits 0 with "do not retry".
  • Staging: always refused. Its platform signs with keys the feedback service never trusts.
  • Platform unreachable, or anything but 401: send without an assertion and warn. The report is kept; it just opens no ticket.
  • insta-oss / custom hosts: unchanged. No assertion, never refused.
  • An INSTA_ENV typo keeps its --json error object; loading the client moved ahead of buildPayload, so it has its own catch.

Release after the platform deploys (instacloud-platform PR, linked below). Before that, every signed-in report warns and opens no ticket. Docs: instacloud-skills PR, to merge once this is released.

Tests: test/feedback.test.ts, with fakes only (no network, no config writes). Mutation-checked: each refusal, its ordering ahead of validation, the header, evidence: false, the timeout signal, the warning, and the self-host path go red. Full suite 1799 pass.

Platform: https://github.com/InsForge/instacloud-platform/pull/534

🤖 Generated with Claude Code


Summary by cubic

insta feedback against InstaCloud production now proves who is sending, so a report opens a support ticket the team can answer in the console's Support.

  • Signed-in users fetch a five-minute assertion from GET /me/feedback-assertion (bearer-only, evidence: false) and attach it as Insta-User-Assertion.
  • Signed-out and staging reports are refused with exit 2 before any prompting or validation; signed-out stderr suggests running insta login.
  • If the control plane cannot vouch (unreachable or non-401), the report still sends with a warning and no assertion.
  • insta-oss and self-hosted control planes are unchanged.
  • Release after the platform-side signing deploys; until then every report warns and opens no ticket.

Written for commit 124b96e. Summary will update on new commits.

Review in cubic

Fermionic-Lyu and others added 4 commits September 23, 2026 13:31
A cloud report carries a five-minute platform-signed assertion so the
team can reply; signed out, the CLI refuses with insta login (exit 2)
before prompting. insta-oss sends none and is never refused.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Agent evidence made the fetch unbounded by its timeout and turned
clock-skew 401s into a sign-in refusal. Staging's platform signs with
keys the feedback service never trusts, so staging does not send.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@agent-zhang-beihai agent-zhang-beihai Bot 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.

Reviewed by Wang Miao

I'm approving this. On InstaCloud production, insta feedback now asks the control plane for a five-minute token that proves who is sending and attaches it to the report. It refuses with exit 2 when you're signed out, when the token request gets a 401, or when the target is staging. Self-hosted and custom hosts behave as before. I found nothing that blocks the merge.

I checked the platform side of the contract in InsForge/instacloud-platform PR 534. GET /me/feedback-assertion resolves the caller through actor(), and it accepts sessions and insta_ keys. The governance hook lets GET requests through (src/govern/agent-routes.ts:395). So the new request, which sends only the bearer token and no agent evidence, also works for keys minted by agent auth. On an expired session, ApiClient.raw still refreshes the token once, inside the same 5s signal. Only a real 401 after that refresh becomes the sign-in refusal. The INSTA_ENV / readGlobal failure that the old buildPayload catch used to report as a JSON error is still reported that way through inputError. I couldn't run the tests or the typecheck here, because the checkout has no node_modules.

@cubic-dev-ai cubic-dev-ai Bot 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.

3 issues found across 4 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="test/feedback.test.ts">

<violation number="1" location="test/feedback.test.ts:225">
P3: The refusal test mocks `process.stderr.write` but never asserts it, so the documented stderr refusal message (`not signed in to InstaCloud — run insta login...` / staging text) could regress or disappear without this suite noticing. Capture the stderr spy in a variable and assert its content alongside the stdout JSON and exit code 2.</violation>
</file>

<file name="src/index.ts">

<violation number="1" location="src/index.ts:590">
P3: The feedback help still implies that logging in enables reports on every InstaCloud environment, but staging is always refused regardless of session. Say production explicitly and document the staging refusal in both user-facing descriptions.</violation>
</file>

<file name="src/commands/feedback.ts">

<violation number="1" location="src/commands/feedback.ts:272">
P2: `feedback` silently sends an unasserted report when the assertion endpoint returns 2xx without a non-empty `token`, instead of warning for this non-401 failure. Validate the response and throw so the existing catch logs the warning.</violation>
</file>

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread src/commands/feedback.ts
if (env === 'prod') {
try {
// Bearer only: agent evidence adds a session round trip the timeout cannot bound, and 401s signing in cannot fix.
assertion = (await api.request<{ token: string }>('GET', '/me/feedback-assertion', undefined, { evidence: false, signal: AbortSignal.timeout(ASSERTION_TIMEOUT_MS) })).token

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2: feedback silently sends an unasserted report when the assertion endpoint returns 2xx without a non-empty token, instead of warning for this non-401 failure. Validate the response and throw so the existing catch logs the warning.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/commands/feedback.ts, line 272:

<comment>`feedback` silently sends an unasserted report when the assertion endpoint returns 2xx without a non-empty `token`, instead of warning for this non-401 failure. Validate the response and throw so the existing catch logs the warning.</comment>

<file context>
@@ -230,13 +261,22 @@ export async function feedback(opts: FeedbackOpts, deps: FeedbackDeps = {}): Pro
+  if (env === 'prod') {
+    try {
+      // Bearer only: agent evidence adds a session round trip the timeout cannot bound, and 401s signing in cannot fix.
+      assertion = (await api.request<{ token: string }>('GET', '/me/feedback-assertion', undefined, { evidence: false, signal: AbortSignal.timeout(ASSERTION_TIMEOUT_MS) })).token
+    } catch (e) {
+      if (e instanceof ApiError && e.status === 401) refuseFeedback(SIGNED_OUT, opts.json)
</file context>
Suggested change
assertion = (await api.request<{ token: string }>('GET', '/me/feedback-assertion', undefined, { evidence: false, signal: AbortSignal.timeout(ASSERTION_TIMEOUT_MS) })).token
const response = await api.request<{ token?: unknown }>('GET', '/me/feedback-assertion', undefined, { evidence: false, signal: AbortSignal.timeout(ASSERTION_TIMEOUT_MS) })
if (typeof response?.token !== 'string' || response.token.length === 0) throw new Error('feedback assertion response did not include a token')
assertion = response.token

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Declining: GET /me/feedback-assertion is declared with a 200: { token: string } response schema (instacloud-platform#534), so a 2xx without a token is a platform that no longer serves the route as written — no branch for it here.

Comment thread test/feedback.test.ts

it('refuses a signed-out cloud user, and staging, with exit 2, before sending anything', async () => {
const out = vi.spyOn(process.stdout, 'write').mockImplementation(() => true)
vi.spyOn(process.stderr, 'write').mockImplementation(() => true)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P3: The refusal test mocks process.stderr.write but never asserts it, so the documented stderr refusal message (not signed in to InstaCloud — run insta login... / staging text) could regress or disappear without this suite noticing. Capture the stderr spy in a variable and assert its content alongside the stdout JSON and exit code 2.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At test/feedback.test.ts, line 225:

<comment>The refusal test mocks `process.stderr.write` but never asserts it, so the documented stderr refusal message (`not signed in to InstaCloud — run insta login...` / staging text) could regress or disappear without this suite noticing. Capture the stderr spy in a variable and assert its content alongside the stdout JSON and exit code 2.</comment>

<file context>
@@ -180,11 +197,76 @@ describe('feedback command', () => {
+
+  it('refuses a signed-out cloud user, and staging, with exit 2, before sending anything', async () => {
+    const out = vi.spyOn(process.stdout, 'write').mockImplementation(() => true)
+    vi.spyOn(process.stderr, 'write').mockImplementation(() => true)
+    for (const plane of [
+      controlPlane({ signedIn: false }),
</file context>

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Declining: exit 2 plus the {"status":"refused"} object already pin the refusal; the stderr line is written by the shared refuse() in src/util.ts, not by this command.

Comment thread src/index.ts
// ---- feedback (agent + human hurdle reports → the InstaCloud team) ----
program.command('feedback')
.description('Report an InstaCloud-side hurdle (bug / missing feature / friction) to the InstaCloud team — about the insta toolkit itself, NEVER about the app you are building. Works logged-out and unlinked.')
.description('Report an InstaCloud-side hurdle (bug / missing feature / friction) to the InstaCloud team — about the insta toolkit itself, NEVER about the app you are building. On InstaCloud it needs `insta login`, so the team can reply; works unlinked.')

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P3: The feedback help still implies that logging in enables reports on every InstaCloud environment, but staging is always refused regardless of session. Say production explicitly and document the staging refusal in both user-facing descriptions.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/index.ts, line 590:

<comment>The feedback help still implies that logging in enables reports on every InstaCloud environment, but staging is always refused regardless of session. Say production explicitly and document the staging refusal in both user-facing descriptions.</comment>

<file context>
@@ -587,7 +587,7 @@ withSetupAgentOptions(setupCompat.command('agent').description('Alias of `insta
 // ---- feedback (agent + human hurdle reports → the InstaCloud team) ----
 program.command('feedback')
-  .description('Report an InstaCloud-side hurdle (bug / missing feature / friction) to the InstaCloud team — about the insta toolkit itself, NEVER about the app you are building. Works logged-out and unlinked.')
+  .description('Report an InstaCloud-side hurdle (bug / missing feature / friction) to the InstaCloud team — about the insta toolkit itself, NEVER about the app you are building. On InstaCloud it needs `insta login`, so the team can reply; works unlinked.')
   .option('--type <type>', `what kind of hurdle: ${feedbackCmd.TYPES.join(' | ')}`)
   .option('--component <component>', `which part of the toolkit: ${feedbackCmd.COMPONENTS.join(' | ')}`)
</file context>

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Declining: staging is an internal deployment and the refusal names itself when it fires; the help line stays about production.

@agent-zhang-beihai agent-zhang-beihai Bot 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.

Reviewed by Yang Dong

This adds production-signed identity assertions to feedback, refuses signed-out production and staging submissions, and preserves unsigned self-hosted/outage reporting. The authentication and failure paths are sound, so I would approve it with no findings.

@jwfing jwfing left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM - approved.

@Fermionic-Lyu
Fermionic-Lyu merged commit 191882a into main Sep 24, 2026
3 checks passed
Sign up for free to 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.

2 participants