GoogleCloudPlatform / GoogleCloudPlatform/knowledge-catalog

mdcode: a 403 reads as "not found" — pull reports success, push attempts a create

Open
#308 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
TypeScript
Stars
9.2k
Forks
782
Avg merge
6h 36m
Merged PRs (30d)
85

Description

Found while working on a change in the same area, not from a bug report.

The Catalog API returns 403 for both "does not exist" and "you lack permission"
— noted in the code at `src/libts/sync.ts:47-49`. Both sync paths treat a
non-200 as "absent", which produces a wrong outcome in each.

### pull reports success for a snapshot it did not take

```ts
// sync.ts:50-56
if (res.status != 200 || !res.result) {
continue;
}
...
return { success: true };
```

A pull in which every entry 403s skips every entry and returns
`{success: true}`. The user sees `Successfully updated local snapshot` over an
empty or partial snapshot, with nothing indicating entries were skipped.

### push attempts a create on an entry that exists

```ts
// sync.ts:82-88
const exist = await this._catalog.lookupEntry(project, location, entry.name);
if (exist.status != 200 || !exist.result) {
// ... createEntry ...
if (createEntryRes.status != 200) {
return { success: false, details: `Failed to create entry ...` };
}
```

A 403 on the lookup reads as "not there", so push tries to create it, the
service returns 409, and the whole push aborts reporting `Failed to create
entry` — which points at the wrong problem. Earlier entries in the loop have
already been written, and there is no rollback.

### Reproduce

Run `kcmd pull` (or `push`) as a principal with catalog access but without read
permission on the entries in the group.

Observed: pull exits 0 having written nothing; push exits 1 naming a create
failure.
Expected: something that distinguishes "absent" from "forbidden", or at least
does not report success.

Counting skips and reporting them would separate the two cases without needing
the API to.

---

cc @libei

Contributor guide

Open the contributing guide

Research direction

Start in src/libts/sync.ts at the lookup and status handling around lines 47-49 and 82-88. Run kcmd pull and kcmd push with catalog access but without read permission on the entries, then verify forbidden entries are not treated as absent and pull does not report success after skipping them.

Written by the indexing model from the issue text.

Assessment

Tech stack
typescript
Domain
api, cli
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
70/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.