feat: app publisher identity in the manifest + CLI pipeline (v0.10.0) [BDOK-678] - #24
Merged
Merged
Conversation
Phase 2 of app identity (BDOK-678). Adds optional publisher identity to bagdock.json and wires it through the CLI. Publisher fields inherit from the owning org's profile by default and override per field, so the existing first-party apps need no manifest edits. - config.ts: `BagdockJson` gains `publisher` (company/website/supportEmail/ docsUrl/privacyPolicy), `icon` (repo-relative path to a square PNG/SVG), and `description`. Field names follow the BDOK-560 camelCase-manifest convention. - validate: dependency-free icon checks (square, PNG/SVG, >=128px, <=256KB via PNG IHDR / SVG width|height|viewBox parsing) + publisher field validation. A public app with no publisher block WARNs (the offline CLI cannot see the org profile that may complete it), never fails. - init: inherit-by-default (no publisher block scaffolded); interactive, TTY-gated overrides. New `--yes` skips the prompts for scripted use. - deploy: ships `publisher` + `description` in metadata and the icon bytes as an `icon` file part; surfaces the server's new `publisher_warning`. - submit: advisory publisher-completeness pre-check for public apps. Requires the server-side deploy handler that consumes `metadata.publisher` and the `icon` part (ships first). Bumps to 0.10.0.
WalkthroughBagdock 0.10.0 adds optional publisher identity, description, and icon manifest fields. Initialisation can collect publisher overrides, validation checks publisher data and icons, deployment uploads the metadata and icon, and submission reports incomplete publisher identity for public apps. ChangesPublisher identity and icon workflow
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/deploy.ts`:
- Around line 141-150: The icon upload logic in deploy must prevent arbitrary
file disclosure. Before reading bytes in the config.icon path, resolve the path
and ensure it remains within the repository, then reuse checkIcon’s format and
size validation so only supported icons are uploaded; skip or reject invalid
paths and files without reading them.
In `@src/validate.ts`:
- Around line 191-202: Update the Icon validation flow in the shown path after
resolving abs: catch statSync and readFileSync filesystem errors, return an Icon
failure instead of allowing validation to terminate, and reject non-file paths
by checking stat.isFile(). Preserve the existing missing-file and size-limit
failures for valid regular files.
- Around line 201-218: Update the PNG branch around pngDimensions in
src/validate.ts:201-218 to fully decode or structurally validate PNG chunks
before returning a passing result, rejecting truncated header-only data;
preserve the existing dimension and minimum-size checks. In
tests/validate.test.ts:10-16, replace makePng() with a complete valid PNG
fixture and add coverage asserting truncated PNG files are rejected.
- Around line 253-264: Update the publisher validation guard in the surrounding
validation function to reject null explicitly alongside non-object and array
values, before calling Object.keys or Object.entries. In the publisher field
loop, stop skipping null values; treat null like undefined or any non-string
value and add the existing non-empty-string validation error, while preserving
valid string handling.
- Around line 218-226: Update the SVG validation branch using svgDimensions and
ICON_MIN_PX so parsing must yield positive dimensions, both dimensions meet the
128px minimum, and the aspect ratio remains square. Return an Icon failure for
missing, malformed, undersized, or non-square dimensions; preserve the existing
pass result for valid SVGs.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 73dab82c-6820-4e2c-a99a-232fcffa2e9f
📒 Files selected for processing (9)
CHANGELOG.mdbin/bagdock.tspackage.jsonsrc/config.tssrc/deploy.tssrc/init.tssrc/submit.tssrc/validate.tstests/validate.test.ts
Comment on lines
+141
to
+150
| if (config.icon) { | ||
| const iconPath = join(cwd, config.icon) | ||
| if (existsSync(iconPath)) { | ||
| const iconBytes = readFileSync(iconPath) | ||
| const ext = config.icon.toLowerCase().slice(config.icon.lastIndexOf('.')) | ||
| const iconType = ext === '.svg' ? 'image/svg+xml' : ext === '.png' ? 'image/png' : 'application/octet-stream' | ||
| formData.append('icon', new Blob([iconBytes], { type: iconType }), `icon${ext}`) | ||
| } else { | ||
| console.log(chalk.yellow(` Icon not found at ${config.icon}, skipping upload.`)) | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🔴 Critical | ⚡ Quick win
Prevent arbitrary local-file disclosure through icon.
deploy does not run checkIcon, confine the resolved path to the repository, or restrict extensions. A malicious bagdock.json can therefore set icon to ../… and upload any readable local file to the deploy endpoint. Resolve and enforce repository containment, then apply the same format and size validation before reading bytes.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/deploy.ts` around lines 141 - 150, The icon upload logic in deploy must
prevent arbitrary file disclosure. Before reading bytes in the config.icon path,
resolve the path and ensure it remains within the repository, then reuse
checkIcon’s format and size validation so only supported icons are uploaded;
skip or reject invalid paths and files without reading them.
Comment on lines
+191
to
+202
| const abs = join(dir, iconPath) | ||
| if (!existsSync(abs)) { | ||
| return { name: 'Icon', status: 'fail', message: `File not found: ${iconPath}` } | ||
| } | ||
|
|
||
| const size = statSync(abs).size | ||
| if (size > ICON_MAX_BYTES) { | ||
| return { name: 'Icon', status: 'fail', message: `${(size / 1024).toFixed(0)} KB exceeds the ${ICON_MAX_BYTES / 1024} KB limit` } | ||
| } | ||
|
|
||
| const ext = iconPath.toLowerCase().slice(iconPath.lastIndexOf('.')) | ||
| const buf = readFileSync(abs) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Reject directories and handle filesystem errors as validation failures.
A directory or unreadable path can make readFileSync() throw, terminating bagdock validate instead of returning an Icon failure. Check stat.isFile() and catch statSync/readFileSync errors.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/validate.ts` around lines 191 - 202, Update the Icon validation flow in
the shown path after resolving abs: catch statSync and readFileSync filesystem
errors, return an Icon failure instead of allowing validation to terminate, and
reject non-file paths by checking stat.isFile(). Preserve the existing
missing-file and size-limit failures for valid regular files.
Comment on lines
+201
to
+218
| const ext = iconPath.toLowerCase().slice(iconPath.lastIndexOf('.')) | ||
| const buf = readFileSync(abs) | ||
|
|
||
| if (ext === '.png' || isPng(buf)) { | ||
| const dims = pngDimensions(buf) | ||
| if (!dims) { | ||
| return { name: 'Icon', status: 'fail', message: `${iconPath} is not a valid PNG` } | ||
| } | ||
| if (dims.width !== dims.height) { | ||
| return { name: 'Icon', status: 'fail', message: `${iconPath} must be square (got ${dims.width}×${dims.height})` } | ||
| } | ||
| if (dims.width < ICON_MIN_PX) { | ||
| return { name: 'Icon', status: 'fail', message: `${iconPath} is ${dims.width}px — must be at least ${ICON_MIN_PX}px` } | ||
| } | ||
| return { name: 'Icon', status: 'pass', message: `${iconPath} (${dims.width}×${dims.height} PNG, ${(size / 1024).toFixed(0)} KB)` } | ||
| } | ||
|
|
||
| if (ext === '.svg' || isSvg(buf)) { |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Validate complete PNG files rather than header-only fixtures.
The implementation and test fixture jointly establish a truncated 24-byte header as a valid PNG.
src/validate.ts#L201-L218: fully decode or structurally validate PNG chunks before accepting the icon.tests/validate.test.ts#L10-L16: replacemakePng()with a complete PNG and add a truncated-file rejection test.
📍 Affects 2 files
src/validate.ts#L201-L218(this comment)tests/validate.test.ts#L10-L16
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/validate.ts` around lines 201 - 218, Update the PNG branch around
pngDimensions in src/validate.ts:201-218 to fully decode or structurally
validate PNG chunks before returning a passing result, rejecting truncated
header-only data; preserve the existing dimension and minimum-size checks. In
tests/validate.test.ts:10-16, replace makePng() with a complete valid PNG
fixture and add coverage asserting truncated PNG files are rejected.
Comment on lines
+218
to
+226
| if (ext === '.svg' || isSvg(buf)) { | ||
| const dims = svgDimensions(buf.toString('utf-8')) | ||
| if (dims && dims.width > 0 && dims.height > 0) { | ||
| const ratio = dims.width / dims.height | ||
| if (ratio < 0.98 || ratio > 1.02) { | ||
| return { name: 'Icon', status: 'fail', message: `${iconPath} must be square (viewBox/size is ${dims.width}×${dims.height})` } | ||
| } | ||
| } | ||
| return { name: 'Icon', status: 'pass', message: `${iconPath} (SVG, ${(size / 1024).toFixed(0)} KB)` } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Fail SVGs without parseable square dimensions and enforce 128px.
An empty or malformed .svg passes when svgDimensions() returns null, and a 64×64 SVG also passes despite the documented minimum. Require dimensions and apply ICON_MIN_PX consistently.
Proposed validation adjustment
if (ext === '.svg' || isSvg(buf)) {
const dims = svgDimensions(buf.toString('utf-8'))
- if (dims && dims.width > 0 && dims.height > 0) {
- const ratio = dims.width / dims.height
- if (ratio < 0.98 || ratio > 1.02) {
- return { name: 'Icon', status: 'fail', message: `${iconPath} must be square (viewBox/size is ${dims.width}×${dims.height})` }
- }
+ if (!dims || dims.width <= 0 || dims.height <= 0) {
+ return { name: 'Icon', status: 'fail', message: `${iconPath} must declare valid width/height or viewBox dimensions` }
+ }
+ const ratio = dims.width / dims.height
+ if (ratio < 0.98 || ratio > 1.02) {
+ return { name: 'Icon', status: 'fail', message: `${iconPath} must be square (viewBox/size is ${dims.width}×${dims.height})` }
+ }
+ if (dims.width < ICON_MIN_PX || dims.height < ICON_MIN_PX) {
+ return { name: 'Icon', status: 'fail', message: `${iconPath} must be at least ${ICON_MIN_PX}px` }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if (ext === '.svg' || isSvg(buf)) { | |
| const dims = svgDimensions(buf.toString('utf-8')) | |
| if (dims && dims.width > 0 && dims.height > 0) { | |
| const ratio = dims.width / dims.height | |
| if (ratio < 0.98 || ratio > 1.02) { | |
| return { name: 'Icon', status: 'fail', message: `${iconPath} must be square (viewBox/size is ${dims.width}×${dims.height})` } | |
| } | |
| } | |
| return { name: 'Icon', status: 'pass', message: `${iconPath} (SVG, ${(size / 1024).toFixed(0)} KB)` } | |
| if (ext === '.svg' || isSvg(buf)) { | |
| const dims = svgDimensions(buf.toString('utf-8')) | |
| if (!dims || dims.width <= 0 || dims.height <= 0) { | |
| return { name: 'Icon', status: 'fail', message: `${iconPath} must declare valid width/height or viewBox dimensions` } | |
| } | |
| const ratio = dims.width / dims.height | |
| if (ratio < 0.98 || ratio > 1.02) { | |
| return { name: 'Icon', status: 'fail', message: `${iconPath} must be square (viewBox/size is ${dims.width}×${dims.height})` } | |
| } | |
| if (dims.width < ICON_MIN_PX || dims.height < ICON_MIN_PX) { | |
| return { name: 'Icon', status: 'fail', message: `${iconPath} must be at least ${ICON_MIN_PX}px` } | |
| } | |
| return { name: 'Icon', status: 'pass', message: `${iconPath} (SVG, ${(size / 1024).toFixed(0)} KB)` } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/validate.ts` around lines 218 - 226, Update the SVG validation branch
using svgDimensions and ICON_MIN_PX so parsing must yield positive dimensions,
both dimensions meet the 128px minimum, and the aspect ratio remains square.
Return an Icon failure for missing, malformed, undersized, or non-square
dimensions; preserve the existing pass result for valid SVGs.
Comment on lines
+253
to
+264
| if (typeof pub !== 'object' || Array.isArray(pub)) { | ||
| checks.push({ name: 'Publisher', status: 'fail', message: '"publisher" must be an object of { company?, website?, supportEmail?, docsUrl?, privacyPolicy? }' }) | ||
| return checks | ||
| } | ||
|
|
||
| const problems: string[] = [] | ||
| const unknown = Object.keys(pub).filter((k) => !PUBLISHER_FIELDS.includes(k as any)) | ||
| if (unknown.length) problems.push(`unknown field(s): ${unknown.join(', ')}`) | ||
|
|
||
| for (const [field, val] of Object.entries(pub)) { | ||
| if (val === undefined || val === null) continue | ||
| if (typeof val !== 'string' || !val.trim()) { problems.push(`"${field}" must be a non-empty string`); continue } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Handle null publisher values without crashing or silently accepting them.
publisher: null reaches Object.keys(pub) and crashes validation. Field-level null values are also skipped and reported as valid despite the string-only schema. Reject both forms explicitly.
Proposed fix
- if (typeof pub !== 'object' || Array.isArray(pub)) {
+ if (pub === null || typeof pub !== 'object' || Array.isArray(pub)) {
checks.push({ name: 'Publisher', status: 'fail', message: '"publisher" must be an object of { company?, website?, supportEmail?, docsUrl?, privacyPolicy? }' })
return checks
}
@@
- if (val === undefined || val === null) continue
+ if (val === undefined) continue
if (typeof val !== 'string' || !val.trim()) { problems.push(`"${field}" must be a non-empty string`); continue }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if (typeof pub !== 'object' || Array.isArray(pub)) { | |
| checks.push({ name: 'Publisher', status: 'fail', message: '"publisher" must be an object of { company?, website?, supportEmail?, docsUrl?, privacyPolicy? }' }) | |
| return checks | |
| } | |
| const problems: string[] = [] | |
| const unknown = Object.keys(pub).filter((k) => !PUBLISHER_FIELDS.includes(k as any)) | |
| if (unknown.length) problems.push(`unknown field(s): ${unknown.join(', ')}`) | |
| for (const [field, val] of Object.entries(pub)) { | |
| if (val === undefined || val === null) continue | |
| if (typeof val !== 'string' || !val.trim()) { problems.push(`"${field}" must be a non-empty string`); continue } | |
| if (pub === null || typeof pub !== 'object' || Array.isArray(pub)) { | |
| checks.push({ name: 'Publisher', status: 'fail', message: '"publisher" must be an object of { company?, website?, supportEmail?, docsUrl?, privacyPolicy? }' }) | |
| return checks | |
| } | |
| const problems: string[] = [] | |
| const unknown = Object.keys(pub).filter((k) => !PUBLISHER_FIELDS.includes(k as any)) | |
| if (unknown.length) problems.push(`unknown field(s): ${unknown.join(', ')}`) | |
| for (const [field, val] of Object.entries(pub)) { | |
| if (val === undefined) continue | |
| if (typeof val !== 'string' || !val.trim()) { problems.push(`"${field}" must be a non-empty string`); continue } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/validate.ts` around lines 253 - 264, Update the publisher validation
guard in the surrounding validation function to reject null explicitly alongside
non-object and array values, before calling Object.keys or Object.entries. In
the publisher field loop, stop skipping null values; treat null like undefined
or any non-string value and add the existing non-empty-string validation error,
while preserving valid string handling.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
CLI half of BDOK-678 Phase 2 (app identity). Adds optional publisher identity to bagdock.json and wires it through validate / init / deploy / submit. Publisher fields inherit from the owning org's profile by default and override per field, so existing first-party apps need zero manifest edits. Bumps @bagdock/cli 0.9.1 -> 0.10.0.
Changes:
Testing: new tests/validate.test.ts (15 tests); tsc clean; 28/28 pass; validate + init --yes driven end-to-end against fixtures.
Sequencing: requires the server-side deploy handler that consumes metadata.publisher + the icon part (monorepo PR, ships first). Non-breaking either way. Release this after the server reaches prod.
Linear: Refs BDOK-678 (Phase 2; issue stays open for Phases 3-4).
Summary by CodeRabbit
bagdock init, with a non-interactive option.