Skip to content
Merged
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
180 changes: 179 additions & 1 deletion .github/workflows/deploy.yml
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ jobs:

outputs:
published: ${{ steps.check-changes.outputs.has_changes }}
package_versions: ${{ steps.increment.outputs.package_versions }}

steps:
- name: Checkout Project
Expand Down Expand Up @@ -126,10 +127,15 @@ jobs:
echo "npmAuthToken: ${{ env.token }}" >> ~/.yarnrc.yml

- name: Sync and Increment Package Versions
id: increment
if: steps.check-changes.outputs.has_changes == 'true'
run: |
yarn package-tools sync --tag ${GITHUB_REF##*/}
yarn package-tools increment --packages ${{ needs.analyze-changes.outputs.node-recursive }} --tag ${GITHUB_REF##*/}
OUTPUT=$(yarn package-tools increment --packages ${{ needs.analyze-changes.outputs.node-recursive }} --tag ${GITHUB_REF##*/})
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔴 High — Unquoted expression injection in shell

The ${{ needs.analyze-changes.outputs.node-recursive }} expression is expanded directly into the shell command without quoting. This is also a potential injection risk. Pass it through a step-level env: variable instead:

env:
  NODE_RECURSIVE: ${{ needs.analyze-changes.outputs.node-recursive }}
run: |
  OUTPUT=$(yarn package-tools increment --packages "$NODE_RECURSIVE" --tag ${GITHUB_REF##*/})

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

it is not raw user input from a form.

echo "$OUTPUT"
echo "package_versions<<EOF" >> $GITHUB_OUTPUT
echo "$OUTPUT" >> $GITHUB_OUTPUT
echo "EOF" >> $GITHUB_OUTPUT
- name: Build Changed Packages
if: steps.check-changes.outputs.has_changes == 'true'
run: yarn run build:prod
Expand Down Expand Up @@ -348,3 +354,175 @@ jobs:
echo "=========================================="
echo "Tagging complete!"
echo "=========================================="

comment-on-pr:
name: Comment on Released PRs
needs: [publish-npm, publish-tag]
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@riteshfyi comment-on-pr job runs whenever publish-npm and publish-tag complete successfully, regardless of whether packages were actually published. have you handled the case if there is no package published in that situation why we need to post the comment?

Scenario A: Developer merges a PR that only updates README.md (no package changes)
- Should they get a comment? If yes, what should it say? if it is just like PR merged Thanks for your contribution why we even need that?

if: needs.publish-npm.outputs.published == 'true' why don't we add this for that case

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

it will tell the user that no pacakge was affected that's a good thing right?.

runs-on: ubuntu-latest

steps:
- name: Get PR Number
id: get-pr
uses: actions/github-script@v7
with:
script: |
const deploySha = context.sha;
const owner = context.repo.owner;
const repo = context.repo.repo;

try {
const prs = await github.rest.repos.listPullRequestsAssociatedWithCommit({
owner, repo, commit_sha: deploySha
Comment on lines +374 to +375
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 Badge Resolve PRs for all pushed commits, not only HEAD

The PR lookup is keyed only to context.sha, which is just the head commit of the push. When a push to next contains multiple commits (for example, batched merges or catch-up pushes), only one associated PR is discovered and commented, while other merged PRs in the same deployment are missed entirely.

Useful? React with 👍 / 👎.

});
const mergedPR = prs.data.find(pr => pr.merged_at);
if (mergedPR) {
core.setOutput('pr_number', String(mergedPR.number));
return;
}
} catch (error) {
console.log(`Failed to discover PR from commit: ${error.message}`);
}
core.setOutput('pr_number', '');

- name: Post Release Comment on PR
if: steps.get-pr.outputs.pr_number != ''
uses: actions/github-script@v7
with:
Comment on lines +389 to +390
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 Badge Configure a write-capable token for PR comment calls

This github-script step relies on the default github.token, but the workflow never sets github-token here (even though GH_TOKEN is defined globally) and doesn’t declare write permissions for issues/PRs. In repositories/orgs with hardened default GITHUB_TOKEN permissions, the later issues.createComment/issues.updateComment calls will 403, so release comments are silently skipped while the workflow still appears successful.

Useful? React with 👍 / 👎.

script: |
const prNumber = parseInt('${{ steps.get-pr.outputs.pr_number }}');

const raw = `${{ needs.publish-npm.outputs.package_versions }}`;
const packageVersions = {};
for (const line of raw.split('\n')) {
const match = line.match(/^(.+?)\s+=>\s+(.+)$/);
if (match) packageVersions[match[1].trim()] = match[2].trim();
}
const packageEntries = Object.entries(packageVersions);

const owner = context.repo.owner;
const repo = context.repo.repo;
const repoUrl = `https://github.com/${owner}/${repo}`;
const hasPackages = packageEntries.length > 0;

const taggablePackages = {
'@webex/widgets': 'webex-widgets',
'@webex/cc-widgets': 'webex-cc-widgets'
};

const aggregators = ['@webex/cc-widgets', '@webex/widgets'];

let commentBody;

if (hasPackages) {
const primaryPackage = aggregators.find(p => packageVersions[p])
|| packageEntries[0][0];
const primaryVersion = packageVersions[primaryPackage];
const stableVersion = primaryVersion
.replace(/-next\..*/, '')
.replace(/-[a-z]*\..*/, '');

let cname = 'widgets.webex.com';
try {
const cnameFile = await github.rest.repos.getContent({ owner, repo, path: 'docs/CNAME' });
const encoded = cnameFile.data.content;
if (typeof encoded === 'string') {
cname = Buffer.from(encoded, 'base64').toString().trim();
}
} catch (e) {
console.log(`Could not read docs/CNAME, using default changelog host: ${e.message}`);
}

const changelogUrl = new URL(`https://${cname}/changelog/`);
if (stableVersion) {
changelogUrl.searchParams.set('stable_version', stableVersion);
}
changelogUrl.searchParams.set('package', primaryPackage);
changelogUrl.searchParams.set('version', primaryVersion);

const tagLinkParts = Object.entries(taggablePackages)
.filter(([pkg]) => packageVersions[pkg])
.map(([pkg, prefix]) => {
const tag = `${prefix}-v${packageVersions[pkg]}`;
return `[\`${tag}\`](${repoUrl}/releases/tag/${tag})`;
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 Badge Link pushed tags to tag pages instead of releases

The PR comment builds tag links as .../releases/tag/${tag}, but in the workflows I inspected this pipeline only creates and pushes Git tags (no GitHub Release creation step). In that setup, releases/tag/... URLs can point to missing release pages, so the “Released in” links in the comment break for contributors. Point to tag URLs (for example /tree/${tag} or /tags) or create releases before using release URLs.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

have to decide on this.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

the tag link finally redirects to releases/tag/${tag} only

});
const releaseLine = tagLinkParts.length
? `| **Released in:** ${tagLinkParts.join(', ')} |`
: '';

const rows = packageEntries
.sort(([a], [b]) => {
if (a === '@webex/cc-widgets') return -1;
if (b === '@webex/cc-widgets') return 1;
if (a === '@webex/widgets') return -1;
if (b === '@webex/widgets') return 1;
return a.localeCompare(b);
})
.map(([pkg, ver]) => `| \`${pkg}\` | \`${ver}\` |`)
.join('\n');

const packagesTable = [
'',
'| Packages Updated | Version |',
'|---------|---------|',
rows,
''
].join('\n');

commentBody = [
'| :tada: Your changes are now available! |',
'|---|',
releaseLine,
`| :book: **[View full changelog →](${changelogUrl})** |`,
packagesTable,
'Thank you for your contribution!',
'',
`_:robot: This is an automated message. For queries, please contact [support](https://developer.webex.com/support)._`
].filter(Boolean).join('\n');
} else {
commentBody = [
':white_check_mark: **Your changes have been merged!**',
'',
'Thank you for your contribution!',
'',
`_:robot: This is an automated message. For queries, please contact [support](https://developer.webex.com/support)._`
].join('\n');
}

try {
const pr = await github.rest.pulls.get({ owner, repo, pull_number: prNumber });
if (!pr.data.merged_at) return;

const comments = await github.paginate(github.rest.issues.listComments, {
owner, repo,
issue_number: prNumber,
per_page: 100
});

const detailedComment = comments.find(comment =>
comment.user.type === 'Bot' &&
comment.body.includes('Your changes are now available')
);
const mergedComment = comments.find(comment =>
comment.user.type === 'Bot' &&
comment.body.includes('Your changes have been merged')
);

if (detailedComment) return;
if (!hasPackages && mergedComment) return;

if (mergedComment && hasPackages) {
await github.rest.issues.updateComment({
owner, repo,
comment_id: mergedComment.id,
body: commentBody
});
} else {
await github.rest.issues.createComment({
owner, repo,
issue_number: prNumber,
body: commentBody
});
}
} catch (error) {
core.warning(`Failed to comment on PR #${prNumber}: ${error.message}`);
}
Loading