Skip to content

design-proposal: cozymarketplace supplementary — backend, private sources, publication validation#23

Open
IvanHunters wants to merge 4 commits into
cozystack:mainfrom
IvanHunters:feat/cozymarketplace-supplementary
Open

design-proposal: cozymarketplace supplementary — backend, private sources, publication validation#23
IvanHunters wants to merge 4 commits into
cozystack:mainfrom
IvanHunters:feat/cozymarketplace-supplementary

Conversation

@IvanHunters

@IvanHunters IvanHunters commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

Supplementary, not competing, to #18 by Andrei Kvapil (@kvaps). Accepts the repository-centric model and the meta-index design as-is; fills in three concrete pieces that #18 lists as open questions or leaves implicit:

  1. In-cluster backend for the dashboard marketplace view — a small set of endpoints in cozystack-api, a TapIndex cache controller, and well-defined RBAC.
  2. SecretRef on PackageSourceRef — additive, nil-default CRD field so that one-command cozypkg tap works against private OCI/Git sources. Symmetric with the platform-source change already in flight in feat(operator): support private OCI registry credentials for platform source cozystack#2472.
  3. cozypkg validate + CI gate — offline linter for marketplace repositories, reused in a GitHub Actions workflow that validates submissions before they appear in the meta-index.

Per-package version pinning is explicitly out of scope, siding with #18.

Rendered: design-proposals/cozymarketplace-supplementary/README.md

Related: #18 (parent proposal), cozystack/cozystack#2472 (already-in-flight private platform source), cozystack/cozystack#2455 (motivation issue).

Opening as Draft for early feedback before #18 lands.

Summary by CodeRabbit

  • Documentation
    • Added a new marketplace design proposal for an in-dashboard, in-cluster marketplace experience, including repository browsing, package search, and package catalog metadata.
    • Outlines how private repository “one-command” taps will work using stored credentials, plus defined security boundaries.
    • Introduces an offline marketplace content validation workflow (including linting, dependency checks, and optional signature verification) and a CI gate with a controlled merge policy for index submissions.

…rces, publication validation

Supplementary to cozystack#18 by @kvaps. Accepts the repository-centric
model and fills three concrete pieces that cozystack#18 lists as open
questions or leaves implicit: in-cluster backend for the dashboard
marketplace view, secretRef on PackageSourceRef for one-command
private taps, and a cozypkg validate subcommand plus CI gate for
publication validation. Per-package version pinning remains out of
scope, siding with cozystack#18.

Signed-off-by: IvanHunters <xorokhotnikov@gmail.com>
@coderabbitai

coderabbitai Bot commented Jun 24, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

This pull request adds a supplementary Phase 1 Cozymarketplace design proposal covering marketplace API aggregation, private repository credential handling, offline package validation, CI publication gates, security boundaries, failure behavior, testing, and rollout considerations.

Changes

Cozymarketplace Supplementary Design Proposal

Layer / File(s) Summary
Proposal context and scope
design-proposals/cozymarketplace-supplementary/README.md
Defines the proposal’s relationship to existing designs, its goals and non-goals, and the deferred static catalog approach.
Marketplace and publication workflows
design-proposals/cozymarketplace-supplementary/README.md
Specifies cozystack-api marketplace endpoints, the TapIndex cache controller, Flux-source spec.secretRef credential plumbing without CRD changes, the cozypkg validate command, and CI merge lanes.
Operations, security, and rollout
design-proposals/cozymarketplace-supplementary/README.md
Documents user-facing behavior, compatibility, credential protection, degraded-cache and orphan handling, testing, rollout sequencing, open questions, and rejected alternatives.

Estimated code review effort: 1 (Trivial) | ~5 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main changes: marketplace backend, private source support, and publication validation.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@gemini-code-assist gemini-code-assist 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.

Code Review

This pull request introduces a supplementary design proposal for Cozymarketplace, detailing the backend endpoints, private repository support, and publication validation. Feedback on the proposal highlights an inconsistency regarding the namespace of the referenced Secret, pointing out that the PackageSourceRef struct contains a Namespace field while the text states the Secret must reside in cozy-system.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment on lines +69 to +75
type PackageSourceRef struct {
Kind, Name, Namespace, Path string
SecretRef *corev1.LocalObjectReference // NEW; nil preserves current behaviour
}
```

The `packagesource-reconciler` sets `spec.secretRef` on the materialized Flux source when `SecretRef != nil`. Secret format depends on source kind, matching what Flux source-controller already documents: `kubernetes.io/dockerconfigjson` for OCI; Opaque with `username`+`password` or `bearerToken` for Git over HTTPS; Opaque with `identity` (PEM private key) and `known_hosts` for Git over SSH. The Secret must exist in `cozy-system` before the reconciler runs; otherwise Flux reports a failed condition until it appears. This is symmetric with the platform-source change already in flight in `cozystack/cozystack#2472` — that PR closed the gap for the bootstrap platform source; this CRD field closes the same gap for every user-tapped repository.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

There is an inconsistency between the PackageSourceRef struct definition and the described namespace behavior for the referenced Secret:

  1. The PackageSourceRef struct (lines 69-72) contains a Namespace field, indicating that the referenced or materialized Flux source can reside in namespaces other than cozy-system.
  2. However, line 75 (and line 117) states that the Secret must exist in cozy-system.

Since Flux's spec.secretRef is namespace-scoped and cannot reference secrets across namespaces, the Secret must reside in the same namespace as the materialized Flux source (i.e., the namespace specified by PackageSourceRef.Namespace). If all materialized Flux sources are strictly created in cozy-system, the Namespace field in PackageSourceRef might be redundant or misleading.

@coderabbitai coderabbitai 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.

Actionable comments posted: 3

🤖 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 `@design-proposals/cozymarketplace-supplementary/README.md`:
- Around line 52-63: The marketplace read endpoints are currently described as
open to any authenticated user, which leaks tap/package metadata across tenants
when private taps exist. Update the access model in the `cozystack-api`
marketplace handlers so `GET /marketplace/taps`, `GET
/marketplace/taps/{name}/packages`, and `GET /marketplace/search` only return
taps the caller is authorized to view; add tap-level visibility checks or filter
by the caller’s allowed taps in the `TapIndex`-backed response path.
- Around line 79-81: The GitHub Actions validator flow currently trusts
PR-supplied `source.url`, creating an arbitrary outbound fetch path; tighten the
`cozypkg validate` workflow by adding an allowlist for permitted schemes/hosts
or by running the validator in a network-restricted sandbox before it is used on
PRs. Update the README/workflow description around the `validate` subcommand and
the PR-triggered meta-index job so the `source.url` and `source.tag` inputs are
explicitly constrained before any fetch occurs.
- Around line 115-117: Clarify the tap-removal retention flow by adding an
explicit finalizer/orphaning path in the tap and package lifecycle logic, since
helm.sh/resource-policy: keep alone does not protect controller-owned Package
CRs. Update the tap-removal handling referenced by the OrphanedSource behavior
so installed Package resources remain intact and are marked orphaned until
re-tapped or deleted via cozypkg del, rather than relying only on Helm
retention.
🪄 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: defaults

Review profile: CHILL

Plan: Pro

Run ID: fe5e6b3d-7a1f-4ba0-8e9b-3f8894270d9a

📥 Commits

Reviewing files that changed from the base of the PR and between fbfc6ba and c958f39.

📒 Files selected for processing (1)
  • design-proposals/cozymarketplace-supplementary/README.md

Comment on lines +52 to +63
The dashboard reads marketplace state through a small set of endpoints layered into `cozystack-api`, which already mediates dashboard access to platform CRs and existing auth/RBAC. No new component, no new CRDs — the marketplace state is fully derived from `PackageSource` plus the parsed artifact contents.

| Method | Path | Purpose |
|---|---|---|
| `GET` | `/marketplace/taps` | List connected taps with metadata. |
| `GET` | `/marketplace/taps/{name}/packages` | Packages exposed by one tap. |
| `GET` | `/marketplace/search?q=<query>` | Search across all taps by name, tag, description. |
| `POST` | `/marketplace/taps` | Connect a tap; creates `PackageSource` and, when given, the `Secret`. |
| `DELETE` | `/marketplace/taps/{name}` | Disconnect a tap. Lifecycle of installed `Package` CRs is an open question (see below). |

A `TapIndex` cache controller in the same binary watches `OCIRepository.status.artifact.revision`, pulls the parsed `marketplace.yaml` on each revision change, and serves the GET endpoints from memory. Without it, every search would hit OCI per request. Cluster-admin is required for `POST` and `DELETE`, matching the existing `Package` cluster-scoped model. `GET` is open to any authenticated user so tenant-admins can browse.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

file='design-proposals/cozymarketplace-supplementary/README.md'

echo '--- around lines 1-140 ---'
nl -ba "$file" | sed -n '1,140p'

echo
echo '--- search for auth / private / visibility terms ---'
rg -n -i 'auth|authorization|private|tenant|rbac|cluster-admin|search|browse|visibility|secretref|secret ref|tap' "$file"

Repository: cozystack/community

Length of output: 224


🏁 Script executed:

#!/bin/bash
set -euo pipefail

file='design-proposals/cozymarketplace-supplementary/README.md'

echo '--- around lines 1-140 ---'
cat -n "$file" | sed -n '1,140p'

echo
echo '--- search for auth / private / visibility terms ---'
rg -n -i 'auth|authorization|private|tenant|rbac|cluster-admin|search|browse|visibility|secretref|secret ref|tap' "$file"

Repository: cozystack/community

Length of output: 24411


Scope marketplace browse/search to authorized taps.

GET /marketplace/taps, /marketplace/taps/{name}/packages, and /marketplace/search should not be open to every authenticated user if private taps are supported; otherwise tap names, packages, tags, and descriptions leak across tenants. Scope these endpoints to the caller’s allowed taps or add tap-level visibility checks.

🧰 Tools
🪛 LanguageTool

[grammar] ~58-~58: Ensure spelling is correct
Context: ...e/search?q=| Search across all taps by name, tag, description. | |POST` |...

(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)

🤖 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 `@design-proposals/cozymarketplace-supplementary/README.md` around lines 52 -
63, The marketplace read endpoints are currently described as open to any
authenticated user, which leaks tap/package metadata across tenants when private
taps exist. Update the access model in the `cozystack-api` marketplace handlers
so `GET /marketplace/taps`, `GET /marketplace/taps/{name}/packages`, and `GET
/marketplace/search` only return taps the caller is authorized to view; add
tap-level visibility checks or filter by the caller’s allowed taps in the
`TapIndex`-backed response path.

Comment thread design-proposals/cozymarketplace-supplementary/README.md Outdated
Comment thread design-proposals/cozymarketplace-supplementary/README.md Outdated
Move private-repository credential handling off a proposed
PackageSourceRef.secretRef field and onto the Flux source created at
tap time, matching the merged cozystack/cozystack#2472 pattern: the
PackageSource reconciler only reads SourceRef to build the
ArtifactGenerator and never creates the source, so the field would have
had no reconcile path. Derive marketplace state from PackageSource plus
ApplicationDefinition rather than a new marketplace.yaml, and align tap
and untap semantics, the community index, and the validation reuse with
community#12. No CRD change and no migration.

Signed-off-by: IvanHunters <xorokhotnikov@gmail.com>
…s to cozymarketplace supplementary

Add two mermaid diagrams (backend/tap architecture and the
author/push/validate CI-gate flow) and scannable bold lead-ins on the
Goals section, matching the visual structure of the sibling proposals

Signed-off-by: IvanHunters <xorokhotnikov@gmail.com>
cozystack#18 and cozystack#12.

@kvaps Andrei Kvapil (kvaps) 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.

Reviewing this as the backend / private-source / publication-validation layer that sits on top of #18 (the repository-as-unit model and meta-index) and #12 (the cozypkg authoring + tap/untap workflow and the community index). Read together the three proposals cohere well and this one fills exactly the gaps #18 lists as open questions — the dashboard's server-side data source, the credential plumbing for private taps, and the index-side publication gate. No blocking objections to the design; the notes below are things worth nailing down, roughly in order of how much I'd want them resolved before implementation starts.

What works well

  • Credentials on the Flux source, not on a CRD field. This is the strongest call in the proposal and it's correct. I checked the code: PackageSourceRef is a by-name (Kind, Name, Namespace) reference to an already existing Flux source (api/v1alpha1/packagesource_types.go), and the PackageSource reconciler never constructs an OCIRepository/GitRepository — it only reads the ref to build the ArtifactGenerator. So a secretRef on the ref genuinely would have no reconcile path, and setting spec.secretRef on the source at tap time (symmetric with cozystack/cozystack#2472) is the right layer. Rejecting the earlier PackageSourceRef.secretRef draft was the right move, and the "Alternatives considered" entry captures the reasoning cleanly.
  • No new CRDs, state derived from PackageSource + ApplicationDefinition. Keeping the marketplace state fully derived and additive keeps migration cost at zero and keeps a single source of truth. Good.
  • One cozypkg validate used in two places (author's desk + index CI) is the right shape — a submission fails fast where it's cheap to fix rather than in a consumer's cluster.
  • Rollout / dependency ordering is honest about what depends on #18/#12 landing first vs. what can land independently.

Coherence across #12 / #18 / #23 — one gap to close

The one thing that isn't fully pinned down is the relationship between #12's "community index" (cozystack/packages-index) and #18's "meta-index". This proposal treats them as the same artifact ("#18's meta-index / #12's cozystack/packages-index", and the CI gate targets "the index repository"), but #18 itself doesn't reference #12 at all, and the two describe the index in different terms (repository-of-repositories vs. metadata-only community index). Before the CI validator is built, it's worth stating explicitly, in one place, that these are one index with one schema — otherwise validate.yaml risks being written against an entry shape that #18 and #12 haven't reconciled. This is the main "is anything lost between the docs" risk, and it lives in the seam between the three, not inside this doc.

Security

  • Cross-tenant visibility of private-tap metadata. GET /marketplace/* is open to any authenticated user, while taps are cluster-scoped and admin-created. The doc correctly guarantees Secret names/contents never leak — but the existence, package names, tags and descriptions of a private tap still become visible to every authenticated user, including tenant users who had no part in the admin's decision to tap it. For a private/commercial source that listing itself can be sensitive. I'd like the threat model stated explicitly: is "catalog metadata is cluster-global and visible to all authenticated users" an accepted property, or should private taps be hideable / RBAC-filtered? Right now the doc asserts the safe half (no credential leak) without addressing the metadata half.
  • CI validator fetches a PR-supplied source.url. The index gate resolves an OCI/Git ref straight from an untrusted PR diff and pulls it in a privileged CI context — an arbitrary-outbound-fetch / SSRF surface on a public-submission repo. Worth constraining before this ships: scheme/host allowlist, or run the validator network-sandboxed. The --require-signature path helps only if it's mandatory in CI.
  • Signature verification is opt-in. cosign verification is behind --require-signature and reads as optional in both cozypkg validate and the CI gate. For a marketplace whose whole risk is running third-party charts in an operator's cluster, I'd argue signature verification should be default-on for the index gate (or at minimum required for anything carrying a verified badge), not a flag someone remembers to pass.
  • Privileged-component handling (validation-time badge + install-time confirm) is a good default; no concern.

Smaller points

  • Tap-removal / orphaned Package CRs. The doc leans on #12's warn-and-confirm + OrphanedSource marker, which is fine, but CodeRabbit's point stands: leaving controller-owned Package CRs intact needs an explicit finalizer/ownerRef story, not just Helm resource-policy: keep. Worth a sentence on the mechanism, since it's the part most likely to surprise at implementation time.
  • gemini's namespace nit appears to be against an earlier commit — the current text (Secret in the tap's target community.-prefixed namespace, resolved by source-controller in the source's own namespace) is consistent with the PackageSourceRef.Namespace field that actually exists in the API. No action needed; noting it so it isn't re-litigated.
  • POST /marketplace/taps with inline credentials transits the credential through cozystack-api to create the Secret. The doc says the backend never logs it — good; it'd be worth also stating that inline-cred POST is the discouraged path and secretRef to a pre-created Secret is preferred, so the credential-through-API surface stays narrow.

On scope framing

Small thing, but the "commercial marketplace" is explicitly a non-goal here (and in #18), and this proposal stays entirely in the neutral/OSS layer — private-source support is generic (any private OCI/Git repo), not a commercial-only backend. That's the right boundary; I mention it only because the private-source work is easy to mis-read as "the commercial layer" when it's actually a neutral primitive a future commercial layer could reuse.

Overall: the layering is clean, the central architectural decision is validated against the code, and nothing appears lost between the three proposals except the #12#18 index-identity seam. Resolve that seam and the two security surfaces (metadata visibility model, CI fetch/signature defaults) and this is ready to move to implementation.

@kvaps Andrei Kvapil (kvaps) 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.

Two concrete follow-ups after a closer read (in addition to my earlier review).

1. "community.-prefixed namespace" — wrong layer, and it hides a real gap

The private-tap section says the pull Secret lives in "the community.-prefixed namespace #12 uses". Two problems:

  • In #12, community. is a prefix on the name of the cluster-scoped PackageSource (metadata.name: community.<org>.<repo>), not a namespace. Namespace names are RFC 1123 labels — dots are not allowed — so community.<org>.<repo> is valid as an object name but invalid as a namespace.
  • PackageSource is cluster-scoped and has no namespace at all, whereas the Flux OCIRepository/GitRepository and its pull Secret are namespaced and must co-locate (source-controller resolves secretRef in the source's own namespace).

So the doc actually leaves an open question it currently glosses over: which concrete (dotless) namespace holds the Flux source + pull Secret for a community tap? Suggest naming it explicitly — a fixed namespace, e.g. cozy-community — and keeping the community. prefix only on the cluster-scoped PackageSource name for collision-avoidance. Worth a sentence, because both the backend POST /marketplace/taps handler and cozypkg tap --secret need to agree on where they create the Secret.

2. Publication validation should be two-lane (krew-index style), not "human review for everything"

The doc says the CI gate "does not replace maintainer review" — i.e. every submission waits on a human. That kills the routine-bump UX, which is exactly where a marketplace lives day-to-day. krew-index splits this:

  • Version bump of an existing entry by its owner → auto-merge on green validation, gated on the new tag being signed by the entry's recorded cosign identity (not merely by "the PR author's GitHub account").
  • New entries, or any change to source.url / maintainer / signing identity → maintainer review (these are the security-relevant edits).

Three things fall out of this that are worth adopting:

  • The trust anchor becomes the signing identity, not the GitHub account — a bump still fails if the new artifact isn't signed by the identity maintainers approved originally (the krew "still the same plugin" invariant). This argues for making cosign verification mandatory on the gate, not the current opt-in --require-signature.
  • It bounds the SSRF surface raised in my earlier review: on the auto-merge fast path source.url is unchanged/already-approved; only new/url-change submissions hit an untrusted fetch, and those stay behind a sandboxed validator + human review anyway.
  • It implies the index entry schema should carry an explicit owner + signing identity/issuer, and that an author-side release action opens the bump PR (a krew-release-bot analog) so authors never hand-edit the index.

Neither is blocking — both refine sections already in the doc (the private-namespace detail and the "verified-vs-community" open question). Happy to help sketch the gate workflow if useful.

@kvaps

Copy link
Copy Markdown
Member

One coordination question, mostly for you since #23 builds on both #12 and #18.

Now that #18 reframes the installable/versioned unit as the whole repository — and, in its "Alternatives considered", explicitly folds the package-centric model from #12 in as a possible flat sub-view rather than the primary model — does it still make sense to keep #12 open as a separate proposal?

My read: #12 and #18 describe the same index two different ways (a flat list of packages vs. an index of repositories), and #23's backend currently has to assume they're one artifact with one schema. Collapsing to a single model would remove that ambiguity before we build the TapIndex / validate gate against it.

Concretely I'd suggest: close #12, and carry its still-distinct part — the cozypkg init / push authoring workflow — forward into #18 (or a small standalone proposal), so we're left with one index model for the backend to target.

Does that work for you, or do you see a reason to keep the package-centric index as its own thing?

Fix the private-tap namespace confusion: PackageSource is cluster-scoped
and namespace names forbid dots, so the community. prefix cannot be a
namespace. Name a fixed dotless namespace (cozy-community) for the Flux
source and pull Secret, keeping community. only on the cluster-scoped
PackageSource name.

Make the publication gate two-lane, krew-index style: owner version-bumps
auto-merge on validation signed by the recorded cosign identity, while new
entries and source.url / maintainer / identity changes go to maintainer
review. This makes cosign mandatory on the gate, bounds the CI outbound-fetch
surface, and adds owner + signing identity to the index entry schema.

Signed-off-by: IvanHunters <xorokhotnikov@gmail.com>
@IvanHunters

Copy link
Copy Markdown
Contributor Author

Agreed, let's collapse to a single index model. #18's repository-as-unit is the primary shape, and #23's backend (the TapIndex cache and the validate gate) has to target one entry schema, so keeping #12 open as a parallel package-centric index only re-introduces the ambiguity you point at.

Concretely:

One naming note that came out of addressing your review: I've pinned the community Flux source and its pull Secret to a fixed, dotless cozy-community namespace, keeping community. only on the cluster-scoped PackageSource name (a namespace can't carry the dots). If init/push moves into #18, worth stating that namespace there too, so tap / backend / authoring all agree on where the Secret is created.

@coderabbitai coderabbitai 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.

Actionable comments posted: 1

🧹 Nitpick comments (1)
design-proposals/cozymarketplace-supplementary/README.md (1)

108-113: 🔒 Security & Privacy | 🔵 Trivial | 💤 Low value

Consider RBAC hygiene for the shared cozy-community credential namespace.

Placing every community tap's pull Secret and Flux source in one fixed namespace is a reasonable answer to the dotless-namespace constraint, but it does mean all community-tap credentials are co-located. Since POST/DELETE already require cluster-admin, the incremental risk is bounded, but it's worth explicitly noting in the Security section that any future relaxation of read access to cozy-community (e.g., a support/read-only role) would expose every tapped repository's credentials at once — not just the exception it applies to.

🤖 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 `@design-proposals/cozymarketplace-supplementary/README.md` around lines 108 -
113, Update the Security section to document the RBAC implications of the shared
cozy-community namespace: any future relaxation of read access, including
support or read-only roles, could expose credentials for every community-tapped
repository rather than only the specific resource involved. Keep the existing
cluster-admin requirement for POST/DELETE and shared-namespace design unchanged.
🤖 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 `@design-proposals/cozymarketplace-supplementary/README.md`:
- Around line 93-103: Update the TapIndex-backed GET endpoint design for
/marketplace/taps, /marketplace/taps/{name}/packages, and /marketplace/search so
responses are filtered by the caller’s authorization to view each tap. Define
and apply a tap visibility or tenant/namespace association check, ensuring
private tap metadata is excluded from listings, package results, and search
results while preserving authorized access.

---

Nitpick comments:
In `@design-proposals/cozymarketplace-supplementary/README.md`:
- Around line 108-113: Update the Security section to document the RBAC
implications of the shared cozy-community namespace: any future relaxation of
read access, including support or read-only roles, could expose credentials for
every community-tapped repository rather than only the specific resource
involved. Keep the existing cluster-admin requirement for POST/DELETE and
shared-namespace design unchanged.
🪄 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: defaults

Review profile: CHILL

Plan: Pro

Run ID: 5dbb2757-274f-4ca7-adbc-95d133022c6e

📥 Commits

Reviewing files that changed from the base of the PR and between c958f39 and 9a69e09.

📒 Files selected for processing (1)
  • design-proposals/cozymarketplace-supplementary/README.md

Comment on lines +93 to +103
The dashboard reads marketplace state through a small set of endpoints layered into `cozystack-api`, which already mediates dashboard access to platform CRs and existing auth/RBAC. No new component, no new CRDs — the marketplace state is fully derived from the `PackageSource` resources on the cluster plus the `ApplicationDefinition`s and chart metadata inside each tapped artifact (the shape `#18` and `#12` already define; no new `marketplace.yaml` file is introduced).

| Method | Path | Purpose |
|---|---|---|
| `GET` | `/marketplace/taps` | List connected taps with metadata, derived from `PackageSource` + `ApplicationDefinition`. |
| `GET` | `/marketplace/taps/{name}/packages` | Packages exposed by one tap. |
| `GET` | `/marketplace/search?q=<query>` | Search across all taps by name, tag, description. |
| `POST` | `/marketplace/taps` | Connect a tap: creates the Flux source (with `spec.secretRef` when a Secret is supplied), the `Secret` when inline credentials are given, and the `PackageSource` — the same resources `cozypkg tap` creates. |
| `DELETE` | `/marketplace/taps/{name}` | Disconnect a tap; mirrors `cozypkg untap`. Lifecycle of installed `Package` CRs follows `#12` (see below). |

A `TapIndex` cache controller in the same binary watches each tap's Flux source `status.artifact.revision`, pulls the assembled artifact on each revision change, parses the `PackageSource` + `ApplicationDefinition`s it carries, and serves the GET endpoints from memory. Without it, every search would hit OCI per request. Cluster-admin is required for `POST` and `DELETE`, matching both the cluster-scoped `PackageSource` model and `#12`'s "tap requires cluster-admin". `GET` returns catalog metadata only and is open to any authenticated user so tenant-admins can browse; it never returns Secret names or contents (see Security).

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Marketplace GET endpoints still open to any authenticated user despite private taps.

This still leaves GET /marketplace/taps, /marketplace/taps/{name}/packages, and /marketplace/search open to any authenticated user with no tap-level visibility check, even though private taps (with credentials on the Flux source) are a first-class feature of this proposal. Catalog metadata (tap names, package names, tags, descriptions) for a private tap would still be visible to every tenant, not just the tap's owner/authorized tenants.

♻️ Suggested direction

Scope the TapIndex-backed GET responses to taps the caller is authorized to view (e.g., filter by tenant/namespace association, or add an explicit visibility field on the tap), rather than treating all catalog metadata as globally readable.

🤖 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 `@design-proposals/cozymarketplace-supplementary/README.md` around lines 93 -
103, Update the TapIndex-backed GET endpoint design for /marketplace/taps,
/marketplace/taps/{name}/packages, and /marketplace/search so responses are
filtered by the caller’s authorization to view each tap. Define and apply a tap
visibility or tenant/namespace association check, ensuring private tap metadata
is excluded from listings, package results, and search results while preserving
authorized access.

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