Skip to content

pkg: fix GetLastError function that was returned always nil#2273

Merged
gustavosbarreto merged 1 commit intomasterfrom
fix/getLastErrorNil
Nov 25, 2022
Merged

pkg: fix GetLastError function that was returned always nil#2273
gustavosbarreto merged 1 commit intomasterfrom
fix/getLastErrorNil

Conversation

@henrybarreto
Copy link
Contributor

@henrybarreto henrybarreto commented Nov 24, 2022

Summary

  • Access control: Restrict /review command to shellhub-io/admin team members. Non-admin users get a reply explaining they're not authorized and how to request a review through the admin team.
  • Fail-closed authorization: Use authorized == 'true' instead of != 'false' so unexpected errors (API failures, missing output) deny access rather than granting it.
  • Script injection hardening: Pass all event-derived data (usernames, IDs, event names, branch refs) through env: blocks instead of inline ${{ }} interpolation. Also fixes a pre-existing github.head_ref injection in the branch detection step.
  • Error observability: Parse HTTP status codes from the team membership API — distinguish 404 (not a member) from 403/500/other errors and emit ::warning:: annotations for unexpected responses instead of silently treating all failures as unauthorized.
  • Zero-findings handling: Always post a Step 5 closing comment. When no issues are found, post a clear "no issues" message instead of skipping silently. Tailor follow-up instructions based on whether the PR author is an admin team member.
  • Bot exclusion: Skip automatic review for PRs opened by bots (e.g. Dependabot).

Prerequisite

The GitHub App (CLOUD_DISPATCH_APP_ID / shellhub-ci-dispatch) must have Organization > Members: Read permission for the team membership check. Configure at: https://github.com/organizations/shellhub-io/settings/apps/shellhub-ci-dispatch

Test plan

  • Zero findings: Trigger /review on a PR with no issues — verify the bot posts a "no issues found" closing comment
  • Admin user: An admin team member posts /review — verify the review runs normally with eyes reaction
  • Non-admin user: A non-admin posts /review — verify they get the unauthorized reply, no eyes reaction, and no review runs
  • Auto PR open: Open a new PR — verify the review triggers without any auth check
  • Bot PR: A Dependabot PR opens — verify the workflow is skipped entirely
  • API failure: Temporarily revoke Members:Read permission and trigger /review — verify a ::warning:: annotation appears and access is denied

@henrybarreto henrybarreto added kind/bug Something isn't working status/work-in-progress go Pull requests that update Go code product/cloud Issue/PR related to Cloud Edition product/enterprise Issue/PR related to Enterprise Edition labels Nov 24, 2022
@henrybarreto henrybarreto self-assigned this Nov 24, 2022
@gustavosbarreto gustavosbarreto merged commit 21d9347 into master Nov 25, 2022
@gustavosbarreto gustavosbarreto deleted the fix/getLastErrorNil branch November 25, 2022 12:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

go Pull requests that update Go code kind/bug Something isn't working product/cloud Issue/PR related to Cloud Edition product/enterprise Issue/PR related to Enterprise Edition status/work-in-progress

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants